fix: Fail roll forward .NET Core 3.x -> .NET 5.0 by RussKie · Pull Request #2923 · dotnet/winforms

RussKie · 2020-02-28T06:46:55Z

As part of consolidation initiative (refer to #2781) we have merged System.Windows.Forms.Design.Editors.dll into System.Windows.Forms.Design.dll.

However doing so we broke roll-forward upgrades for any app depending on types declared in System.Windows.Forms.Design.Editors.dll.

Resolves #2921.

Proposed changes

Create a facade assembly that forwards types from original assembly.

Customer Impact

Customers can roll forward from .NET Core 3.x to .NET 5.0

Regression?

Yes

Test methodology

Use the repro app or generate a new Windows Forms app targeting .NET Core 3.0 (or 3.1) and set it to rollforward:
```
{
  "sdk": {
    "rollForward": "major"
  }
}
```

Add the following line to Form1:

AnchorEditor editor = new AnchorEditor();

Ensure no .NET Core 3.x runtime is installed. Best to run a VM with only .NET5 Preview1 installed
Run the app, observe the failure:
Build the current commit with pack option:
```
PS> .\build.ps1 -pack
```
Copy System.Windows.Forms.Design.Editors.dll
- from .\artifacts\packages\Debug\NonShipping\Microsoft.Private.Winforms.5.0.0-dev.nupkg\lib\netcoreapp5.0 --> C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App\5.0.0-preview.2.20119.5
- from .\artifacts\packages\Debug\NonShipping\Microsoft.Private.Winforms.5.0.0-dev.nupkg\ref\netcoreapp5.0 --> C:\Program Files\dotnet\packs\Microsoft.WindowsDesktop.App.Ref\5.0.0-preview.2.20119.5\ref\netcoreapp5.0

Update the WindowsDesktop manifest to include the new assembly:

--- a/C:/Program Files/dotnet/shared/Microsoft.WindowsDesktop.App/5.0.0-preview.2.20119.5/Microsoft.WindowsDesktop.App.deps.json
+++ b/C:/Program Files/dotnet/shared/Microsoft.WindowsDesktop.App/5.0.0-preview.2.20119.5/Microsoft.WindowsDesktop.App.deps.json
          "runtimes/win-x64/lib/netcoreapp5.0/System.Windows.Forms.Design.dll": {
            "assemblyVersion": "5.0.0.0",
            "fileVersion": "5.0.20.11805"
          },
+         "runtimes/win-x64/lib/netcoreapp5.0/System.Windows.Forms.Design.Editors.dll": {
+           "assemblyVersion": "5.0.0.0",
+           "fileVersion": "5.0.20.11805"
+         },
          "runtimes/win-x64/lib/netcoreapp5.0/System.Windows.Forms.Primitives.dll": {
            "assemblyVersion": "5.0.0.0",
            "fileVersion": "5.0.20.11805"
          },

Re-run the app, observe the app start

Microsoft Reviewers: Open in CodeFlow

As part of consolidation initiative (refer to dotnet#2781) we have merged System.Windows.Forms.Design.Editors.dll into System.Windows.Forms.Design.dll. However doing so we broke roll-forward upgrades for any app depending on types delcared in System.Windows.Forms.Design.Editors.dll. Create a facade assembly that forwards types from original assembly. Resolves dotnet#2921.

RussKie · 2020-02-28T06:52:28Z

Winforms.sln

 		.editorconfig = .editorconfig
 	EndProjectSection
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Windows.Forms.Design.Editors.Facade3x", "src\System.Windows.Forms.Design.Editors\src\System.Windows.Forms.Design.Editors.Facade3x.csproj", "{E0681991-228A-420E-85D5-A9E796F0AAE0}"


Need a difference "Facade" name for the purpose of having a different set of contract references to .NET Framework.
Naming-wise I was split between System.Windows.Forms.Design.Editors.Facade3x and System.Windows.Forms.Design.Editors.Facade.3x. Happy to change if you think the second one is better.

codecov · 2020-02-28T07:05:48Z

Codecov Report

Merging #2923 into master will decrease coverage by 0.0023%.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##              master       #2923         +/-   ##
===================================================
- Coverage   60.22621%   60.22391%   -0.00231%     
===================================================
  Files           1248        1248                 
  Lines         433396      433396                 
  Branches       38850       38850                 
===================================================
- Hits          261018      261008         -10     
- Misses        167006      167010          +4     
- Partials        5372        5378          +6

Flag	Coverage Δ
#Debug	`60.22391% <ø> (-0.00231%)`	⬇️
#production	`32.4199% <ø> (-0.00396%)`	⬇️
#test	`98.96294% <ø> (ø)`	⬆️

vatsan-madhavan · 2020-02-28T16:48:24Z

Is rollForward from 3 -> 5 supported, or are you doing this as a best-effort solution ?

weltkante · 2020-02-28T17:48:20Z

Is rollForward from 3 -> 5 supported

This probably still needs a decision, some of the changes beeing considered for 5.0 won't be compatible with roll forward, see my notes in the issue (#2921)

vatsan-madhavan · 2020-02-28T18:39:41Z

@ericstj perhaps knows more?

RussKie · 2020-03-02T06:18:24Z

The main driving force behind this PR is to unblock the Designer which is currently unable to resolve type editors/converters previously located in System.Windows.Forms.Design.Editors assembly.

Is rollForward from 3 -> 5 supported, or are you doing this as a best-effort solution ?

It is a best-effort only.
I think with the current API surface it should be possible to roll forward (with this fix and #2932) if an app doesn't rely on any deprecated API (e.g. #2798 and #2157).

RussKie · 2020-03-03T05:02:01Z

I run tests and verified the fix. Added test methodology section to the description.

@dagood I couldn't find how Microsoft.WindowsDesktop.App.runtimeconfig.json gets generated. Is this done by WindowsDesktop project?
I just want to confirm my test procedure wasn't completely wrong, and the new assembly will automagically end up in the config.

ericstj · 2020-03-03T14:20:49Z

Is rollForward from 3 -> 5 supported

The library ecosystem will need to continue working. That is the nature of TFMs.

Apps won’t roll forward by default but nothing prevents an app author from making the choice to roll forward.

Cc @PriyaPurkayastha

PriyaPurkayastha · 2020-03-03T19:05:10Z

Yes, starting .NET Core 3.0, rollForward for major versions is supported. Actually this is how we perform compat testing in the AppCompat lab where we opt-in to major version upgrade (.NET 5) for 2.x/3.x apps.

AdamYoblick

I tested this on a VM and the roll forward seems to be working just fine with these changes. We just need to make sure the WindowsDesktop.App.deps.json gets generated correctly once these changes flow into the WindowsDesktop build.

RussKie · 2020-03-03T23:18:01Z

I'm going to merge it as I don't believe there is anything else to do in this repo.
The manifest belongs to WindowsDesktop, so if it doesn't get updated we'll deal with it there.

dagood · 2020-03-10T18:05:07Z

I couldn't find how Microsoft.WindowsDesktop.App.runtimeconfig.json gets generated. Is this done by WindowsDesktop project?

Yes, here: https://github.com/dotnet/arcade/blob/3679f317fe2e94ce0173ac4955718cd6a4475e7b/src/Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk/targets/framework.sharedfx.targets#L71-L100

As for Microsoft.WindowsDesktop.App.deps.json (I think you're really asking about this?), it's generated via the SDK Publish target, so I'd expect this DLL to get added automatically there: https://github.com/dotnet/arcade/blob/3679f317fe2e94ce0173ac4955718cd6a4475e7b/src/Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk/targets/framework.sharedfx.targets#L284

As part of consolidation initiative (refer to dotnet#2781) we have merged System.Windows.Forms.Design.Editors.dll into System.Windows.Forms.Design.dll. However doing so we broke roll-forward upgrades for any app depending on types delcared in System.Windows.Forms.Design.Editors.dll. Create a facade assembly that forwards types from original assembly. Resolves dotnet#2921.

RussKie requested review from DustinCampbell, Shyam-Gupta and dreddy-work February 28, 2020 06:46

RussKie requested a review from a team as a code owner February 28, 2020 06:46

ghost assigned RussKie Feb 28, 2020

RussKie commented Feb 28, 2020

View reviewed changes

AdamYoblick approved these changes Mar 3, 2020

View reviewed changes

RussKie merged commit 8bb18ff into dotnet:master Mar 3, 2020

RussKie deleted the fix_2921_SWFDE_TF branch March 3, 2020 23:18

ghost added this to the 5.0 milestone Mar 3, 2020

ghost locked as resolved and limited conversation to collaborators Feb 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fail roll forward .NET Core 3.x -> .NET 5.0#2923

fix: Fail roll forward .NET Core 3.x -> .NET 5.0#2923
RussKie merged 1 commit intodotnet:masterfrom
RussKie:fix_2921_SWFDE_TF

RussKie commented Feb 28, 2020 •

edited

Loading

Uh oh!

RussKie Feb 28, 2020

Uh oh!

codecov bot commented Feb 28, 2020

Uh oh!

vatsan-madhavan commented Feb 28, 2020

Uh oh!

weltkante commented Feb 28, 2020 •

edited

Loading

Uh oh!

vatsan-madhavan commented Feb 28, 2020

Uh oh!

RussKie commented Mar 2, 2020

Uh oh!

RussKie commented Mar 3, 2020

Uh oh!

ericstj commented Mar 3, 2020

Uh oh!

PriyaPurkayastha commented Mar 3, 2020

Uh oh!

AdamYoblick left a comment

Uh oh!

RussKie commented Mar 3, 2020

Uh oh!

dagood commented Mar 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

RussKie commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Test methodology

Microsoft Reviewers: Open in CodeFlow

Uh oh!

RussKie Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 28, 2020

Codecov Report

Uh oh!

vatsan-madhavan commented Feb 28, 2020

Uh oh!

weltkante commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vatsan-madhavan commented Feb 28, 2020

Uh oh!

RussKie commented Mar 2, 2020

Uh oh!

RussKie commented Mar 3, 2020

Uh oh!

ericstj commented Mar 3, 2020

Uh oh!

PriyaPurkayastha commented Mar 3, 2020

Uh oh!

AdamYoblick left a comment

Choose a reason for hiding this comment

Uh oh!

RussKie commented Mar 3, 2020

Uh oh!

dagood commented Mar 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

RussKie commented Feb 28, 2020 •

edited

Loading

weltkante commented Feb 28, 2020 •

edited

Loading