Make MSBuild tasks safe for multi-threaded execution

Directory.Packages.props

@@ -39,6 +39,7 @@
     <PackageVersion Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCoreVersion)" />
     <PackageVersion Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCoreVersion)" />
     <PackageVersion Include="Microsoft.Build" Version="$(MicrosoftBuildVersion)" />
+    <PackageVersion Include="Microsoft.Build.TaskAuthoring.Analyzer" Version="$(MicrosoftBuildTaskAuthoringAnalyzerVersion)" />
     <!-- nuget -->
     <PackageVersion Include="NuGet.Commands" Version="$(NuGetCommandsVersion)" />
     <PackageVersion Include="NuGet.Frameworks" Version="$(NuGetFrameworksVersion)" />

eng/BuildTask.targets

@@ -38,6 +38,19 @@
     <PackageReference Update="Microsoft.Build.Utilities.Core" ExcludeAssets="runtime" />
   </ItemGroup>
 
+  <!--
+    Analyzer that validates tasks opting into MSBuild's multithreaded execution model via
+    [MSBuildMultiThreadableTask]. Scoped to those tasks only (see .editorconfig), so it acts as a
+    regression guard for already-migrated tasks without flagging the ones still to be migrated.
+    Not available when building from source, where the package feeds aren't reachable.
+  -->
+  <ItemGroup Condition="'$(DotNetBuildSourceOnly)' != 'true'">
+    <PackageReference Include="Microsoft.Build.TaskAuthoring.Analyzer" PrivateAssets="all" IncludeAssets="analyzers" />
+    <!-- GlobalAnalyzerConfigFiles is folded into EditorConfigFiles by Microsoft.Managed.Core.targets,
+         which is imported before this file, so contribute to EditorConfigFiles directly. -->
+    <EditorConfigFiles Include="$(RepositoryEngineeringDir)MultiThreadableTaskAnalyzer.globalconfig" />
+  </ItemGroup>
+
   <!-- Publish assets and include them in the package under $(BuildTaskTargetFolder) directory. -->
   <Target Name="_AddBuildOutputToPackage">
     <ItemGroup>

eng/MultiThreadableTaskAnalyzer.globalconfig

@@ -0,0 +1,17 @@
+# Configuration for Microsoft.Build.TaskAuthoring.Analyzer (referenced from eng/BuildTask.targets).
+#
+# Arcade's tasks are being migrated to MSBuild's multithreaded execution model incrementally
+# (see https://github.com/dotnet/arcade/issues/17378). Scoping the analyzer to tasks that have
+# already opted in via [MSBuildMultiThreadableTask] keeps it as a regression guard for migrated
+# tasks without drowning the build in diagnostics for the ones still to be migrated.
+is_global = true
+
+msbuild_task_analyzer.scope = multithreadable_only
+
+# API-shape suggestions (typed path parameters, ITaskItem<T>, constructor injection). Useful when
+# authoring a new task, but not worth churning the existing task surface over, and several of them
+# would be binary-breaking for tasks whose parameters are set from targets across the ecosystem.
+dotnet_diagnostic.MSBuildTask0006.severity = none
+dotnet_diagnostic.MSBuildTask0007.severity = none
+dotnet_diagnostic.MSBuildTask0008.severity = none
+dotnet_diagnostic.MSBuildTask0011.severity = none

eng/Version.Details.props

@@ -20,10 +20,11 @@ This file should be imported by eng/Versions.props
     <!-- dotnet-dnceng dependencies -->
     <MicrosoftDncEngSecretManagerPackageVersion>1.1.0-beta.26407.1</MicrosoftDncEngSecretManagerPackageVersion>
     <!-- dotnet-msbuild dependencies -->
