Don't overwrite pinned assembly versions in servicing #84079
Conversation
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsFixes #84031 Eight packable projects pin their assembly version for .NET Framework 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:
This change should be backported into release/6.0 and release/7.0.
|
There are eight packable projects that pin their assembly version for .NET Framework compatibility. The incremental package servicing infrastructure didn't check if the assembly version is pinned and always changed it. 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
ViktorHofer
force-pushed
the
FixPinnedAssemblyVersion
branch
from
0986a83
to
02eb3a8
Compare
carlossanlop
added
Servicing-consider
src/libraries/System.Reflection.Context/ref/System.Reflection.Context.csproj
Show resolved
Hide resolved
| <!-- AssemblyVersion is frozen to that which originally shipped in 2.1 --> | ||
| <AssemblyVersion>4.0.0.0</AssemblyVersion> |
There was a problem hiding this comment.
This library only ships inside the Microsoft.NETCore.App shared framework. I assume we don't need to pin its assembly version anymore?
There was a problem hiding this comment.
I think it once had a package and this is left over from that time.
There was a problem hiding this comment.
What is the expected side-effect of removing AssemblyVersion from here?
There was a problem hiding this comment.
The assembly will version in the same way as every other assembly that ships only in the targeting packs (N.0.0.0 which remains frozen in servicing).
There was a problem hiding this comment.
Did you consider only applying AssemblyVersion to the netstandard2.0 assembly -- that way we wouldn't break folks targeting .NETCore who consumed the serviced packages? It also limits the "ugliness" of this pinned assembly version - letting us do the right thing in new versions and only having this fixed version for .NETStandard.
These assemblies pin their AssemblyVersion to be compatible with .NETFramework where the same assembly is inbox. The only TargetFramework we build that is compatible with .NETFramework is netstandard2.0, so only pin the assembly on that framework.
|
I tested this change in servicing and noticed that it isn't behaving quite right. |
Previously assemblies would set AssemblyVersion in Directory.Build.props which was too early, because later packaging.targets could not distinguish between a value that was explicitly set and the default value of the repo. Instead override the value after the Servicing value is set, in Directory.Build.targets
|
Of the 8 packages listed, While I do think we should make the change I mentioned above #84079 (review) to reduce the places where we hardcode the version, I recommend we avoid doing that in servicing for Of the remaining 7 packages, only |
The only thing this target did was a redundant check that AssemblyVersion was set for projects in the Targeting Pack. That check was failing for the NETStandard build of System.DirectoryServices. This target was providing no value, instead we need to fix dotnet#42961 which will be a proper check of our servicing version rules.
ericstj
force-pushed
the
FixPinnedAssemblyVersion
branch
from
7bc3c92
to
09dd019
Compare
|
Here's the 7.0 version of the fix. https://github.com/dotnet/runtime/compare/release/7.0-staging...ericstj:runtime:FixPinnedAssemblyVersion-7.0?expand=1 Here's the diff of assembly versions: https://gist.github.com/ericstj/5e914be725066df9824fd1e3fca12c08 Note the change for 5 binaries which we never shipped. These were still seeing the |
ghost
locked as resolved and limited conversation to collaborators
Fixes #84031
Eight packable projects pin their assembly version for .NET Framework 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:
This change should be backported into release/6.0 and release/7.0.