Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 12830 multi target demo console #12996

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ricardobossan
Copy link
Member

@ricardobossan ricardobossan commented Feb 21, 2025

Fixes #12830

Proposed changes

  • Updated DemoConsole.csproj and DesignSurfaceExt.csproj to support multiple target frameworks ($(NetCurrent)-windows;net481).
  • Adjusted TargetFramework property to avoid over-building.
  • Enabled SignAssembly and GenerateAssemblyInfo properties in the project files.
  • Updated resource and reference handling in DemoConsole.csproj and DesignSurfaceExt.csproj to improve compatibility.
  • Fixed Controls.AddRange usage to use the correct syntax in MainForm.MyUserControl.cs.
  • Corrected Controls.AddRange call in MainForm.cs to Controls.Add.
  • Suppressed IDE0057 warning and reverted to Substring method for compatibility in NameCreationServiceImp.cs.
  • Changed StartsWith('_') check by directly accessing the first character in NameCreationServiceImp.cs for compatibility.
  • Disabled error CA1824 for projects DemoConsole and DesignSurficeExt, because it would be triggered even though properly configured in those projects, according to the fix provided by the documentation.

Customer Impact

  • None

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

  • DemoConsole would not run on net481.

After

image

Test methodology

  • Manual

Test environment(s)

  • 10.0.100-alpha.1.25077.2

@ricardobossan ricardobossan self-assigned this Feb 21, 2025
@ricardobossan
Copy link
Member Author

Drafted because:

  • Need to address nullability warnings
  • net481 compiles without errors but doesn't launch

@ricardobossan
Copy link
Member Author

ricardobossan commented Feb 21, 2025

@Tanya-Solyanik @LeafShi1 @Epica3055 I was able to get the DemoConsole project to compile for multiple targets ($(NetCurrent)-windows and net481), but it doesn't launch on net481. There's no error message explaining why. It just fails silently. Do you have any idea what might be causing this?

I’m aware of the nullability warnings that I still need to address, but could they be related to this issue?

@dotnet-policy-service dotnet-policy-service bot added the draft draft PR label Feb 21, 2025
Copy link
Member

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enable all CLR exceptions before starting to debug this project as 4.8.1. I guess immutable collection reference is not discovered.

@ricardobossan ricardobossan force-pushed the Issue_12830_Multi-Target_DemoConsole branch from e1471f3 to 78c0cbd Compare February 21, 2025 18:03
@Tanya-Solyanik
Copy link
Member

@ricardobossan - the reason the NET481 version does not run is that the dll is not strong-name signed, we should delay-sign it. Arcade should have tools for that. Maybe like this <SignAssembly>true</SignAssembly> https://github.com/dotnet/arcade/blob/ecdb2f499cb5f5c99b58fba1a14fdf977c56e1ac/Documentation/ArcadeSdk.md?plain=1#L953
Search arcade docs to see if they recommend any signing test keys to use. We should not use a production key.

Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.28720%. Comparing base (e00df8a) to head (8e31d0c).
Report is 4 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #12996         +/-   ##
===================================================
+ Coverage   61.03240%   61.28720%   +0.25479%     
===================================================
  Files           1539        1541          +2     
  Lines         158039      158281        +242     
  Branches       14721       14743         +22     
===================================================
+ Hits           96455       97006        +551     
+ Misses         60889       60577        -312     
- Partials         695         698          +3     
Flag Coverage Δ
Debug 61.28720% <ø> (+0.25479%) ⬆️
integration 10.73249% <ø> (-0.00523%) ⬇️
production 39.15640% <ø> (+0.33021%) ⬆️
test 95.67280% <ø> (+0.00238%) ⬆️
unit 36.58217% <ø> (+0.33435%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ricardobossan ricardobossan marked this pull request as ready for review February 24, 2025 20:00
@ricardobossan ricardobossan requested a review from a team as a code owner February 24, 2025 20:00
@ricardobossan ricardobossan added waiting-review This item is waiting on review by one or more members of team and removed draft draft PR labels Feb 24, 2025
@LeafShi1
Copy link
Member

When using only net481 in the DemoConsole project, TabPage6 does not display properly. When enabling all CLR exceptions before starting to debug this project, I receive the following error

image

@ricardobossan
Copy link
Member Author

@LeafShi1 I was able to run the project and navigate through the tabs on net481 with all CLR exceptions enabled, as shown in the GIF below:

clr exceptions enabled net481 works

Here’s how I set up and ran the project:

# From the runtime repo root folder:
gh pr checkout 12996
msbuild /t:Clean .\Winforms.sln
.\Restore.cmd
.\build.cmd
.\start-vs.cmd

Could you share the exact steps you're using to run the project? That might help pinpoint any differences that could explain the discrepancy in our results.

@LeafShi1
Copy link
Member

Could you share the exact steps you're using to run the project? That might help pinpoint any differences that could explain the discrepancy in our results.

When using <TargetFrameworks>$(NetCurrent)-windows;net481</TargetFrameworks>, it shows up fine on my end。
I just tried changing the TargetFramework to net481, like this
image

But it doesn't seem to work, because it causes compilation to fail

So please ignore my comment

@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Feb 26, 2025
ricardobossan pushed a commit to ricardobossan/winforms that referenced this pull request Feb 28, 2025
…e everything visible in the surface designer.

Related dotnet#12996

- Resizes `DemoConsole` project's `MainForm` for convenience and to make everything visible in the surface designer.

- None

- No

- Minimal

- Manual

- 10.0.100-preview.3.25125.5
@ricardobossan ricardobossan added waiting-review This item is waiting on review by one or more members of team and removed waiting-author-feedback The team requires more information from the author labels Mar 3, 2025
ricardobossan added a commit that referenced this pull request Mar 3, 2025
…e everything visible in the surface designer (#13050)

Resizes `DemoConsole` project's `MainForm` for convenience and to make everything visible in the surface designer.

Related #12996

- Resizes `DemoConsole` project's `MainForm` for convenience and to make everything visible in the surface designer.

- None

- No

- Minimal

- Manual

- 10.0.100-preview.3.25125.5

Co-authored-by: Ricardo Bossan (BEYONDSOFT CONSULTING INC) (from Dev Box) <v-rbossan@microsoft.com>
LeafShi1
LeafShi1 previously approved these changes Mar 5, 2025
Copy link
Member

@LeafShi1 LeafShi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When setting up Multi target in a project, we need to make sure it compiles and runs properly in a separate TargetFramework.
@Tanya-Solyanik Is that right?

Copy link
Member

@LeafShi1 LeafShi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the version order in TargetFrameworks is changed to put net481 first, the controls in ToolStripContainer cannot be displayed

<TargetFrameworks>net481;$(NetCurrent)-windows</TargetFrameworks>

image

@LeafShi1 LeafShi1 self-requested a review March 5, 2025 09:43
@Epica3055
Copy link
Member

When I started DemoConsole project, I got this error.

image

@LeafShi1
Copy link
Member

LeafShi1 commented Mar 6, 2025

The PR need to rebase main because the tests under src/System.Windows.Forms/tests/IntegrationTests/DesignSurface have been moved to path src\test\integration\DesignSurface.

@LeafShi1 LeafShi1 added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Mar 6, 2025
LeafShi1 pushed a commit to LeafShi1/winforms that referenced this pull request Mar 6, 2025
…e everything visible in the surface designer (dotnet#13050)

Resizes `DemoConsole` project's `MainForm` for convenience and to make everything visible in the surface designer.

Related dotnet#12996

- Resizes `DemoConsole` project's `MainForm` for convenience and to make everything visible in the surface designer.

- None

- No

- Minimal

- Manual

- 10.0.100-preview.3.25125.5

Co-authored-by: Ricardo Bossan (BEYONDSOFT CONSULTING INC) (from Dev Box) <v-rbossan@microsoft.com>
@ricardobossan ricardobossan force-pushed the Issue_12830_Multi-Target_DemoConsole branch from c6f9052 to 0193c58 Compare March 6, 2025 23:22
… multiple target frameworks (`$(NetCurrent)-windows;net481`).

Fixes dotnet#12830

- Updated `DemoConsole.csproj` and `DesignSurfaceExt.csproj` to support multiple target frameworks (`$(NetCurrent)-windows;net481`).
- Adjusted `TargetFramework` property to avoid over-building.
- Enabled `SignAssembly` and `GenerateAssemblyInfo` properties in the project files.
- Updated resource and reference handling in `DemoConsole.csproj` and `DesignSurfaceExt.csproj` to improve compatibility.
- Fixed `Controls.AddRange` usage to use the correct syntax in `MainForm.MyUserControl.cs`.
- Corrected `Controls.AddRange` call in `MainForm.cs` to `Controls.Add`.
- Suppressed `IDE0057` warning and reverted to `Substring` method for compatibility in `NameCreationServiceImp.cs`.
- Changed `StartsWith('_')` check by directly accessing the first character in `NameCreationServiceImp.cs` for compatibility.
- Disabled error `CA1824` for projects `DemoConsole` and `DesignSurficeExt`, because it would be triggered even though properly configured in those projects, according to the fix provided by the [documentation](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1824#fix-violations).

- None

- No

- Minimal

- `DemoConsole` would not run on `net481`.

- Manual

- 10.0.100-preview.3.25125.5
@ricardobossan ricardobossan force-pushed the Issue_12830_Multi-Target_DemoConsole branch from 0193c58 to 9db5536 Compare March 7, 2025 01:25
@ricardobossan
Copy link
Member Author

@LeafShi1 @Epica3055 Thanks for your comments! I investigated both issues in detail, and here are my findings:

On Leaf's Comment:

  • The ToolStripContainer controls do not appear in the net481 build, regardless of the order of frameworks in the TargetFrameworks tag.
  • On my 2025/02/25 comment, I initially observed that TabPage6 displayed correctly under net481 and provided a video demonstration:
    Issue 12830 multi target demo console #12996 (comment)
  • However, I can now confirm that this issue occurs for me as well, as you described.
  • I retraced my commits using a backup branch from before the rebase, testing each step, but the issue persists. This suggests that an external factor (outside my changes) might have affected the behavior.
  • This needs further investigation.

On Epica's Comment:

  • The "Object reference" error started appearing after the rebase I made to accommodate changes in new commits on the main branch but only affects the net481 build.
  • Before rebasing, the backup branch did not show this error.
  • I confirmed that my code was properly merged, yet this issue emerged post-rebase.
  • This also requires further investigation to identify what changed upstream.

The net10.0-windows build runs flawlessly. However, in net481, debugging triggers the CLR error that Epica mentioned, and the controls inside ToolStripContainer do not appear.

Furthermore, in net481, whether debugging or not, the issue described in Leaf's comment persists: the ToolStripContainer tab stopped showing its controls. This behavior is unexpected since my current code changes previously worked correctly, as demonstrated in my 2025/02/25 video.

I'll look into these issues further after I'm back from my time OOP due to vacation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-author-feedback The team requires more information from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the DemoConsole application multi-targeted to both .NET and .NET Framework
4 participants