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

CS8002 Referenced assembly does not have a strong name triggers when it doesn't need to. #76197

Open
blowdart opened this issue Dec 1, 2024 · 9 comments
Assignees
Milestone

Comments

@blowdart
Copy link

blowdart commented Dec 1, 2024

Steps to Reproduce:

  1. Create a .NET 8.0 project
  2. Turn on all analyzers
  3. Add a nuget package or reference to an assembly that is not strong named.
  4. Watch CS8002 light up.

Diagnostic Id:

CS8002: Referenced assembly 'whatever, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name.

Expected Behavior:

According to the strong name docs

Strong naming has no benefits on .NET Core/5+. C# compiler produces CS8002 warning for strong-named assemblies referencing non-strong named assemblies. It is fine to suppress this warning for libraries that target .NET Core/5+ only.

I would expect if I am targeting a runtime later than v5 (it's v8 in my project) the analyzer doesn't trigger at all.

A less optimal fix would be when creating a project targeting >= .net 5 you automatically add the lines to disable it in the project.

(For those searching for what to do about CS8002 add the following to your project if you're targeting a runtime of >= .net 5)

  <PropertyGroup>
    <!-- Disable strongname warning as that's not been enforced since .net 5.-->
    <NoWarn>$(NoWarn);CS8002</NoWarn>
  </PropertyGroup>

Actual Behavior:

The analyzer triggers, and I must supress it, because in release builds I have all warnings configured as errors.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 1, 2024
@jaredpar jaredpar added Feature - File-Local Types File-local types (file types) and removed untriaged Issues and PRs which have not yet been triaged by a lead Feature - File-Local Types File-local types (file types) labels Dec 2, 2024
@jaredpar
Copy link
Member

jaredpar commented Dec 2, 2024

Turn on all analyzers

Shouldn't be necessary here as this is a compiler warning vs. analyzer diagnostic.

I would expect if I am targeting a runtime later than v5 (it's v8 in my project) the analyzer doesn't trigger at all.

The trouble is that this diagnostic comes from the compiler and it has no concept of .NET core. It just sees a bunch of references and makes decisions based on that. There is nothing I can think of in corelib that indicates strong names don't matter. Even types like System.Runtime.CompilerServices.RuntimeFeature exist on .NET Framework.

The compiler could look for say RuntimeFeature.CovariantReturnsOfClasses but that would only fix the issue in net5.0 and higher.

A less optimal fix would be when creating a project targeting >= .net 5 you automatically add the lines to disable it in the project.

For net5.0 and higher we could use CovariantReturnsOfClasses. But for all .NET Core we'd need to suppress the warning at the msbuild level where we have that information.

@jaredpar jaredpar added this to the 17.13 milestone Dec 2, 2024
@blowdart
Copy link
Author

blowdart commented Dec 3, 2024

Congrats @RikkiGibson 😈

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Dec 3, 2024

Is there a need to fix this in unsupported .NET versions (Core 3.1, 5)? Basically, is it adequate to use the RuntimeFeature heuristic here and say, if you don't have some particular old RuntimeFeature way back in 3.1, then sorry, we'll give you an annoying warning.

@jaredpar
Copy link
Member

jaredpar commented Dec 3, 2024

I think that it's fine to just do this on net5.0 and higher. As you mentioned, earlier runtimes are well out of support. Customers can always disable the warning if they want in those environments.

@RikkiGibson
Copy link
Contributor

I created the following project to try and repro the issue: https://github.com/RikkiGibson/repro-roslyn-76197

Here consumer/ is referencing a project which is not strong named. This could be manually changed to a raw Reference for purposes of experimentation. But it seems like a proper ProjectReference should work just as well. Both are "just references" to csc anyway.

  <ItemGroup>
    <Reference Include="reference">
      <HintPath>../reference/bin/Debug/net8.0/reference.dll</HintPath>
    </Reference>
  </ItemGroup>

However I wasn't able to repro the issue. It seems like for the warning in the issue description to occur, the project being built must have strong naming enabled. Is there some step I missed which ends up causing that to get switched on? I wasn't sure how to "turn on all analyzers", possibly that is related.

@RikkiGibson RikkiGibson modified the milestones: 17.13, 17.14 Jan 8, 2025
@NikolaMilosavljevic
Copy link
Member

@RikkiGibson - this issue is blocking nuget.client flow to sdk: dotnet/sdk#46290

The issue does not repro in nuget.client repo build, as they are obtaining the package from nuget.org - which is strong-name signed.

Source-build builds the package from source and it does not get strong-name signed, by design.

@jaredpar
Copy link
Member

jaredpar commented Jan 29, 2025

Source-build builds the package from source and it does not get strong-name signed, by design.

That seems like the wrong design then. The binaries that come out of source build should be functionally equivalent to those from official builds. I know it's inconvenient but strong naming impacts scenarios like this.

At the very least we need to be consistent across the stack in our choices here.

@NikolaMilosavljevic
Copy link
Member

@mmitche

@mmitche
Copy link
Member

mmitche commented Jan 29, 2025

Source-build builds the package from source and it does not get strong-name signed, by design.

That seems like the wrong design then. The binaries that come out of source build should be functionally equivalent to those from official builds. I know it's inconvenient but strong naming impacts scenarios like this.

At the very least we need to be consistent across the stack in our choices here.

This isn't an option. The issue is that strong name signing requires use of weak crypto which is disabled on some platforms. RedHat did some work to disable the need to SN sign when building in their environment:

dotnet/runtime#65874 for the runtime discussion about this, with https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets#L7-L13

So really, if we wanted to be consistent between our distro partner source-only builds and MS builds, the only way to go is towards no strong naming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants