Protect strong name key changes in ApiCompat / package validation

[ViktorHofer]

For System.Composition* packages we accidentally changed the strong name key in 5.0 and just reverted that: dotnet/runtime#45756. We should add validation to prevent such mistakes in the future.

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

---

For System.Composition* packages we accidentally changed the strong name key in 5.0 and just reverted that: dotnet/runtime#45756. We should add validation to prevent such mistakes in the future.

Author:	ViktorHofer
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	6.0.0

[ericstj]

Confirmed that APICompat doesn't catch this:

C:\Users\erics\.nuget\packages>dotnet microsoft.dotnet.apicompat\6.0.0-beta.20601.2\tools\netcoreapp2.1\Microsoft.DotNet.ApiCompat.dll system.composition.attributedmodel\1.4.1\lib\netstandard2.0\System.Composition.AttributedModel.dll -i system.composition.attributedmodel\5.0.0\lib\netstandard2.0\ --warn-on-missing-assemblies
Total Issues: 0

C:\Users\erics\.nuget\packages>sn -T system.composition.attributedmodel\1.4.1\lib\netstandard2.0\System.Composition.AttributedModel.dll

Microsoft (R) .NET Framework Strong Name Utility  Version 4.0.30319.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Public key token is b03f5f7f11d50a3a

C:\Users\erics\.nuget\packages>sn -T system.composition.attributedmodel\5.0.0\lib\netstandard2.0\System.Composition.AttributedModel.dll

Microsoft (R) .NET Framework Strong Name Utility  Version 4.0.30319.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Public key token is cc7b13ffcd2ddd51

[safern]

@ericstj should we just make this part of the roslyn based genAPI? instead of fixing it on the current one? If so, I'll self assign and add it to my GenAPI notes.

[ViktorHofer]

If we add this validation in existing tooling (GenAPI) then we could enable it in release/5.0 as well. That said, it's highly unlikely that we would unintentionally change a public key token in a servicing release without anyone noticing.

[safern]

If we add this validation in existing tooling (GenAPI) then we could enable it in release/5.0 as well.

I think the break was due to a major refactor (removing DIrectory.Build.props) which shouldn't be accepted on release/5.0. My question was more like, is it worth it implementing it on both sides, new GenAPI and old GenAPI?

I guess comparing public key tokens at assembly level should be "simple" to implement on existing GenAPI though.

[ViktorHofer]

I think the break was due to a major refactor (removing DIrectory.Build.props) which shouldn't be accepted on release/5.0.

The regression happened in dotnet/corefx@9a4ed0d#diff-739e1ffbaee417f430131875c0471f5ab63a11561f06de857adcd32f4eddce15L5 which didn't remove any Directory.Build.props file but consolidated the AssemblyVersion properties. The problem was that unintentionally the line that specified the public key token was removed as well. Servicing changes usually touch PackageVersion or AssemblyVersion properties for individual libraries (i.e. the current one for 5.0.2).

[safern]

The problem was that unintentionally the line that specified the public key token was removed as well. Servicing changes usually touch PackageVersion or AssemblyVersion properties for individual libraries (i.e. the current one for 5.0.2).

Right, what I meant it was a major refactor to share AssemblyVersion properties and as part of that the StrongNameKey property was removed (accidentally), given the change was quite large we missed it in code review. So I was saying I wouldn't expect 5.0 to have changes like that... while I don't see GenAPI supporting that and including it into 5.0 hurting, I don't know if it is worthwhile the investment of adding support for this to a tool that would be obsolete.

[ViktorHofer]

Setting milestone to 6.0.0 as we believe we want to tackle this as part of the new ApiCompat work.

[ericstj]

I believe this is already handled by the new APICompat:

sdk/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/AssemblyIdentityMustMatch.cs

Line 13 in cf2fd1e

public class AssemblyIdentityMustMatch : Rule