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

[release/6.0] Fix pinned assembly version 6.0 #84357

Merged

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Apr 5, 2023

Backport of #84079 to release/6.0-staging

Customer Impact

Customer using 6.0.1 and 7.0.1 package of System.Management faces assembly load errors when running on .NETFramework, or all frameworks when upgrading from these packages to a non-servicing release (7.0.0, 8.0.0-preview).
Customer using 6.0.1 System.DirectoryServices.Protocols package faces assembly load errors when running on .NETFramework, or all frameworks when upgrading from this packages to a non-servicing release (7.0.0, 8.0.0-preview).

Unhandled Exception: System.IO.FileNotFoundException: Could not load file or assembly 'System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

Customer on .NETFramework who consumes a NETStandard library using either package will see build warnings like:

warning MSB3277: Found conflicts between different versions of "System.DirectoryServices.Protocols" that could not be resolved. 
warning MSB3277: There was a conflict between "System.DirectoryServices.Protocols, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". 
warning MSB3277:     "System.DirectoryServices.Protocols, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was chosen because it was primary and "System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was not. 

Root cause is that we accidentally increased the assembly versions of these libraries (and 5 others that luckily haven't shipped) from 4.0.0.0 to 6|7.0.0.n during our servicing builds.

Testing

Build / diff all assembly versions. Package testing. TODO: Targeted manual regression tests along upgrade path and in cross-framework consumption scenario.
7.0 diff: https://gist.github.com/ericstj/5e914be725066df9824fd1e3fca12c08
6.0 diff: https://gist.github.com/ericstj/83579179870f159bdbbe0c923f457526

Risk

Low, the change is isolated to the 8 libraries that pin their assembly versions to enable .NETFramework support. We further reduced risk by only allowing the higher versions of the 2 libraries that already shipped. To reduce the number of people that need to update to get a fix, we let those 2 libraries stay at the higher version for .NET 7. In .NET 8 we made all 8 libraries reduce their use of this pinned assembly version so-as to make them less special.

… compatibility. The incremental package servicing infrastructure didn't check if the assembly version is pinned and changed it during servicing.

As an example, System.Speech pins its assembly version to 4.0.0.0 but that version gets overwritten during servicing. I.e. for .NET 7 the version would then change to "7.0.0.$(ServicingVersion)" which is incorrect.

Please find the full list of impacted assemblies below:
- System.ComponentModel.Composition
- System.DirectoryServices
- System.DirectoryServices.AccountManagement
- System.DirectoryServices.Protocols
- System.Management
- System.Reflection.Context
- System.Runtime.Caching
- System.Speech

For System.DirectoryServices.Protocols and System.Management we'll only pin the version for the .NETStandard assembly since those previously shipped the newer versions in servicing.
@ghost
Copy link

ghost commented Apr 5, 2023

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #84079 to release/6.0-staging

Customer Impact

Customer using 6.0.1 and 7.0.1 package of System.Management faces assembly load errors when running on .NETFramework, or all frameworks when upgrading from these packages to a non-servicing release (7.0.0, 8.0.0-preview).
Customer using 6.0.1 System.DirectoryServices.Protocols package faces assembly load errors when running on .NETFramework, or all frameworks when upgrading from this packages to a non-servicing release (7.0.0, 8.0.0-preview).

Unhandled Exception: System.IO.FileNotFoundException: Could not load file or assembly 'System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

Customer on .NETFramework who consumes a NETStandard library using either package will see build warnings like:

warning MSB3277: Found conflicts between different versions of "System.DirectoryServices.Protocols" that could not be resolved. 
warning MSB3277: There was a conflict between "System.DirectoryServices.Protocols, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". 
warning MSB3277:     "System.DirectoryServices.Protocols, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was chosen because it was primary and "System.DirectoryServices.Protocols, Version=6.0.0.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was not. 

Root cause is that we accidentally increased the assembly versions of these libraries (and 5 others that luckily haven't shipped) from 4.0.0.0 to 6|7.0.0.n during our servicing builds.

Testing

Build / diff all assembly versions. Package testing. TODO: Targeted manual regression tests along upgrade path and in cross-framework consumption scenario.
7.0 diff: https://gist.github.com/ericstj/5e914be725066df9824fd1e3fca12c08
6.0 diff: https://gist.github.com/ericstj/83579179870f159bdbbe0c923f457526

Risk

Low, the change is isolated to the 8 libraries that pin their assembly versions to enable .NETFramework support. We further reduced risk by only allowing the higher versions of the 2 libraries that already shipped. To reduce the number of people that need to update to get a fix, we let those 2 libraries stay at the higher version for .NET 7. In .NET 8 we made all 8 libraries reduce their use of this pinned assembly version so-as to make them less special.

Author: ericstj
Assignees: ericstj
Labels:

area-Infrastructure-libraries

Milestone: -

@ericstj ericstj changed the title Fix pinned assembly version 6.0 [release/6.0] Fix pinned assembly version 6.0 Apr 5, 2023
@@ -7,7 +7,7 @@
<IsPackable>true</IsPackable>
<!-- If you enable GeneratePackageOnBuild for this package and bump ServicingVersion, make sure to also bump ServicingVersion in Microsoft.Windows.Compatibility.csproj once for the next release. -->
Copy link
Member

Choose a reason for hiding this comment

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

Both OOB packages that are getting built need Microsoft.Windows.Compatibility to be built as well.

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

Just needs the OOB changes in M.W.Compatibility, but otherwise LGTM.

@ericstj ericstj added the Servicing-consider Issue for next servicing release review label Apr 6, 2023
@runfoapp runfoapp bot mentioned this pull request Apr 6, 2023
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 6, 2023
@rbhanda rbhanda added this to the 6.0.16 milestone Apr 6, 2023
@ericstj
Copy link
Member Author

ericstj commented Apr 6, 2023

All failures are known and tracked.

@ericstj ericstj merged commit 3c935ba into dotnet:release/6.0-staging Apr 6, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants