dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.17k stars 1.34k forks source link

[Bug]: `ResolveKeySource` is not a dependency of `CoreCompile` #10306

Open jjonescz opened 3 days ago

jjonescz commented 3 days ago

Issue Description

CoreCompile target calls Csc with KeyFile="$(KeyOriginatorFile)". This KeyOriginatorFile property is set by the ResolveKeySource target. ResolveKeySource target is not a dependency of CoreCompile (although it is a dependency of e.g., Compile and CoreBuild) Most of the time it seems the ResolveKeySource target is actually executed before CoreCompile so everything works fine - but sometimes it doesn't: https://github.com/dotnet/roslyn/issues/74156

Steps to Reproduce

  1. Clone roslyn at 31e6ba81af842e960aa04850b0e63229874d4f02
  2. Run .\Restore.cmd
  3. Open in VS IntPreview Version 17.11.0 Preview 3.0 [35026.344.main]
  4. Run Build > Build Solution - succeeds
  5. Run Build > Build Solution again - fails - see https://github.com/dotnet/roslyn/issues/74156

Creating a minimal repro would be complicated, I don't know the exact conditions needed to get the buggy ordering of targets.

Here's the binlog (from a VS build) where I saw the issue: vs.binlog.zip

Expected Behavior

Build succeeds the second time.

Actual Behavior

CSC error CS8102: Public signing was specified and requires a public key, but no public key was specified. [D:\roslyn-D\src\VisualStudio\CSharp\Impl\Microsoft.VisualStudio.LanguageServices.CSharp.csproj]

Analysis

No response

Versions & Configurations

No response

rainersigwald commented 2 days ago

In that binlog:

  1. CoreCompile is running because of <Target Name="_SetGeneratedOutputItems" DependsOnTargets="CoreCompile"> from microsoft.visualstudio.extensibility.jsongenerators.sdk\17.10.2079\build\Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk.props
  2. That's depended on by ExtensionJsonOutputGroup
  3. That's explicitly requested in the reference through VSSDK targets by

https://github.com/dotnet/roslyn/blob/879be6a9604d74f51916939625856e6602303256/src/VisualStudio/Setup/Roslyn.VisualStudio.Setup.csproj#L209C146-L209C170

I think two things should be done:

  1. The VSSDK should be updated so that reference is Compile instead of CoreCompile, which would trigger all of $(CompileDependsOn) in order and work.
  2. CoreCompile for VB and C# should consider adding an explicit dependency on ResolveKeySource.

The reason I'm saying "consider" for 2 is that right now CoreCompile doesn't really have any explicit dependencies and depends on the implicit $(BuildDependsOn)/$(CompileDependsOn) ordering. That can produce other problems like this (I'm a bit surprised for example that NuGet references made it into this compilation)--but adding a constraints to target order has been a surprisingly breaking operation in the past due to how people have hooked the fragile existing system.