search

CommunityToolkit / dotnet

.NET Community Toolkit is a collection of helpers and APIs that work for all .NET developers and are agnostic of any specific UI platform. The toolkit is maintained and published by Microsoft, and part of the .NET Foundation.
https://docs.microsoft.com/dotnet/communitytoolkit/?WT.mc_id=dotnet-0000-bramin
Other
3.08k stars 300 forks source link

Latest preview assemblies are not signed #254

Closed hymccord closed 2 years ago

hymccord commented 2 years ago

Describe the bug

Since v8.0.0-preview.1, the toolkit DLL is no longer strong name signed and cannot be loaded by .NETFramework libraries/executables that are strong named.

Regression

Yes. v7.1.2 is signed.

Steps to reproduce

1. Have a WPF app that targets both `net48` and `net6.0-windows` and is strong named.
2. Try to use the latest preview.
3. Works for NET 6.0 (because strong names no longer matter there) but not for net48.

Expected behavior

CommunityToolkit.Mvvm.dll should be signed with the toolkit.snk that's in the root directory.

Screenshots

v7.1.2

image

v8.0.0-preview

image

IDE and version

VS 2022 Preview

IDE version

17.3-p1

Nuget packages

Nuget package version(s)

v8.0.0-preview.3

Additional context

No response

Help us help you

Yes, I'd like to be assigned to work on this item

Sergio0694 commented 2 years ago

@michael-hawker would you know whether we're missing any steps in the CI to get the assemblies to be signed? I though that was a requirement since we're publishing through the .NET Foundation, is the issue just for the preview ones?

hymccord commented 2 years ago

In the original WindowCommunityTookit, this portion in the Directory.Build.targets handled it.

In this repo, all you need is the following code. I can submit a PR for updating the targets file or it can go in the Directory.Build.props since it's only assigning properties and not having to check target frameworks.

      <PropertyGroup>
        <SignAssembly>true</SignAssembly>
        <AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)toolkit.snk</AssemblyOriginatorKeyFile>
      </PropertyGroup>
Sergio0694 commented 2 years ago

@InKahootz I've assigned you to this work item 🙂 Please keep the check, we should only be signing assemblies for our core projects, not for the tests too.

hymccord commented 2 years ago

@Sergio0694 Guidance on CS1726?

TL;DR Strong-named assemblies can only grant internal access (InternalsVisibleToAttribute) to other strongly signed assemblies.

Any downsides to signing the unit tests? I've always signed everything in my personal projects. Never had any issues. I checked dotnet/roslyn and dotnet/runtime. Everything (managed) strong-named.

I'm going to go ahead and sign everything and put up a PR. We can discuss if there are alternatives.