Use ArtifactsPath in sdk 8+ instead of IntermediateOutputPath.

timcassell · timcassell · commit fb9c47ac5648 · 2024-12-21T20:56:39.000-05:00
Pass PublishDir property to dotnet.
diff --git a/src/BenchmarkDotNet/Templates/WasmCsProj.txt b/src/BenchmarkDotNet/Templates/WasmCsProj.txt
@@ -16,7 +16,7 @@
     <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
     <TargetFramework>$TFM$</TargetFramework>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <AppDir>$(MSBuildThisFileDirectory)\bin\$TFM$\browser-wasm\publish</AppDir>
+    <AppDir>$(PublishDir)</AppDir>
     <AssemblyName>$PROGRAMNAME$</AssemblyName>
     <RuntimeIdentifier>browser-wasm</RuntimeIdentifier>
     <SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
@@ -49,8 +49,8 @@
 
   <Target Name="PrepareForWasmBuild" AfterTargets="Publish">
     <ItemGroup>
-      <WasmAssembliesToBundle Include="$(TargetDir)publish\*.dll" Condition="'$(RunAOTCompilation)' != 'true'" />
-      <WasmAssembliesToBundle Include="$(TargetDir)publish\*.dll" Exclude="$(TargetDir)publish\KernelTraceControl.dll" Condition="'$(RunAOTCompilation)' == 'true'" />
+      <WasmAssembliesToBundle Include="$(PublishDir)*.dll" Condition="'$(RunAOTCompilation)' != 'true'" />
+      <WasmAssembliesToBundle Include="$(PublishDir)*.dll" Exclude="$(PublishDir)KernelTraceControl.dll" Condition="'$(RunAOTCompilation)' == 'true'" />
     </ItemGroup>
   </Target>
 
diff --git a/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs b/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs
@@ -4,10 +4,11 @@ namespace BenchmarkDotNet.Toolchains
 {
     public class ArtifactsPaths
     {
-        public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", "", "");
+        public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", "", "", "");
 
         [PublicAPI] public string RootArtifactsFolderPath { get; }
         [PublicAPI] public string BuildArtifactsDirectoryPath { get; }
+        [PublicAPI] public string PublishDirectoryPath { get; }
         [PublicAPI] public string BinariesDirectoryPath { get; }
         [PublicAPI] public string IntermediateDirectoryPath { get; }
         [PublicAPI] public string ProgramCodePath { get; }
@@ -22,6 +23,7 @@ public class ArtifactsPaths
         public ArtifactsPaths(
             string rootArtifactsFolderPath,
             string buildArtifactsDirectoryPath,
+            string publishDirectoryPath,
             string binariesDirectoryPath,
             string intermediateDirectoryPath,
             string programCodePath,
@@ -35,6 +37,7 @@ public ArtifactsPaths(
         {
             RootArtifactsFolderPath = rootArtifactsFolderPath;
             BuildArtifactsDirectoryPath = buildArtifactsDirectoryPath;
+            PublishDirectoryPath = publishDirectoryPath;
             BinariesDirectoryPath = binariesDirectoryPath;
             IntermediateDirectoryPath = intermediateDirectoryPath;
             ProgramCodePath = programCodePath;
diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs
@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Text;
 using BenchmarkDotNet.Characteristics;
+using BenchmarkDotNet.Environments;
 using BenchmarkDotNet.Extensions;
 using BenchmarkDotNet.Jobs;
 using BenchmarkDotNet.Loggers;
@@ -32,6 +33,9 @@ public class DotNetCliCommand
 
         [PublicAPI] public bool LogOutput { get; }
 
+        // Whether to use ArtifactsPath or IntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+.
+        private readonly bool _useArtifactsPath;
+
         public DotNetCliCommand(string cliPath, string arguments, GenerateResult generateResult, ILogger logger,
             BuildPartition buildPartition, IReadOnlyList<EnvironmentVariable> environmentVariables, TimeSpan timeout, bool logOutput = false)
         {
@@ -43,6 +47,8 @@ public DotNetCliCommand(string cliPath, string arguments, GenerateResult generat
             EnvironmentVariables = environmentVariables;
             Timeout = timeout;
             LogOutput = logOutput || (buildPartition is not null && buildPartition.LogBuildOutput);
+
+            _useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath);
         }
 
         public DotNetCliCommand WithArguments(string arguments)
@@ -71,12 +77,12 @@ public BuildResult RestoreThenBuild()
             if (BuildPartition.ForcedNoDependenciesForIntegrationTests)
             {
                 var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments(
-                    GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
+                    GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
                 if (!restoreResult.IsSuccess)
                     return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
 
                 return DotNetCliCommandExecutor.Execute(WithArguments(
-                    GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
+                    GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
                     .ToBuildResult(GenerateResult);
             }
             else
@@ -130,59 +136,62 @@ public DotNetCliCommandResult AddPackages()
 
         public DotNetCliCommandResult Restore()
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "restore")));
+                GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "restore")));
 
         public DotNetCliCommandResult Build()
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "build")));
+                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "build")));
 
         public DotNetCliCommandResult BuildNoRestore()
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "build-no-restore")));
+                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore", "build-no-restore")));
 
         public DotNetCliCommandResult Publish()
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "publish")));
+                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "publish")));
 
         // PublishNoBuildAndNoRestore was removed because we set --output in the build step. We use the implicit build included in the publish command.
         public DotNetCliCommandResult PublishNoRestore()
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "publish-no-restore")));
+                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore", "publish-no-restore")));
 
         internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
             => GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);
 
-        internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
+        internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
+            bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
             => new StringBuilder()
                 .AppendArgument("restore")
                 .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"--packages \"{artifactsPaths.PackagesDirectoryName}\"")
                 .AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
                 .AppendArgument(extraArguments)
                 .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
                 .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
-                .MaybeAppendOutputPaths(artifactsPaths, true, excludeOutput)
+                .MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath, true, excludeOutput)
                 .ToString();
 
-        internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
+        internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
+            bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
             => new StringBuilder()
                 .AppendArgument($"build -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
                 .AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
                 .AppendArgument(extraArguments)
                 .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
                 .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
                 .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
-                .MaybeAppendOutputPaths(artifactsPaths, excludeOutput: excludeOutput)
+                .MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath, excludeOutput: excludeOutput)
                 .ToString();
 
-        internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null)
+        internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
+            bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null)
             => new StringBuilder()
                 .AppendArgument($"publish -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
                 .AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
                 .AppendArgument(extraArguments)
                 .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
                 .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
                 .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
-                .MaybeAppendOutputPaths(artifactsPaths)
+                .MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath)
                 .ToString();
 
         private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string suffix)
@@ -257,15 +266,22 @@ internal static class DotNetCliCommandExtensions
         // We force the project to output binaries to a new directory.
         // Specifying --output and --no-dependencies breaks the build (because the previous build was not done using the custom output path),
         // so we don't include it if we're building no-deps (only supported for integration tests).
-        internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool isRestore = false, bool excludeOutput = false)
+        internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool useArtifactsPath, bool isRestore = false, bool excludeOutput = false)
             => excludeOutput
                 ? stringBuilder
                 : stringBuilder
                     // Use AltDirectorySeparatorChar so it's not interpreted as an escaped quote `\"`.
-                    .AppendArgument($"/p:IntermediateOutputPath=\"{artifactsPaths.IntermediateDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
+                    .AppendArgument(useArtifactsPath
+                        // We set ArtifactsPath for dotnet sdk 8+, fallback to IntermediateOutputPath for older sdks.
+                        ? $"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}\""
+                        // This is technically incorrect (#2664, #2425), but it's the best we can do for older sdks.
+                        // MSBuild does not support setting BaseIntermediateOutputPath from command line. https://github.com/dotnet/sdk/issues/2003#issuecomment-369408964
+                        : $"/p:IntermediateOutputPath=\"{artifactsPaths.IntermediateDirectoryPath}{Path.AltDirectorySeparatorChar}\""
+                    )
                     .AppendArgument($"/p:OutDir=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
                     // OutputPath is legacy, per-project version of OutDir. We set both just in case. https://github.com/dotnet/msbuild/issues/87
                     .AppendArgument($"/p:OutputPath=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
+                    .AppendArgument($"/p:PublishDir=\"{artifactsPaths.PublishDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
                     .AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"");
     }
 }
diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs
@@ -6,6 +6,7 @@
 using System.Text;
 using System.Text.RegularExpressions;
 using BenchmarkDotNet.Detectors;
+using BenchmarkDotNet.Environments;
 using BenchmarkDotNet.Extensions;
 using BenchmarkDotNet.Helpers;
 using BenchmarkDotNet.Jobs;
@@ -55,9 +56,24 @@ public static DotNetCliCommandResult Execute(DotNetCliCommand parameters)
             }
         }
 
-        internal static string? GetDotNetSdkVersion()
+        internal static bool DotNetSdkSupportsArtifactsPath(string? customDotNetCliPath)
         {
-            using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath: null, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
+            var version = string.IsNullOrEmpty(customDotNetCliPath)
+                ? HostEnvironmentInfo.GetCurrent().DotNetSdkVersion.Value
+                : GetDotNetSdkVersion(customDotNetCliPath);
+            if (string.IsNullOrEmpty(version))
+            {
+                return false;
+            }
+            version = CoreRuntime.GetParsableVersionPart(version);
+            return Version.TryParse(version, out var semVer) && semVer.Major >= 8;
+        }
+
+        internal static string? GetDotNetSdkVersion() => GetDotNetSdkVersion(null);
+
+        internal static string? GetDotNetSdkVersion(string? customDotNetCliPath)
+        {
+            using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
             using (new ConsoleExitHandler(process, NullLogger.Instance))
             {
                 try
diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs
@@ -20,13 +20,18 @@ public abstract class DotNetCliGenerator : GeneratorBase
 
         protected bool IsNetCore { get; }
 
+        // Whether to use ArtifactsPath or IntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+.
+        private protected readonly bool _useArtifactsPath;
+
         [PublicAPI]
         protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore)
         {
             TargetFrameworkMoniker = targetFrameworkMoniker;
             CliPath = cliPath;
             PackagesPath = packagesPath;
             IsNetCore = isNetCore;
+
+            _useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath);
         }
 
         protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe";
@@ -101,8 +106,8 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
         protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
         {
             var content = new StringBuilder(300)
-                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition)}")
-                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition)}")
+                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, _useArtifactsPath)}")
+                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, _useArtifactsPath)}")
                 .ToString();
 
             File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);
diff --git a/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs b/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs
@@ -44,6 +44,13 @@ public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger log
         /// </summary>
         [PublicAPI] protected abstract string GetBuildArtifactsDirectoryPath(BuildPartition assemblyLocation, string programName);
 
+        /// <summary>
+        /// returns a path where the publish directory should be found after the build (usually \publish)
+        /// </summary>
+        [PublicAPI]
+        protected virtual string GetPublishDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
+            => Path.Combine(buildArtifactsDirectoryPath, "publish");
+
         /// <summary>
         /// returns a path where executable should be found after the build (usually \bin)
         /// </summary>
@@ -137,6 +144,7 @@ private ArtifactsPaths GetArtifactsPaths(BuildPartition buildPartition, string r
             return new ArtifactsPaths(
                 rootArtifactsFolderPath: rootArtifactsFolderPath,
                 buildArtifactsDirectoryPath: buildArtifactsDirectoryPath,
+                publishDirectoryPath: GetPublishDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration),
                 binariesDirectoryPath: binariesDirectoryPath,
                 intermediateDirectoryPath: GetIntermediateDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration),
                 programCodePath: Path.Combine(buildArtifactsDirectoryPath, $"{programName}{codeFileExtension}"),
diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs
@@ -11,6 +11,7 @@ public InProcessEmitArtifactsPath(
             ArtifactsPaths baseArtifacts) : base(
             baseArtifacts.RootArtifactsFolderPath,
             baseArtifacts.BuildArtifactsDirectoryPath,
+            baseArtifacts.PublishDirectoryPath,
             baseArtifacts.BinariesDirectoryPath,
             baseArtifacts.IntermediateDirectoryPath,
             baseArtifacts.ProgramCodePath,
diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs
@@ -46,6 +46,7 @@ private ArtifactsPaths GetArtifactsPaths(BuildPartition buildPartition, string r
             return new ArtifactsPaths(
                 rootArtifactsFolderPath: rootArtifactsFolderPath,
                 buildArtifactsDirectoryPath: buildArtifactsDirectoryPath,
+                publishDirectoryPath: null,
                 binariesDirectoryPath: binariesDirectoryPath,
                 intermediateDirectoryPath: null,
                 programCodePath: null,
diff --git a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs
diff --git a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs
