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

Revert versions to f3ac1266 level, where MicrosoftNETCoreApppackageVersion=5.0.0-alpha1.19514.1 #2497

Merged
merged 4 commits into from
Dec 11, 2019

Conversation

vatsan-madhavan
Copy link
Member

@vatsan-madhavan vatsan-madhavan commented Dec 10, 2019

Temporarily, revert versions to f3ac1266 level, where MicrosoftNETCoreApppackageVersion=5.0.0-alpha1.19514.1.

This version is very likely compatible with the WPF repo right now.

--

In dotnet/wpf#2118, transport packages originating from WinForms are unable to build successfully due to a CoreCLR + Compiler problem. This prevents WinForms work from flowing into dotnet/windowsdesktop and ultimately into dotnet/core-sdk.

WPF currently ingests Microsoft.NETCore.App version 5.0.0-alpha1.19514.1 - this can be seen in the LHS of eng/Version.Detail.xml in https://github.com/dotnet/wpf/pull/2118/files. This version comes from WinForms through a CoherentParentDependency like this:

    <Dependency Name="Microsoft.NETCore.App" Version="5.0.0-alpha1.19514.1" CoherentParentDependency="Microsoft.Private.Winforms">

If WinForms repo were moved back temporarily to this version of Microsoft.NETCore.App, then the packages it would subsequently flow into WPF ought to build cleanly, and then flow all the way to dotnet/windowsdesktop and later onto dotnet/core-sdk.

Once WPF gets fixes for C++ compiler and corresponding updates for CoreCLR (which could take a while), WinForms can fast-forward to building against latest versions of CoreFx/CoreCLR.

/cc @RussKie, @AdamYoblick, @merriemcgaw
/cc @rladuca

Microsoft Reviewers: Open in CodeFlow

…rsion=5.0.0-alpha1.19514.1. This version is very likely compatible with the WPF repo right now.
@vatsan-madhavan vatsan-madhavan requested a review from a team as a code owner December 10, 2019 21:03
@AdamYoblick
Copy link
Member

@vatsan-madhavan The only thing I'm worried about here is - all these dependency versions are revved by our darc subscriptions. Is this going to prevent us from taking arcade fixes (via maestro) if we need them?

@vatsan-madhavan
Copy link
Member Author

Arcade updates are in the tools-channel. That should be fine. I'm reverting the whole file to match the f3ac126 - since I didn't want to manually pick and choose what to revert and what to keep. Once this flows correctly, tools-channel updates can continue flowing, but dev-channel updates have to be stopped until we get compiler fixes figured out.

@AdamYoblick
Copy link
Member

Ah ok, then we just have to make sure not to take any other package updates through maestro until this is resolved. @RussKie I guess we just let them sit?

@vatsan-madhavan
Copy link
Member Author

vatsan-madhavan commented Dec 10, 2019

I guess we just let them sit?

yes. until we fix the compiler (and possibly coreclr alongwith it), (a) either winforms+wpf stops corefx+coreclr ingestion; or (b) winforms+wpf stops insertion into core-sdk. (b) is the current state - i'm proposing that we switch to (a). switching to (a) requires that we "rewind" a bit first - that's what this PR does.

@vatsan-madhavan
Copy link
Member Author

i'm not sure how to fixup the failing tests - i think they depend on new API's from corefx that don't exist in the older version... one of you have any ideas?

@AdamYoblick
Copy link
Member

If this is blocking wpf and windowsdesktop insertions, we should just skip the failing tests imo. But I'll defer to @RussKie on this.

@RussKie
Copy link
Member

RussKie commented Dec 10, 2019

I'll have a look.
@AdamYoblick can you please pause the core-sdk updates for the time being?

@AdamYoblick
Copy link
Member

Done.

For reference, here's what I did:

  1. darc get-subscriptions --target-repo https://github.com/dotnet/winforms --target-branch master
https://github.com/dotnet/arcade (.NET Eng - Latest) ==> 'https://github.com/dotnet/winforms' ('master')
  - Id: 4d0c844d-0758-4fc5-c1ad-08d6354da8a8
https://github.com/dotnet/runtime (.NET Core 5 Dev) ==> 'https://github.com/dotnet/winforms' ('master')
  - Id: be4b0f38-c1d5-43ab-c5d9-08d76fa9c820

The second one is the one we're interested in

  1. darc subscription-status --id be4b0f38-c1d5-43ab-c5d9-08d76fa9c820 --disable
  2. darc get-subscriptions --target-repo https://github.com/dotnet/winforms --target-branch master again to verify
https://github.com/dotnet/runtime (.NET Core 5 Dev) ==> 'https://github.com/dotnet/winforms' ('master')
  - Id: be4b0f38-c1d5-43ab-c5d9-08d76fa9c820
  - Update Frequency: EveryDay
  - Enabled: False

And we're good 😄

@RussKie RussKie merged commit b17f8d9 into dotnet:master Dec 11, 2019
@RussKie RussKie deleted the wf-wpf-wd-fixup branch December 11, 2019 01:10
@ghost ghost added this to the 5.0 milestone Dec 11, 2019
@RussKie RussKie removed this from the 5.0 milestone Dec 11, 2019
@RussKie
Copy link
Member

RussKie commented Dec 11, 2019

The internal build succeeded

@vatsan-madhavan
Copy link
Member Author

The pr into wpf is failing for a different reason now. I’ll look.

@vatsan-madhavan
Copy link
Member Author

I closed out the old PR from winforms and triggered a new one - dotnet/wpf#2306.

It conforms to my theory - it's only attempting to update winforms transport package version and little else.

@vatsan-madhavan
Copy link
Member Author

update: dotnet/wpf#2306 is merged.

RussKie added a commit to RussKie/winforms that referenced this pull request Jan 14, 2020
Revert versions to f3ac126 level, where MicrosoftNETCoreApppackageVersion=5.0.0-alpha1.19514.1
until dotnet/wpf#2399 (comment) is resolved.

This version is very likely compatible with the WPF repo right now.
@ghost ghost locked as resolved and limited conversation to collaborators Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants