Skip to content

Add Microsoft StrongNameKey back to System.Composition* - #45756

Merged
ViktorHofer merged 1 commit into
masterfrom
ViktorHofer-systemcomposition-strongnamekey
Dec 8, 2020
Merged

Add Microsoft StrongNameKey back to System.Composition*#45756
ViktorHofer merged 1 commit into
masterfrom
ViktorHofer-systemcomposition-strongnamekey

Conversation

@ViktorHofer

Copy link
Copy Markdown
Member

@ghost

ghost commented Dec 8, 2020

Copy link
Copy Markdown

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

Issue Details

Regressed with dotnet/corefx@9a4ed0d#diff-739e1ffbaee417f430131875c0471f5ab63a11561f06de857adcd32f4eddce15L5 and was reported via #45638.

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries

Milestone: -

@ViktorHofer

Copy link
Copy Markdown
Member Author

/backport to release/5.0

@github-actions

github-actions Bot commented Dec 8, 2020

Copy link
Copy Markdown
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/408248639

<Import Project="..\Directory.Build.props" />
</Project> No newline at end of file
<PropertyGroup>
<StrongNameKeyId>Microsoft</StrongNameKeyId>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an APICompat run that should have caught this or is APICompat ignoring PKT?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's my assumption as well. I believe APICompat currently doesn't validate the PKT in the assembly metadata:

.assembly extern System.Composition.AttributedModel
{
  .publickeytoken = (CC 7B 13 FF CD 2D DD 51 )                         // .{...-.Q
  .ver 5:0:0:0
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know there were some intentional differences, back in Silverlight days and the runtime supported it. Those should be paired with [assembly: AssemblyFlags(AssemblyNameFlags.Retargetable)]. I wonder if we could add this attribute to make it compatible with folks who compiled against the bad PKT 🤔
cc @safern who's looking at API compat scenarios.

@ViktorHofer
ViktorHofer merged commit 7b90eac into master Dec 8, 2020
@ViktorHofer
ViktorHofer deleted the ViktorHofer-systemcomposition-strongnamekey branch December 8, 2020 15:24
@danmoseley

Copy link
Copy Markdown
Contributor

Does this change fix the underlying reason why we broke this twice? 🙂

@ViktorHofer

Copy link
Copy Markdown
Member Author

Does this change fix the underlying reason why we broke this twice? 🙂

Nope, this PR doesn't switch away from MSBuild as the build engine. Jokes aside, I filed https://github.com/dotnet/runtime/issues/45899 to track validation for strong name keys.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants