From 67caa206619e33ddd0360be53d7367388f2e2f7b Mon Sep 17 00:00:00 2001 From: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:14:59 +0200 Subject: [PATCH 1/2] Migrate GenAPI and GenFacades tasks to the multithreaded task model Annotates ClearAssemblyReferenceVersions with [MSBuildMultiThreadableTask] and routes path handling in both assemblies through the injected TaskEnvironment. AbsolutePath is threaded through SourceGenerator and TypeParser, which is what resolves the transitive MSBuildTask0005 chain out of GenPartialFacadeSource. GenAPITask, GenPartialFacadeSource and NotSupportedAssemblyGenerator keep IMultiThreadableTask without the attribute, so they still route to the TaskHost while their paths resolve against the project. Each records why at its declaration: GenAPITask hands raw paths to HostEnvironment, which expands and probes them with process-wide APIs, and the two RoslynBuildTask subclasses subscribe an instance handler to the process-wide AssemblyLoadContext.Resolving event. Two issues found while auditing the migration itself: - GenPartialFacadeSourceGenerator deduplicated seeds with Path.GetFullPath, which both anchors and canonicalizes. GetAbsolutePath only anchors, so Distinct no longer collapsed two spellings of the same seed and the duplicate-name check below it would report them as multiple versions of one assembly. Restored via the GetCanonicalForm polyfill. - Empty compile file item specs were skipped by TypeParser.GetSourceTrees rather than treated as an error. Absolutizing every item spec turned that into a throw, so they are filtered before resolution. --- src/Microsoft.DotNet.GenAPI/GenAPITask.cs | 26 ++++++++---- .../ClearAssemblyReferenceVersions.cs | 8 +++- .../GenPartialFacadeSource.cs | 41 ++++++++++++++++--- .../GenPartialFacadeSourceGenerator.cs | 25 ++++++----- .../Microsoft.DotNet.GenFacades.csproj | 4 ++ .../NotSupportedAssemblyGenerator.cs | 39 ++++++++++++++---- .../SourceGenerator.cs | 7 ++-- src/Microsoft.DotNet.GenFacades/TypeParser.cs | 9 ++-- 8 files changed, 119 insertions(+), 40 deletions(-) diff --git a/src/Microsoft.DotNet.GenAPI/GenAPITask.cs b/src/Microsoft.DotNet.GenAPI/GenAPITask.cs index 20b7b95157e..8d947fda184 100644 --- a/src/Microsoft.DotNet.GenAPI/GenAPITask.cs +++ b/src/Microsoft.DotNet.GenAPI/GenAPITask.cs @@ -19,8 +19,15 @@ namespace Microsoft.DotNet.GenAPI; -public class GenAPITask : Task +// Deliberately not marked multithreadable: HostEnvironment resolves the raw LibPath and +// Assembly values below through Environment.ExpandEnvironmentVariables plus Directory.Exists/ +// File.Exists (Microsoft.Cci.Extensions/HostEnvironment.cs:719-740), so relative inputs and +// per-project variables would bind to process-wide state in a shared node. Migrating requires +// expanding and resolving those paths through TaskEnvironment before they enter HostEnvironment. +#pragma warning disable MSBuildTask0013 // Interface without the attribute is deliberate; see the comment above. +public class GenAPITask : Task, IMultiThreadableTask { +#pragma warning restore MSBuildTask0013 private const string InternalsVisibleTypeName = "System.Runtime.CompilerServices.InternalsVisibleToAttribute"; private const string DefaultFileHeader = "//------------------------------------------------------------------------------\r\n" + @@ -37,6 +44,9 @@ public class GenAPITask : Task private SyntaxWriterType _syntaxWriterType; private DocIdKinds _docIdKinds = Cci.Writers.DocIdKinds.All; + /// Injected by MSBuild so paths resolve against the project directory in multithreaded builds. + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + /// /// Path for an specific assembly or a directory to get all assemblies. /// @@ -197,7 +207,7 @@ public override bool Execute() } string headerText = GetHeaderText(HeaderFile, _writerType, _syntaxWriterType); - bool loopPerAssembly = Directory.Exists(OutputPath); + bool loopPerAssembly = !string.IsNullOrEmpty(OutputPath) && Directory.Exists(TaskEnvironment.GetAbsolutePath(OutputPath)); if (loopPerAssembly) { @@ -260,11 +270,11 @@ public override bool Execute() return !Log.HasLoggedErrors; } - private static string GetHeaderText(string headerFile, WriterType writerType, SyntaxWriterType syntaxWriterType) + private string GetHeaderText(string headerFile, WriterType writerType, SyntaxWriterType syntaxWriterType) { if (!string.IsNullOrEmpty(headerFile)) { - return File.ReadAllText(headerFile); + return File.ReadAllText(TaskEnvironment.GetAbsolutePath(headerFile)); } string defaultHeader = string.Empty; @@ -286,12 +296,14 @@ private TextWriter GetOutput(string outFilePath, string filename = "") if (string.IsNullOrWhiteSpace(outFilePath)) return new LogTextWriter(Log); - if (Directory.Exists(outFilePath) && !string.IsNullOrEmpty(filename)) + AbsolutePath outputPath = TaskEnvironment.GetAbsolutePath(outFilePath); + + if (Directory.Exists(outputPath) && !string.IsNullOrEmpty(filename)) { - return File.CreateText(Path.Combine(outFilePath, filename)); + return File.CreateText(Path.Combine(outputPath, filename)); } - return File.CreateText(outFilePath); + return File.CreateText(outputPath); } /// diff --git a/src/Microsoft.DotNet.GenFacades/ClearAssemblyReferenceVersions.cs b/src/Microsoft.DotNet.GenFacades/ClearAssemblyReferenceVersions.cs index 64f6af191f7..e20693cd028 100644 --- a/src/Microsoft.DotNet.GenFacades/ClearAssemblyReferenceVersions.cs +++ b/src/Microsoft.DotNet.GenFacades/ClearAssemblyReferenceVersions.cs @@ -14,8 +14,12 @@ namespace Microsoft.DotNet.GenFacades; /// /// Rewrites an Assembly's references to be version 0.0.0.0. /// -public class ClearAssemblyReferenceVersions : Task +[MSBuildMultiThreadableTask] +public class ClearAssemblyReferenceVersions : Task, IMultiThreadableTask { + /// Injected by MSBuild so paths resolve against the project directory in multithreaded builds. + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + /// /// Assembly to rewrite. /// @@ -26,7 +30,7 @@ public override bool Execute() { try { - using (FileStream stream = File.Open(Assembly, FileMode.Open, FileAccess.ReadWrite, FileShare.Read)) + using (FileStream stream = File.Open(TaskEnvironment.GetAbsolutePath(Assembly), FileMode.Open, FileAccess.ReadWrite, FileShare.Read)) using (PEReader peReader = new PEReader(stream)) { using (BinaryWriter writer = new BinaryWriter(stream)) diff --git a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSource.cs b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSource.cs index c9a7847a057..9c673268fd7 100644 --- a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSource.cs +++ b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSource.cs @@ -9,8 +9,24 @@ namespace Microsoft.DotNet.GenFacades; -public class GenPartialFacadeSource : RoslynBuildTask +// TODO: Not opted into multithreading. RoslynBuildTask.Execute subscribes every instance to the +// process-wide AssemblyLoadContext.Resolving event, so with differing RoslynAssembliesPath values +// one instance can satisfy another instance's resolution. The TaskEnvironment below is still used +// for path resolution. Tracked by https://github.com/dotnet/arcade/issues/17378. +// +// Implementing IMultiThreadableTask without the attribute is deliberate. Routing is decided by +// the attribute alone (TaskRouter.NeedsTaskHostInMultiThreadedMode); it cannot key off the +// interface, because ToolTask implements it and that would opt in every ToolTask-derived task in +// the ecosystem. The interface only causes TaskEnvironment to be injected. Do not remove it to +// "make this safe" - that would revert the path resolution below to the process current +// directory while leaving the task exactly as unsafe as it is now. +#pragma warning disable MSBuildTask0013 // Interface without the attribute is deliberate; see the comment above. +public class GenPartialFacadeSource : RoslynBuildTask, IMultiThreadableTask { +#pragma warning restore MSBuildTask0013 + /// Injected by MSBuild so paths resolve against the project directory in multithreaded builds. + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + [Required] public ITaskItem[] ReferencePaths { get; set; } @@ -39,13 +55,18 @@ public override bool ExecuteCore() bool result = true; try { + AbsolutePath[] referencePaths = GetAbsolutePaths(ReferencePaths); + AbsolutePath referenceAssembly = TaskEnvironment.GetAbsolutePath(ReferenceAssembly); + AbsolutePath[] compileFiles = GetAbsolutePaths(CompileFiles); + AbsolutePath outputSourcePath = TaskEnvironment.GetAbsolutePath(OutputSourcePath); + result = GenPartialFacadeSourceGenerator.Execute( - ReferencePaths?.Select(item => item.ItemSpec).ToArray(), - ReferenceAssembly, - CompileFiles?.Select(item => item.ItemSpec).ToArray(), + referencePaths, + referenceAssembly, + compileFiles, DefineConstants, LangVersion, - OutputSourcePath, + outputSourcePath, Log, IgnoreMissingTypes, IgnoreMissingTypesList, @@ -59,4 +80,14 @@ public override bool ExecuteCore() return result && !Log.HasLoggedErrors; } + + private AbsolutePath[] GetAbsolutePaths(ITaskItem[] items) + { + // Empty item specs were previously skipped by TypeParser.GetSourceTrees rather than + // treated as an error, and GetAbsolutePath throws on an empty path, so filter them here. + return items? + .Where(item => !string.IsNullOrEmpty(item.ItemSpec)) + .Select(item => TaskEnvironment.GetAbsolutePath(item.ItemSpec)) + .ToArray(); + } } diff --git a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs index dfe1da24f83..191350a5cf9 100644 --- a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs +++ b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs @@ -16,12 +16,12 @@ namespace Microsoft.DotNet.GenFacades; public class GenPartialFacadeSourceGenerator { public static bool Execute( - string[] seeds, - string contractAssembly, - string[] compileFiles, + AbsolutePath[] seeds, + AbsolutePath contractAssembly, + AbsolutePath[] compileFiles, string defineConstants, string langVersion, - string outputSourcePath, + AbsolutePath outputSourcePath, TaskLoggingHelper logger, bool ignoreMissingTypes = false, string[] ignoreMissingTypesList = null, @@ -32,9 +32,12 @@ public static bool Execute( IEnumerable referenceTypes = GetPublicVisibleTypes(contractAssembly, includeTypeForwards: true); - // Normalizing and Removing Relative Segments from the seed paths. - string[] distinctSeeds = seeds.Select(seed => Path.GetFullPath(seed)).Distinct().ToArray(); - string[] seedNames = distinctSeeds.Select(seed => Path.GetFileName(seed)).ToArray(); + // Normalizing and Removing Relative Segments from the seed paths. GetAbsolutePath only + // anchors, so the canonical form is what makes Distinct below collapse two spellings of + // the same seed; otherwise the duplicate-name check underneath reports them as two + // different versions of one assembly. + AbsolutePath[] distinctSeeds = seeds.Select(seed => seed.GetCanonicalForm()).Distinct().ToArray(); + string[] seedNames = distinctSeeds.Select(seed => Path.GetFileName(seed.Value)).ToArray(); if (distinctSeeds.Count() != seedNames.Distinct(StringComparer.InvariantCultureIgnoreCase).Count()) { @@ -92,7 +95,7 @@ private static Dictionary ParseSeedTypePreferences(ITaskItem[] p return dictionary; } - private static IEnumerable GetPublicVisibleTypes(string assembly, bool includeTypeForwards = false) + private static IEnumerable GetPublicVisibleTypes(AbsolutePath assembly, bool includeTypeForwards = false) { using (var peReader = new PEReader(new FileStream(assembly, FileMode.Open, FileAccess.Read, FileShare.Delete | FileShare.Read))) { @@ -139,15 +142,15 @@ private static bool IsPublic(TypeDefinition typeDefination) return (typeDefination.Attributes & TypeAttributes.Public) != 0; } - private static IReadOnlyDictionary> GenerateTypeTable(IEnumerable seedAssemblies) + private static IReadOnlyDictionary> GenerateTypeTable(IEnumerable seedAssemblies) { var typeTable = new Dictionary>(); - foreach(string assembly in seedAssemblies) + foreach(AbsolutePath assembly in seedAssemblies) { IEnumerable types = GetPublicVisibleTypes(assembly); foreach (string type in types) { - AddTypeToTable(typeTable, type, Path.GetFileName(assembly)); + AddTypeToTable(typeTable, type, Path.GetFileName(assembly.Value)); } } return typeTable; diff --git a/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj b/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj index 334ddeefb28..f8f12397d3b 100644 --- a/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj +++ b/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj @@ -7,6 +7,10 @@ true + + + + diff --git a/src/Microsoft.DotNet.GenFacades/NotSupportedAssemblyGenerator.cs b/src/Microsoft.DotNet.GenFacades/NotSupportedAssemblyGenerator.cs index 57bea594708..550631929a0 100644 --- a/src/Microsoft.DotNet.GenFacades/NotSupportedAssemblyGenerator.cs +++ b/src/Microsoft.DotNet.GenFacades/NotSupportedAssemblyGenerator.cs @@ -16,8 +16,26 @@ namespace Microsoft.DotNet.GenFacades; /// /// The class generates an NotSupportedAssembly from the reference sources. /// -public class NotSupportedAssemblyGenerator : RoslynBuildTask +/// +/// TODO: Not opted into multithreading. RoslynBuildTask.Execute subscribes every instance to the +/// process-wide AssemblyLoadContext.Resolving event, so with differing RoslynAssembliesPath values +/// one instance can satisfy another instance's resolution. The TaskEnvironment below is still used +/// for path resolution. Tracked by https://github.com/dotnet/arcade/issues/17378. +/// +/// Implementing IMultiThreadableTask without the attribute is deliberate. Routing is decided by +/// the attribute alone (TaskRouter.NeedsTaskHostInMultiThreadedMode); it cannot key off the +/// interface, because ToolTask implements it and that would opt in every ToolTask-derived task in +/// the ecosystem. The interface only causes TaskEnvironment to be injected. Do not remove it to +/// "make this safe" - that would revert the path resolution below to the process current +/// directory while leaving the task exactly as unsafe as it is now. +/// +#pragma warning disable MSBuildTask0013 // Interface without the attribute is deliberate; see the comment above. +public class NotSupportedAssemblyGenerator : RoslynBuildTask, IMultiThreadableTask { +#pragma warning restore MSBuildTask0013 + /// Injected by MSBuild so paths resolve against the project directory in multithreaded builds. + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + [Required] public ITaskItem[] SourceFiles { get; set; } @@ -44,27 +62,32 @@ public override bool ExecuteCore() private void GenerateNotSupportedAssemblyFiles(IEnumerable sourceFiles) { string[] apiExclusions = null; - if (!string.IsNullOrEmpty(ApiExclusionListPath) && File.Exists(ApiExclusionListPath)) + if (!string.IsNullOrEmpty(ApiExclusionListPath)) { - apiExclusions = File.ReadAllLines(ApiExclusionListPath); + AbsolutePath apiExclusionListPath = TaskEnvironment.GetAbsolutePath(ApiExclusionListPath); + if (File.Exists(apiExclusionListPath)) + { + apiExclusions = File.ReadAllLines(apiExclusionListPath); + } } foreach (ITaskItem item in sourceFiles) { string sourceFile = item.ItemSpec; string outputPath = item.GetMetadata("OutputPath"); + AbsolutePath sourceFilePath = TaskEnvironment.GetAbsolutePath(sourceFile); - if (!File.Exists(sourceFile)) + if (!File.Exists(sourceFilePath)) { Log.LogError($"File {sourceFile} was not found."); continue; } - GenerateNotSupportedAssemblyForSourceFile(sourceFile, outputPath, apiExclusions); + GenerateNotSupportedAssemblyForSourceFile(sourceFilePath, outputPath, apiExclusions); } } - private void GenerateNotSupportedAssemblyForSourceFile(string sourceFile, string outputPath, string[] apiExclusions) + private void GenerateNotSupportedAssemblyForSourceFile(AbsolutePath sourceFilePath, string outputPath, string[] apiExclusions) { SyntaxTree syntaxTree; @@ -76,7 +99,7 @@ private void GenerateNotSupportedAssemblyForSourceFile(string sourceFile, string Log.LogError($"Invalid LangVersion value '{LangVersion}'"); return; } - syntaxTree = CSharpSyntaxTree.ParseText(File.ReadAllText(sourceFile), new CSharpParseOptions(languageVersion)); + syntaxTree = CSharpSyntaxTree.ParseText(File.ReadAllText(sourceFilePath), new CSharpParseOptions(languageVersion)); } catch(Exception ex) { @@ -87,7 +110,7 @@ private void GenerateNotSupportedAssemblyForSourceFile(string sourceFile, string var rewriter = new NotSupportedAssemblyRewriter(Message, apiExclusions); SyntaxNode root = rewriter.Visit(syntaxTree.GetRoot()); string text = root.GetText().ToString(); - File.WriteAllText(outputPath, text); + File.WriteAllText(TaskEnvironment.GetAbsolutePath(outputPath), text); } } diff --git a/src/Microsoft.DotNet.GenFacades/SourceGenerator.cs b/src/Microsoft.DotNet.GenFacades/SourceGenerator.cs index c4b7e9703cc..7a658b6c901 100644 --- a/src/Microsoft.DotNet.GenFacades/SourceGenerator.cs +++ b/src/Microsoft.DotNet.GenFacades/SourceGenerator.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using System.Collections.Generic; using System.IO; @@ -14,7 +15,7 @@ internal class SourceGenerator private readonly IReadOnlyDictionary _seedTypePreferences; private readonly IEnumerable _referenceTypes; private readonly IReadOnlyDictionary> _seedTypes; - private readonly string _outputSourcePath; + private readonly AbsolutePath _outputSourcePath; private readonly HashSet _ignoreMissingTypesList = new HashSet(); private readonly TaskLoggingHelper _logger; @@ -22,7 +23,7 @@ public SourceGenerator( IEnumerable referenceTypes, IReadOnlyDictionary> seedTypes, IReadOnlyDictionary seedTypePreferences, - string outputSourcePath, + AbsolutePath outputSourcePath, string[] ignoreMissingTypesList, TaskLoggingHelper logger ) @@ -38,7 +39,7 @@ TaskLoggingHelper logger } public bool GenerateSource( - IEnumerable compileFiles, + IEnumerable compileFiles, IEnumerable constants, string langVersion, bool ignoreMissingTypes) diff --git a/src/Microsoft.DotNet.GenFacades/TypeParser.cs b/src/Microsoft.DotNet.GenFacades/TypeParser.cs index 86186fa6a25..f575b1d1cb3 100644 --- a/src/Microsoft.DotNet.GenFacades/TypeParser.cs +++ b/src/Microsoft.DotNet.GenFacades/TypeParser.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Build.Framework; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -13,7 +14,7 @@ namespace Microsoft.DotNet.GenFacades; internal class TypeParser { - public static HashSet GetAllPublicTypes(IEnumerable files, IEnumerable constants, string langVersion) + public static HashSet GetAllPublicTypes(IEnumerable files, IEnumerable constants, string langVersion) { HashSet types = new HashSet(); @@ -130,13 +131,13 @@ private static string GetNamespaceName(NamespaceDeclarationSyntax namespaceSynta return namespaceSyntax.Name.ToFullString().Trim(); } - private static IEnumerable GetSourceTrees(IEnumerable sourceFiles, IEnumerable constants, LanguageVersion languageVersion) + private static IEnumerable GetSourceTrees(IEnumerable sourceFiles, IEnumerable constants, LanguageVersion languageVersion) { CSharpParseOptions options = new CSharpParseOptions(languageVersion: languageVersion, preprocessorSymbols: constants); List result = new List(); - foreach (string sourceFile in sourceFiles) + foreach (AbsolutePath sourceFile in sourceFiles) { - if (string.IsNullOrEmpty(sourceFile)) + if (string.IsNullOrEmpty(sourceFile.Value)) { continue; } From 415147d03697e7bcbc7a2761bfb0c115023dbe02 Mon Sep 17 00:00:00 2001 From: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:16:54 +0200 Subject: [PATCH 2/2] Make GenPartialFacadeSourceGenerator internal The type is only reachable from GenPartialFacadeSource in the same assembly, and build task packages ship their assembly under tools/ with IncludeBuildOutput=false, so a PackageReference exposes no compile-time assets to bind against. Marking it internal matches SourceGenerator and TypeParser and keeps the AbsolutePath signature change off the public surface. --- .../GenPartialFacadeSourceGenerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs index 191350a5cf9..67c699291b4 100644 --- a/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs +++ b/src/Microsoft.DotNet.GenFacades/GenPartialFacadeSourceGenerator.cs @@ -13,7 +13,7 @@ namespace Microsoft.DotNet.GenFacades; -public class GenPartialFacadeSourceGenerator +internal class GenPartialFacadeSourceGenerator { public static bool Execute( AbsolutePath[] seeds,