Skip to content

Commit

Permalink
Use ArtifactsPath in sdk 8+ instead of IntermediateOutputPath.
Browse files Browse the repository at this point in the history
  • Loading branch information
@timcassell
timcassell committed Dec 12, 2024
1 parent 6367ad8 commit 8c489ea
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 22 deletions.
50 changes: 34 additions & 16 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<EnvironmentVariable> environmentVariables, TimeSpan timeout, bool logOutput = false)
{
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -130,59 +138,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)
Expand Down Expand Up @@ -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}\"")
;
}
}
20 changes: 18 additions & 2 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,20 @@ 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)
{
TargetFrameworkMoniker = targetFrameworkMoniker;
CliPath = cliPath;
PackagesPath = packagesPath;
IsNetCore = isNetCore;

_useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath);
}

protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe";
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 8c489ea

Please sign in to comment.