-    <MicrosoftBuildPackageVersion>17.12.50</MicrosoftBuildPackageVersion>
-    <MicrosoftBuildFrameworkPackageVersion>17.12.50</MicrosoftBuildFrameworkPackageVersion>
-    <MicrosoftBuildTasksCorePackageVersion>17.12.50</MicrosoftBuildTasksCorePackageVersion>
-    <MicrosoftBuildUtilitiesCorePackageVersion>17.12.50</MicrosoftBuildUtilitiesCorePackageVersion>
+    <MicrosoftBuildPackageVersion>18.8.2</MicrosoftBuildPackageVersion>
+    <MicrosoftBuildFrameworkPackageVersion>18.8.2</MicrosoftBuildFrameworkPackageVersion>
+    <MicrosoftBuildTasksCorePackageVersion>18.8.2</MicrosoftBuildTasksCorePackageVersion>
+    <MicrosoftBuildUtilitiesCorePackageVersion>18.8.2</MicrosoftBuildUtilitiesCorePackageVersion>
+    <MicrosoftBuildTaskAuthoringAnalyzerPackageVersion>18.11.0-1.26420.118</MicrosoftBuildTaskAuthoringAnalyzerPackageVersion>
     <!-- dotnet-roslyn dependencies -->
     <MicrosoftCodeAnalysisCSharpPackageVersion>4.8.0</MicrosoftCodeAnalysisCSharpPackageVersion>
     <MicrosoftNetCompilersToolsetPackageVersion>4.8.0</MicrosoftNetCompilersToolsetPackageVersion>
@@ -81,6 +82,7 @@ This file should be imported by eng/Versions.props
     <MicrosoftBuildFrameworkVersion>$(MicrosoftBuildFrameworkPackageVersion)</MicrosoftBuildFrameworkVersion>
     <MicrosoftBuildTasksCoreVersion>$(MicrosoftBuildTasksCorePackageVersion)</MicrosoftBuildTasksCoreVersion>
     <MicrosoftBuildUtilitiesCoreVersion>$(MicrosoftBuildUtilitiesCorePackageVersion)</MicrosoftBuildUtilitiesCoreVersion>
+    <MicrosoftBuildTaskAuthoringAnalyzerVersion>$(MicrosoftBuildTaskAuthoringAnalyzerPackageVersion)</MicrosoftBuildTaskAuthoringAnalyzerVersion>
     <!-- dotnet-roslyn dependencies -->
     <MicrosoftCodeAnalysisCSharpVersion>$(MicrosoftCodeAnalysisCSharpPackageVersion)</MicrosoftCodeAnalysisCSharpVersion>
     <MicrosoftNetCompilersToolsetVersion>$(MicrosoftNetCompilersToolsetPackageVersion)</MicrosoftNetCompilersToolsetVersion>

eng/Version.Details.xml

@@ -129,21 +129,25 @@
       <Sha>eb583d0664328fb0d370586c4d4e974212687cb0</Sha>
     </Dependency>
     <!-- Dependencies required for source build to lift to the previously-source-built version. -->
-    <Dependency Name="Microsoft.Build" Version="17.12.50">
+    <Dependency Name="Microsoft.Build" Version="18.8.2">
       <Uri>https://github.com/dotnet/msbuild</Uri>
-      <Sha>d1cce8d7cc03c23a4f1bad8e9240714fd9d199a3</Sha>
+      <Sha>ce25c01082c9c46cd02ad1ff3ff8f16fe5cc2f44</Sha>
     </Dependency>
-    <Dependency Name="Microsoft.Build.Framework" Version="17.12.50">
+    <Dependency Name="Microsoft.Build.Framework" Version="18.8.2">
       <Uri>https://github.com/dotnet/msbuild</Uri>
-      <Sha>d1cce8d7cc03c23a4f1bad8e9240714fd9d199a3</Sha>
+      <Sha>ce25c01082c9c46cd02ad1ff3ff8f16fe5cc2f44</Sha>
     </Dependency>
-    <Dependency Name="Microsoft.Build.Tasks.Core" Version="17.12.50">
+    <Dependency Name="Microsoft.Build.Tasks.Core" Version="18.8.2">
       <Uri>https://github.com/dotnet/msbuild</Uri>
-      <Sha>d1cce8d7cc03c23a4f1bad8e9240714fd9d199a3</Sha>
+      <Sha>ce25c01082c9c46cd02ad1ff3ff8f16fe5cc2f44</Sha>
     </Dependency>
-    <Dependency Name="Microsoft.Build.Utilities.Core" Version="17.12.50">
+    <Dependency Name="Microsoft.Build.Utilities.Core" Version="18.8.2">
       <Uri>https://github.com/dotnet/msbuild</Uri>
-      <Sha>d1cce8d7cc03c23a4f1bad8e9240714fd9d199a3</Sha>
+      <Sha>ce25c01082c9c46cd02ad1ff3ff8f16fe5cc2f44</Sha>
+    </Dependency>
+    <Dependency Name="Microsoft.Build.TaskAuthoring.Analyzer" Version="18.11.0-1.26420.118">
+      <Uri>https://github.com/dotnet/msbuild</Uri>
+      <Sha>ce25c01082c9c46cd02ad1ff3ff8f16fe5cc2f44</Sha>
     </Dependency>
     <!-- NuGet dependencies required for source build to lift to the previously-source-built version.
          Arcade redistributes these in its MSBuild task packages and needs real executable assemblies,

src/Microsoft.DotNet.Arcade.Sdk/src/CalculateAssemblyAndFileVersions.cs

@@ -12,6 +12,7 @@ namespace Microsoft.DotNet.Arcade.Sdk
     /// File version has 4 parts and need to increase every official build. This is especially important when building MSIs.
     /// See https://github.com/dotnet/arcade/blob/master/Documentation/CorePackages/Versioning.md#assembly-version.
     /// </summary>
+    [MSBuildMultiThreadableTask]
     public class CalculateAssemblyAndFileVersions : Microsoft.Build.Utilities.Task
     {
         private const int MaxMinor = 654;

src/Microsoft.DotNet.Arcade.Sdk/src/CheckRequiredDotNetVersion.cs

@@ -11,22 +11,30 @@
 
 namespace Microsoft.DotNet.Arcade.Sdk
 {
-    public class CheckRequiredDotNetVersion : Microsoft.Build.Utilities.Task
+    [MSBuildMultiThreadableTask]
+    public class CheckRequiredDotNetVersion : Microsoft.Build.Utilities.Task, IMultiThreadableTask
     {
         private static readonly string s_cacheKey = "CheckRequiredDotNetVersion-6ED0A075-A4B3-46B1-97D4-448558D515D3";
 
         private sealed class CacheEntry
         {
+            public readonly string GlobalJsonPath;
+            public readonly string SdkVersion;
             public readonly DateTime LastWrite;
             public readonly bool Success;
 
-            public CacheEntry(DateTime lastWrite, bool success)
+            public CacheEntry(string globalJsonPath, string sdkVersion, DateTime lastWrite, bool success)
             {
+                GlobalJsonPath = globalJsonPath;
+                SdkVersion = sdkVersion;
                 LastWrite = lastWrite;
                 Success = success;
             }
         }
 
+        /// <summary>Injected by MSBuild so paths resolve against the project directory in multithreaded builds.</summary>
+        public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
+
         [Required]
         public string RepositoryRoot { get; set; }
 
@@ -41,7 +49,7 @@ public override bool Execute()
                 return false;
             }
 
-            var globalJsonPath = Path.Combine(RepositoryRoot, "global.json");
+            var globalJsonPath = TaskEnvironment.GetAbsolutePath(Path.Combine(RepositoryRoot, "global.json"));
             DateTime lastWrite;
             try
             {
@@ -53,8 +61,19 @@ public override bool Execute()
                 return false;
             }
 
+            // The cache is registered per build, not per project, so the repository and the SDK
+            // version being validated have to be part of the entry. Otherwise a different repository
+            // or a different required version could reuse this result.
+            //
+            // The read/write pair below is not atomic, so under multithreaded execution two threads
+            // can both miss and both run the check. The check itself is pure, so the result is
+            // identical either way; the only observable effect is that a failing check can log its
+            // error twice, since deduplicating that reporting is part of what the cache buys.
             var cachedResult = (CacheEntry)BuildEngine4.GetRegisteredTaskObject(s_cacheKey, RegisteredTaskObjectLifetime.Build);
-            if (cachedResult != null && lastWrite == cachedResult.LastWrite)
+            if (cachedResult != null &&
+                string.Equals(globalJsonPath.Value, cachedResult.GlobalJsonPath, StringComparison.OrdinalIgnoreCase) &&
+                string.Equals(SdkVersion, cachedResult.SdkVersion, StringComparison.Ordinal) &&
+                lastWrite == cachedResult.LastWrite)
             {
                 // Error has already been reported if the current SDK version is not sufficient.
                 if (!cachedResult.Success)
@@ -103,7 +122,7 @@ bool execute()
             }
 
             bool success = execute();
-            BuildEngine4.RegisterTaskObject(s_cacheKey, new CacheEntry(lastWrite, success), RegisteredTaskObjectLifetime.Build, allowEarlyCollection: true);
+            BuildEngine4.RegisterTaskObject(s_cacheKey, new CacheEntry(globalJsonPath.Value, SdkVersion, lastWrite, success), RegisteredTaskObjectLifetime.Build, allowEarlyCollection: true);
             return success;
         }
     }

src/Microsoft.DotNet.Arcade.Sdk/src/CompareVersions.cs

@@ -7,6 +7,7 @@
 
 namespace Microsoft.DotNet.Arcade.Sdk
 {
+    [MSBuildMultiThreadableTask]
     public class CompareVersions : Microsoft.Build.Utilities.Task
     {
         [Required]

src/Microsoft.DotNet.Arcade.Sdk/src/DownloadFile.cs

@@ -14,8 +14,12 @@
 
 namespace Microsoft.DotNet.Arcade.Sdk
 {
-    public class DownloadFile : Microsoft.Build.Utilities.Task, ICancelableTask
+    [MSBuildMultiThreadableTask]
+    public class DownloadFile : Microsoft.Build.Utilities.Task, ICancelableTask, IMultiThreadableTask
     {
+        /// <summary>Injected by MSBuild so paths resolve against the project directory in multithreaded builds.</summary>
+        public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
+
         /// <summary>
         /// List of URls to attempt download from. Accepted metadata are:
         ///     - Token: Base64 encoded token to be appended to base URL for accessing private locations.
@@ -59,7 +63,7 @@ public override bool Execute()
                 return false;
             }
 
-            if (File.Exists(DestinationPath) && !Overwrite)
+            if (File.Exists(TaskEnvironment.GetAbsolutePath(DestinationPath)) && !Overwrite)
             {
                 return true;
             }
@@ -69,7 +73,7 @@ public override bool Execute()
                 return false;
             }
 
-            Directory.CreateDirectory(Path.GetDirectoryName(DestinationPath));
+            Directory.CreateDirectory(TaskEnvironment.GetAbsolutePath(Path.GetDirectoryName(DestinationPath)));
 
             if (!string.IsNullOrWhiteSpace(Uri)) {
                 return DownloadFromUriAsync(Uri).Result;
@@ -112,9 +116,9 @@ private async Tasks.Task<bool> DownloadFromUriAsync(string uri) {
             {
                 var filePath = uri.Substring(FileUriProtocol.Length);
 
-                if (File.Exists(filePath)) {
+                if (File.Exists(TaskEnvironment.GetAbsolutePath(filePath))) {
                     Log.LogMessage($"Copying '{filePath}' to '{DestinationPath}'");
-                    File.Copy(filePath, DestinationPath, overwrite: true);
+                    File.Copy(TaskEnvironment.GetAbsolutePath(filePath), TaskEnvironment.GetAbsolutePath(DestinationPath), overwrite: true);
                     return true;
                 } else {
                     Log.LogMessage($"'{filePath}' does not exist.");
@@ -191,7 +195,7 @@ private async Tasks.Task<bool> DownloadWithRetriesAsync(HttpClient httpClient, s
 
                     httpResponse.EnsureSuccessStatusCode();
 
-                    using (var outStream = File.Create(DestinationPath))
+                    using (var outStream = File.Create(TaskEnvironment.GetAbsolutePath(DestinationPath)))
                     {
                         await httpResponse.Content.CopyToAsync(outStream).ConfigureAwait(false);
                     }