APICompat fails when Roslyn's dependencies move past our own.

[ericstj]

First found here: dotnet/msbuild#8674 (comment)

Describe the bug

API Compat shares some dependencies with Roslyn. Those that are in the framework should get unified on our behalf. Those that are carried with the task will not. The task tries to unify them (it has a deps file) but this won't work if Roslyn uses versions higher than the task. We might consider loading the versions from Roslyn instead of those in the SDK.

To Reproduce

Using 7.0 GA sdk, try to have the task use latest compiler (for example with the compilers nuget package). Apparently this can happen with some combination of SDK / VS version.

Exceptions (if any)

D:\a\1\s\.dotnet\sdk\7.0.203\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error MSB4018: The "Microsoft.DotNet.ApiCompat.Task.ValidatePackageTask" task failed unexpectedly.
System.MissingMethodException: Method not found: 'Void Microsoft.CodeAnalysis.CSharp.CSharpCompilationOptions..ctor(Microsoft.CodeAnalysis.OutputKind, Boolean, System.String, System.String, System.String, System.Collections.Generic.IEnumerable`1<System.String>, Microsoft.CodeAnalysis.OptimizationLevel, Boolean, Boolean, System.String, System.String, System.Collections.Immutable.ImmutableArray`1<Byte>, System.Nullable`1<Boolean>, Microsoft.CodeAnalysis.Platform, Microsoft.CodeAnalysis.ReportDiagnostic, Int32, System.Collections.Generic.IEnumerable`1<System.Collections.Generic.KeyValuePair`2<System.String,Microsoft.CodeAnalysis.ReportDiagnostic>>, Boolean, Boolean, Microsoft.CodeAnalysis.XmlReferenceResolver, Microsoft.CodeAnalysis.SourceReferenceResolver, Microsoft.CodeAnalysis.MetadataReferenceResolver, Microsoft.CodeAnalysis.AssemblyIdentityComparer, Microsoft.CodeAnalysis.StrongNameProvider, Boolean, Microsoft.CodeAnalysis.MetadataImportOptions, Microsoft.CodeAnalysis.NullableContextOptions)'.
   at Microsoft.DotNet.ApiCompatibility.AssemblySymbolLoader..ctor(Boolean resolveAssemblyReferences)
   at Microsoft.DotNet.ApiCompatibility.AssemblySymbolLoaderFactory.Create(Boolean shouldResolveReferences)
   at Microsoft.DotNet.ApiCompatibility.Runner.ApiCompatRunner.CreateAssemblySymbols(IReadOnlyList`1 metadataInformation, ApiCompatRunnerOptions options, Boolean& resolvedExternallyProvidedAssemblyReferences)
   at Microsoft.DotNet.ApiCompatibility.Runner.ApiCompatRunner.ExecuteWorkItems()
   at Microsoft.DotNet.ApiCompat.ValidatePackage.Run(Func`2 logFactory, Boolean generateSuppressionFile, String[] suppressionFiles, String suppressionOutputFile, String noWarn, Boolean enableRuleAttributesMustMatch, String[] excludeAttributesFiles, Boolean enableRuleCannotChangeParameterName, String packagePath, Boolean runApiCompat, Boolean enableStrictModeForCompatibleTfms, Boolean enableStrictModeForCompatibleFrameworksInPackage, Boolean enableStrictModeForBaselineValidation, String baselinePackagePath, String runtimeGraph, Dictionary`2 packageAssemblyReferences, Dictionary`2 baselinePackageAssemblyReferences)
   at Microsoft.DotNet.ApiCompat.Task.ValidatePackageTask.ExecuteCore()
   at Microsoft.NET.Build.Tasks.TaskBase.Execute()
   at Microsoft.DotNet.ApiCompat.Task.ValidatePackageTask.Execute()
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() 

Further technical details

• Include the output of dotnet --info
• The IDE (VS / VS Code/ VS4Mac) you're running on, and its version

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

@dotnet/area-infrastructure-libraries a new issue has been filed in the ApiCompat area, please triage

[ViktorHofer]

This is now again causing dependency flow uptake issues in msbuild: dotnet/msbuild#9642. That PR updates to a newer roslyn which now references SCI and SRM 8.0.0.

[ViktorHofer]

The task tries to unify them (it has a deps file)

Looking at my local SDK folder, I don't see a deps file next to the APICompat task. Should it be included?

C:\Program Files\dotnet\sdk\8.0.101\Sdks\Microsoft.NET.Sdk\tools\net8.0>dir /b
cs
de
es
fr
it
ja
ko
Microsoft.Deployment.DotNet.Releases.dll
Microsoft.DotNet.ApiCompat.Task.dll
Microsoft.DotNet.ApiCompatibility.dll
Microsoft.DotNet.ApiSymbolExtensions.dll
Microsoft.DotNet.PackageValidation.dll
Microsoft.NET.Build.Tasks.dll
pl
pt-BR
ru
tr
zh-Hans
zh-Hant

[jaredpar]

Normal applications built on top of Roslyn do not experience this type of problem. They reference Microsoft.CodeAnalysis.nupkg hence get items like SCI / SRM via transitive references. Can we change APICompat such that it's built more like an external program and hence gets the proper references?

[ericstj]

It is built like that - the problem is that it allows for an externally provided copy of Microsoft.CodeAnalysis. This was done so that we wouldn't ship duplicate copies of CodeAnalysis in the SDK.

[jaredpar]

Why doesn't the installer just do a hard link here? Building dependencies this way is just asking for problems like this.

[ViktorHofer]

This is indeed related to SCI not getting unified:

Interestingly there are also a few NuGet assemblies loaded twice and not unified.

[ViktorHofer]

Trying to load SCI/8.0.0.0 from the roslyn directory via a custom assembly resolver doesn't work, presumably because MSBuild already loaded SCI/7.0.0.0 as part of the task interface and msbuild's binding redirects prohibit unifying to the 8.0.0.0 version.

I believe that this is a desktop msbuild only issue and only happens when binding redirects aren't in-sync with the Roslyn compiler used.

Repro: pkgvalerrdep.zip -> msbuild /t:Restore && msbuild
Note: The version of desktop msbuild must not already have the updated binding redirects for SCI.

I'm not an expert in this area but this sounds to me that we would need a separate AppDomain (.NETFramework) and AssemblyLoadContext (.NETCoreApp) and dynamically instantiate all the CodeAnalysis types that we need.

cc @rainersigwald

[jaredpar]

I'm not an expert in this area but this sounds to me that we would need a separate AppDomain (.NETFramework) and AssemblyLoadContext (.NETCoreApp) and dynamically instantiate all the CodeAnalysis types that we need.

Or ... develop APICompat the way it's intended to be developed and teach the installer to hard link.

I feel like we're in a hole here and think the solution is to dig deeper.

[ViktorHofer]

Sure. I was looking for a short term fix for 8.0.200.

[rainersigwald]

Here's a workaround: use the VS Roslyn bits instead of the ones from the Microsoft.NET.Compiler.Toolset package.

diff --git a/Directory.Build.targets b/Directory.Build.targets
index b57a232300..c0dab293b6 100644
--- a/Directory.Build.targets
+++ b/Directory.Build.targets
@@ -37,4 +37,10 @@
     <RemoveDir Directories="$(_PackageFolderInGlobalPackages)"
                Condition="Exists('$(_PackageFolderInGlobalPackages)')" />
   </Target>
+
+  <Target Name="WorkAroundRoslynMove" AfterTargets="CollectApiCompatInputs">
+    <PropertyGroup>
+      <RoslynAssembliesPath>$(RoslynTargetsPath)</RoslynAssembliesPath>
+    </PropertyGroup>
+  </Target>
 </Project>

[ViktorHofer]

Sounds like an OK workaround. We could condition that target on when running on Desktop and when on an older MSBuildVersion than x. What's the earliest MSBuildVersion that updated the binding redirect for SCI to 8.0.0.0?

[rainersigwald]

I believe that this is a desktop msbuild only issue and only happens when binding redirects aren't in-sync with the Roslyn compiler used.

Yes.

Trying to load SCI/8.0.0.0 from the roslyn directory via a custom assembly resolver doesn't work, presumably because MSBuild already loaded SCI/7.0.0.0 as part of the task interface and msbuild's binding redirects prohibit unifying to the 8.0.0.0 version.

Sort of! What's happening is that C:\Program Files\dotnet\sdk\8.0.101\Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.DotNet.ApiSymbolExtensions.dll has a baked-in dependency on SCI and SRM 5.0.0.0. So the MSBuild 17.8 binding redirects unify it to 7.0.0.0. But the new Roslyn references 8.0.0.0, which is eligible for side-by-side loading . . . but then the types don't match so it explodes.

[rainersigwald]

What's the earliest MSBuildVersion that updated the binding redirect for SCI to 8.0.0.0?

17.9.0 should do the trick for released versions, I'd have to look up the four-part version to get more specific.

[ViktorHofer]

@ericstj I wonder if we should keep close this now given that we don't support the compilers toolset package anymore without an opt-in switch in APICompat.