diff --git a/src/Compilers/Server/VBCSCompiler/AnalyzerConsistencyChecker.cs b/src/Compilers/Server/VBCSCompiler/AnalyzerConsistencyChecker.cs index ba0b1f6507569..e9f5c24b08600 100644 --- a/src/Compilers/Server/VBCSCompiler/AnalyzerConsistencyChecker.cs +++ b/src/Compilers/Server/VBCSCompiler/AnalyzerConsistencyChecker.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#if NETFRAMEWORK + using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -90,13 +92,32 @@ private static bool CheckCore( } // Third, check that the MVIDs of the files on disk match the MVIDs of the loaded assemblies. + var comparer = PathUtilities.Comparer; + var compilerDirectory = Path.GetDirectoryName(typeof(AnalyzerConsistencyChecker).Assembly.CodeBase); + for (int i = 0; i < resolvedPaths.Count; i++) { var resolvedPath = resolvedPaths[i]; var loadedAssembly = loadedAssemblies[i]; + + // When an assembly is loaded from the GAC then the load result would be the same if + // this ran on command line compiler. So there is no consistency issue here, this + // is just runtime rules expressing themselves. + if (loadedAssembly.GlobalAssemblyCache) + { + continue; + } + + // When an assembly is loaded from the compiler directory then this means it's assembly + // binding redirects taking over. For example it's moving from an older version of System.Memory + // to the one shipping in the compiler. This is not a consistency issue. + if (PathUtilities.Comparer.Equals(compilerDirectory, Path.GetDirectoryName(loadedAssembly.CodeBase))) + { + continue; + } + var resolvedPathMvid = AssemblyUtilities.ReadMvid(resolvedPath); var loadedAssemblyMvid = loadedAssembly.ManifestModule.ModuleVersionId; - if (resolvedPathMvid != loadedAssemblyMvid) { var message = $"analyzer assembly '{resolvedPath}' has MVID '{resolvedPathMvid}' but loaded assembly '{loadedAssembly.FullName}' has MVID '{loadedAssemblyMvid}'"; @@ -109,3 +130,5 @@ private static bool CheckCore( } } } + +#endif diff --git a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs index 9513b304ff182..296e8d6ff91e6 100644 --- a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs +++ b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs @@ -74,11 +74,6 @@ internal CompilerServerHost(string clientDirectory, string sdkDirectory, ICompil Logger = logger; } - private bool CheckAnalyzers(string baseDirectory, ImmutableArray analyzers, [NotNullWhen(false)] out List? errorMessages) - { - return AnalyzerConsistencyChecker.Check(baseDirectory, analyzers, AnalyzerAssemblyLoader, Logger, out errorMessages); - } - public bool TryCreateCompiler(in RunRequest request, BuildPaths buildPaths, [NotNullWhen(true)] out CommonCompiler? compiler) { switch (request.Language) @@ -141,16 +136,18 @@ public BuildResponse RunCompilation(in RunRequest request, CancellationToken can return new RejectedBuildResponse(message); } - bool utf8output = compiler.Arguments.Utf8Output; - if (!CheckAnalyzers(request.WorkingDirectory, compiler.Arguments.AnalyzerReferences, out List? errorMessages)) +#if NETFRAMEWORK + if (!AnalyzerConsistencyChecker.Check(request.WorkingDirectory, compiler.Arguments.AnalyzerReferences, AnalyzerAssemblyLoader, Logger, out List errorMessages)) { Logger.Log($"Rejected: {request.RequestId}: for analyzer load issues {string.Join(";", errorMessages)}"); return new AnalyzerInconsistencyBuildResponse(new ReadOnlyCollection(errorMessages)); } +#endif Logger.Log($"Begin {request.RequestId} {request.Language} compiler run"); try { + bool utf8output = compiler.Arguments.Utf8Output; TextWriter output = new StringWriter(CultureInfo.InvariantCulture); int returnCode = compiler.Run(output, cancellationToken); var outputString = output.ToString(); diff --git a/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs b/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs index 47344c21f5506..21b42d7b517df 100644 --- a/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs +++ b/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#if NETFRAMEWORK #nullable disable using System; @@ -43,6 +44,39 @@ public AnalyzerConsistencyCheckerTests(ITestOutputHelper testOutputHelper, Assem TestFixture = testFixture; } + private TempFile CreateNetStandardDll(TempDirectory directory, string assemblyName, string version, ImmutableArray publicKey, string? extraSource = null) + { + var source = $$""" + using System; + using System.Reflection; + + [assembly: AssemblyVersion("{{version}}")] + [assembly: AssemblyFileVersion("{{version}}")] + """; + + var sources = extraSource is null + ? new[] { CSharpTestSource.Parse(source) } + : new[] { CSharpTestSource.Parse(source), CSharpTestSource.Parse(extraSource) }; + + var options = new CSharpCompilationOptions( + OutputKind.DynamicallyLinkedLibrary, + warningLevel: Diagnostic.MaxWarningLevel, + cryptoPublicKey: publicKey, + deterministic: true, + publicSign: true); + + var comp = CSharpCompilation.Create( + assemblyName, + sources, + references: NetStandard20.All, + options: options); + + var file = directory.CreateFile($"{assemblyName}.dll"); + var emitResult = comp.Emit(file.Path); + Assert.Empty(emitResult.Diagnostics.Where(x => x.Severity == DiagnosticSeverity.Error)); + return file; + } + [Fact] public void MissingReference() { @@ -73,25 +107,66 @@ public void DifferingMvids() { var directory = Temp.CreateDirectory(); - // Load Beta.dll from the future Alpha.dll path to prime the assembly loader - var alphaDll = directory.CopyFile(TestFixture.Beta.Path, name: "Alpha.dll"); + var key = NetStandard20.netstandard.GetAssemblyIdentity().PublicKey; + var mvidAlpha1 = CreateNetStandardDll(directory.CreateDirectory("mvid1"), "MvidAlpha", "1.0.0.0", key, "class C { }"); + var mvidAlpha2 = CreateNetStandardDll(directory.CreateDirectory("mvid2"), "MvidAlpha", "1.0.0.0", key, "class D { }"); + + // Can't use InMemoryAssemblyLoader because that uses the None context which fakes paths + // to always be the currently executing application. That makes it look like everything + // is in the same directory + var assemblyLoader = new DefaultAnalyzerAssemblyLoader(); + var analyzerReferences = ImmutableArray.Create( + new CommandLineAnalyzerReference(mvidAlpha1.Path), + new CommandLineAnalyzerReference(mvidAlpha2.Path)); - var assemblyLoader = new InMemoryAssemblyLoader(); - var betaAssembly = assemblyLoader.LoadFromPath(alphaDll.Path); + var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger); + Assert.False(result); + } - // now overwrite the {directory}/Alpha.dll file with the content from our Alpha.dll test resource - alphaDll.CopyContentFrom(TestFixture.Alpha.Path); - directory.CopyFile(TestFixture.Gamma.Path); - directory.CopyFile(TestFixture.Delta1.Path); + /// + /// A differing MVID is okay when it's loading a DLL from the compiler directory. That is + /// considered an exchange type. For example if an analyzer has a reference to System.Memory + /// it will always load the copy the compiler used and that is not a consistency issue. + /// + [Fact] + [WorkItem(64826, "https://github.com/dotnet/roslyn/issues/64826")] + public void LoadingLibraryFromCompiler() + { + var directory = Temp.CreateDirectory(); + var dllFile = CreateNetStandardDll(directory, "System.Memory", "2.0.0.0", NetStandard20.netstandard.GetAssemblyIdentity().PublicKey); + // This test must use the DefaultAnalyzerAssemblyLoader as we want assembly binding redirects + // to take affect here. + var assemblyLoader = new DefaultAnalyzerAssemblyLoader(); var analyzerReferences = ImmutableArray.Create( - new CommandLineAnalyzerReference("Alpha.dll"), - new CommandLineAnalyzerReference("Gamma.dll"), - new CommandLineAnalyzerReference("Delta.dll")); + new CommandLineAnalyzerReference("System.Memory.dll")); var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger); - Assert.False(result); + Assert.True(result); + } + + /// + /// A differing MVID is okay when it's loading a DLL from the GAC. There is no reason that + /// falling back to csc would change the load result. + /// + [Fact] + [WorkItem(64826, "https://github.com/dotnet/roslyn/issues/64826")] + public void LoadingLibraryFromGAC() + { + var directory = Temp.CreateDirectory(); + var dllFile = directory.CreateFile("System.Core.dll"); + dllFile.WriteAllBytes(NetStandard20.Resources.SystemCore); + + // This test must use the DefaultAnalyzerAssemblyLoader as we want assembly binding redirects + // to take affect here. + var assemblyLoader = new DefaultAnalyzerAssemblyLoader(); + var analyzerReferences = ImmutableArray.Create( + new CommandLineAnalyzerReference("System.Core.dll")); + + var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger); + + Assert.True(result); } [Fact] @@ -109,22 +184,15 @@ public void AssemblyLoadException() } [Fact] - public void NetstandardIgnored() + public void LoadingSimpleLibrary() { var directory = Temp.CreateDirectory(); - const string name = "netstandardRef"; - var comp = CSharpCompilation.Create( - name, - new[] { CSharpTestSource.Parse("class C {}") }, - references: new MetadataReference[] { NetStandard20.netstandard }, - options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, warningLevel: Diagnostic.MaxWarningLevel)); - var compFile = directory.CreateFile(name); - comp.Emit(compFile.Path); + var key = NetStandard20.netstandard.GetAssemblyIdentity().PublicKey; + var compFile = CreateNetStandardDll(directory, "netstandardRef", "1.0.0.0", key); + var analyzerReferences = ImmutableArray.Create(new CommandLineAnalyzerReference(compFile.Path)); - var analyzerReferences = ImmutableArray.Create(new CommandLineAnalyzerReference(name)); - - var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, new InMemoryAssemblyLoader(), Logger); + var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, new DefaultAnalyzerAssemblyLoader(), Logger); Assert.True(result); } @@ -152,3 +220,4 @@ public Assembly LoadFromPath(string fullPath) } } } +#endif