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

Pin assembly versions to prevent revving those in servicing releases #6101

Merged
merged 2 commits into from
Feb 15, 2022

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Feb 10, 2022

Resolves dotnet/core#7172 (comment)
This is a complimentary work to dotnet/winforms#6667

Proposed changes

Customer Impact

Currently WPF applications built against 6.0.2 won't run on 6.0 GA or 6.0.1
With the fix applications, libraries or NuGets built against 6.0.2 or later can work. Any applications built on 6.0.2 or later that need to target 6.0.0 or 6.0.1 will need to use the workaround provided in dotnet/core#7176. It is not ideal, but is believed to have a lesser impact than going back to 6.0.0.0 versions, and have 6.0.2 broken.

Regression?

  • Yes

Risk

  • Medium. With this fix we'll stop supporting the folks that compiled against 6.0.2 or later and target 6.0.0 or 6.0.1 without the workaround.
    In general, since 6.0.1 and 6.0.2 are security releases, customers are encouraged to update to the latest version.

Screenshots

Before

6.0.1
image
image

6.0.2
image
image

After

A private build for this fix:

image
image

@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Feb 10, 2022
@ghost ghost requested review from fabiant3, ryalanms and SamBent February 10, 2022 04:32
@RussKie RussKie requested review from singhashish-wpf, ericstj and a team and removed request for SamBent, ryalanms and fabiant3 February 10, 2022 04:33
@RussKie RussKie force-pushed the pin_versions6 branch 2 times, most recently from 59b92d1 to f48a6bd Compare February 11, 2022 03:36
@RussKie RussKie marked this pull request as ready for review February 11, 2022 04:39
There's still a risk that some customers building on 6.0.2+ and trying to run on 6.0.0 and 6.0.1 will still have issue, but
those issues can be mitigated with a workaround described in https://github.com/dotnet/core/issues/7176.
-->
<AssemblyVersion>$(MajorVersion).$(MinorVersion).2.0</AssemblyVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as on the WinForms PR, using the .2 patch version should be conditioned on whether we're building for 6.0, and should not apply to 7.0 and forwards.

Copy link
Contributor

Choose a reason for hiding this comment

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

Version here must be 6.0.0.0 or simply 6.0. At least from my point of view and past experience with version pinning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same comment as on the WinForms PR, using the .2 patch version should be conditioned on whether we're building for 6.0, and should not apply to 7.0 and forwards.

This is a 6.0 branch. For the 7.0 branch it is locked at Major.Minor.0.0, see #6099.
Is there a situation where you expect 6.0 spill into another branch?

@dotMorten
Copy link
Contributor

dotMorten commented Feb 11, 2022

Any applications built on 6.0.2 or later that need to target 6.0.0 or 6.0.1 will need to use the workaround provided in dotnet/core#7176. It is not ideal, but is believed to have a lesser impact than going back to 6.0.0.0 versions, and have 6.0.2 broken.

Is it really? In my mind 6.0.2 is already broken. Why can't you roll the version back to 6.0.0? We now don't have to ensure people have .NET 6 installed, but also have the very latest version of it installed. People have to change their build systems or code just to use a class library built with the latest VS.

@RussKie
Copy link
Member Author

RussKie commented Feb 14, 2022

Is it really? In my mind 6.0.2 is already broken. Why can't you roll the version back to 6.0.0? We now don't have to ensure people have .NET 6 installed, but also have the very latest version of it installed. People have to change their build systems or code just to use a class library built with the latest VS.

If we roll back to 6.0.0.0 - we will have 6.0.2 release broken, and any artifacts produced against 6.0.2 will require a recompilation (app, dlls, NuGets).
If we lock at 6.0.2.0 - we prevent running 6.0.2+ artifacts on 6.0.0 and 6.0.1 releases. However both 6.0.1 and 6.0.2 are security releases, and customers (especially enterprise) are encouraged to update to the latest version, and over time we don't expect many 6.0.0 or 6.0.1 installations.
Unfortunately this is a lose-lose situation, and we're choosing the long term viability.

@dsplaisted
Copy link
Member

@dotMorten To add to what @RussKie said:

  • If we revert to the version number 6.0.0, then all DLLs built using the 6.0.2 reference assemblies will be broken. I don't think there would be any other workaround besides rebuilding, so any NuGet packages published like this would be irreversibly broken and would need to be updated.
  • If we leave the version number at 6.0.2, there is a workaround that lets you build an app or library that can run on 6.0.0 or 6.0.1, documented here: Issue building with Windows Desktop 6.0.2 core#7176
  • Whichever change we make, it won't be until next month's release, so by then a lot more of the runtimes should have been updated to at least 6.0.2 anyway.

So that's some of the reasoning behind why we think the least bad thing to do is to probably leave the version numbers at 6.0.2.

@RussKie
Copy link
Member Author

RussKie commented Feb 15, 2022

Approved over email

@RussKie RussKie merged commit 147609f into release/6.0 Feb 15, 2022
@RussKie RussKie deleted the pin_versions6 branch February 15, 2022 07:06
@ghost ghost locked as resolved and limited conversation to collaborators Apr 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage Servicing-approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants