From 8c489ea757bbbe8eeca1a81790e1da924adc4960 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 15 Apr 2024 18:14:32 -0400 Subject: [PATCH] Use ArtifactsPath in sdk 8+ instead of IntermediateOutputPath. --- .../Toolchains/DotNetCli/DotNetCliCommand.cs | 50 +++++++++++++------ .../DotNetCli/DotNetCliCommandExecutor.cs | 20 +++++++- .../DotNetCli/DotNetCliGenerator.cs | 11 +++- .../Toolchains/NativeAot/Generator.cs | 5 +- 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs index e9078f293f..5128ab51db 100644 --- 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,11 @@ public class DotNetCliCommand [PublicAPI] public bool LogOutput { get; } + // Whether to use ArtifactsPath or BaseIntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+. + // We set BaseIntermediateOutputPath for older sdks. We can't clean up those artifacts, though, because it's relative to each project. + // That's why we use ArtifactsPath with the newer sdks so that we can clean it up after the benchmark is complete. + private readonly bool _useArtifactsPath; + public DotNetCliCommand(string cliPath, string arguments, GenerateResult generateResult, ILogger logger, BuildPartition buildPartition, IReadOnlyList environmentVariables, TimeSpan timeout, bool logOutput = false) { @@ -43,6 +49,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 +79,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,29 +138,30 @@ 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 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}\"") @@ -160,10 +169,11 @@ internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPar .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)) @@ -171,10 +181,11 @@ internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildParti .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)) @@ -182,7 +193,7 @@ internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPar .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 +268,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(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{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 index 5cbff89b47..2c813a4b5c 100644 --- 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 index a432fbf881..5bd8aab21c 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs @@ -20,6 +20,11 @@ public abstract class DotNetCliGenerator : GeneratorBase protected bool IsNetCore { get; } + // Whether to use ArtifactsPath or BaseIntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+. + // We set BaseIntermediateOutputPath for older sdks. We can't clean up those artifacts, though, because it's relative to each project. + // That's why we use ArtifactsPath with the newer sdks so that we can clean it up after the benchmark is complete. + private readonly bool _useArtifactsPath; + [PublicAPI] protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore) { @@ -27,6 +32,8 @@ protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, stri CliPath = cliPath; PackagesPath = packagesPath; IsNetCore = isNetCore; + + _useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath); } protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe"; @@ -101,8 +108,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/NativeAot/Generator.cs b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs index 061b64a9a8..db28a50d74 100644 --- a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs +++ b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs @@ -71,11 +71,12 @@ protected override string GetBinariesDirectoryPath(string buildArtifactsDirector protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths) { + bool useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath); string extraArguments = NativeAotToolchain.GetExtraArguments(runtimeIdentifier); var content = new StringBuilder(300) - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, extraArguments)}") - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, extraArguments)}") + .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, useArtifactsPath, extraArguments)}") + .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, useArtifactsPath, extraArguments)}") .ToString(); File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);