Skip to content

Commit 5a1fad3

Browse files
timcasselltimcassell
committed
Removed IntermediateOutputPath, always use ArtifactsPath.
Updated DotNetSdkValidator.
1 parent 7d37834 commit 5a1fad3

11 files changed

+55
-95
lines changed

‎src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs

+4-7
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ namespace BenchmarkDotNet.Toolchains
44
{
55
public class ArtifactsPaths
66
{
7-
public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", "", "", "");
7+
public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", "", "");
88

99
[PublicAPI] public string RootArtifactsFolderPath { get; }
1010
[PublicAPI] public string BuildArtifactsDirectoryPath { get; }
11-
[PublicAPI] public string PublishDirectoryPath { get; }
1211
[PublicAPI] public string BinariesDirectoryPath { get; }
13-
[PublicAPI] public string IntermediateDirectoryPath { get; }
12+
[PublicAPI] public string PublishDirectoryPath { get; }
1413
[PublicAPI] public string ProgramCodePath { get; }
1514
[PublicAPI] public string AppConfigPath { get; }
1615
[PublicAPI] public string NuGetConfigPath { get; }
@@ -23,9 +22,8 @@ public class ArtifactsPaths
2322
public ArtifactsPaths(
2423
string rootArtifactsFolderPath,
2524
string buildArtifactsDirectoryPath,
26-
string publishDirectoryPath,
2725
string binariesDirectoryPath,
28-
string intermediateDirectoryPath,
26+
string publishDirectoryPath,
2927
string programCodePath,
3028
string appConfigPath,
3129
string nuGetConfigPath,
@@ -37,9 +35,8 @@ public ArtifactsPaths(
3735
{
3836
RootArtifactsFolderPath = rootArtifactsFolderPath;
3937
BuildArtifactsDirectoryPath = buildArtifactsDirectoryPath;
40-
PublishDirectoryPath = publishDirectoryPath;
4138
BinariesDirectoryPath = binariesDirectoryPath;
42-
IntermediateDirectoryPath = intermediateDirectoryPath;
39+
PublishDirectoryPath = publishDirectoryPath;
4340
ProgramCodePath = programCodePath;
4441
AppConfigPath = appConfigPath;
4542
NuGetConfigPath = nuGetConfigPath;

‎src/BenchmarkDotNet/Toolchains/CsProj/CsProjClassicNetToolchain.cs

+1-5
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,8 @@ public override IEnumerable<ValidationError> Validate(BenchmarkCase benchmarkCas
5252
benchmarkCase);
5353
yield break;
5454
}
55-
else if (DotNetSdkValidator.IsCliPathInvalid(CustomDotNetCliPath, benchmarkCase, out var invalidCliError))
56-
{
57-
yield return invalidCliError;
58-
}
5955

60-
foreach (var validationError in DotNetSdkValidator.ValidateFrameworkSdks(benchmarkCase))
56+
foreach (var validationError in DotNetSdkValidator.ValidateFrameworkSdks(CustomDotNetCliPath, benchmarkCase))
6157
{
6258
yield return validationError;
6359
}

‎src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs

-3
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,6 @@ protected override string GetProjectFilePath(string buildArtifactsDirectoryPath)
6464
protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
6565
=> Path.Combine(buildArtifactsDirectoryPath, "bin", configuration, TargetFrameworkMoniker);
6666

67-
protected override string GetIntermediateDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
68-
=> Path.Combine(buildArtifactsDirectoryPath, "obj", configuration, TargetFrameworkMoniker);
69-
7067
[SuppressMessage("ReSharper", "StringLiteralTypo")] // R# complains about $variables$
7168
protected override void GenerateProject(BuildPartition buildPartition, ArtifactsPaths artifactsPaths, ILogger logger)
7269
{

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs

+15-29
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ public class DotNetCliCommand
3333

3434
[PublicAPI] public bool LogOutput { get; }
3535

36-
// Whether to use ArtifactsPath or IntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+.
37-
private readonly bool _useArtifactsPath;
38-
3936
public DotNetCliCommand(string cliPath, string arguments, GenerateResult generateResult, ILogger logger,
4037
BuildPartition buildPartition, IReadOnlyList<EnvironmentVariable> environmentVariables, TimeSpan timeout, bool logOutput = false)
4138
{
@@ -47,8 +44,6 @@ public DotNetCliCommand(string cliPath, string arguments, GenerateResult generat
4744
EnvironmentVariables = environmentVariables;
4845
Timeout = timeout;
4946
LogOutput = logOutput || (buildPartition is not null && buildPartition.LogBuildOutput);
50-
51-
_useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath);
5247
}
5348

5449
public DotNetCliCommand WithArguments(string arguments)
@@ -77,12 +72,12 @@ public BuildResult RestoreThenBuild()
7772
if (BuildPartition.ForcedNoDependenciesForIntegrationTests)
7873
{
7974
var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments(
80-
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
75+
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
8176
if (!restoreResult.IsSuccess)
8277
return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
8378

8479
return DotNetCliCommandExecutor.Execute(WithArguments(
85-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
80+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
8681
.ToBuildResult(GenerateResult);
8782
}
8883
else
@@ -136,62 +131,59 @@ public DotNetCliCommandResult AddPackages()
136131

137132
public DotNetCliCommandResult Restore()
138133
=> DotNetCliCommandExecutor.Execute(WithArguments(
139-
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "restore")));
134+
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "restore")));
140135

141136
public DotNetCliCommandResult Build()
142137
=> DotNetCliCommandExecutor.Execute(WithArguments(
143-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "build")));
138+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "build")));
144139

145140
public DotNetCliCommandResult BuildNoRestore()
146141
=> DotNetCliCommandExecutor.Execute(WithArguments(
147-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore", "build-no-restore")));
142+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "build-no-restore")));
148143

149144
public DotNetCliCommandResult Publish()
150145
=> DotNetCliCommandExecutor.Execute(WithArguments(
151-
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, Arguments, "publish")));
146+
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "publish")));
152147

153148
// PublishNoBuildAndNoRestore was removed because we set --output in the build step. We use the implicit build included in the publish command.
154149
public DotNetCliCommandResult PublishNoRestore()
155150
=> DotNetCliCommandExecutor.Execute(WithArguments(
156-
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, _useArtifactsPath, $"{Arguments} --no-restore", "publish-no-restore")));
151+
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "publish-no-restore")));
157152

158153
internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
159154
=> GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);
160155

161-
internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
162-
bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
156+
internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
163157
=> new StringBuilder()
164158
.AppendArgument("restore")
165159
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"--packages \"{artifactsPaths.PackagesDirectoryName}\"")
166160
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
167161
.AppendArgument(extraArguments)
168162
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
169163
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
170-
.MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath, true, excludeOutput)
164+
.MaybeAppendOutputPaths(artifactsPaths, true, excludeOutput)
171165
.ToString();
172166

173-
internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
174-
bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
167+
internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
175168
=> new StringBuilder()
176169
.AppendArgument($"build -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
177170
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
178171
.AppendArgument(extraArguments)
179172
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
180173
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
181174
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
182-
.MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath, excludeOutput: excludeOutput)
175+
.MaybeAppendOutputPaths(artifactsPaths, excludeOutput: excludeOutput)
183176
.ToString();
184177

185-
internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
186-
bool useArtifactsPath, string? extraArguments = null, string? binLogSuffix = null)
178+
internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null)
187179
=> new StringBuilder()
188180
.AppendArgument($"publish -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
189181
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
190182
.AppendArgument(extraArguments)
191183
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
192184
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
193185
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
194-
.MaybeAppendOutputPaths(artifactsPaths, useArtifactsPath)
186+
.MaybeAppendOutputPaths(artifactsPaths)
195187
.ToString();
196188

197189
private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string suffix)
@@ -266,18 +258,12 @@ internal static class DotNetCliCommandExtensions
266258
// We force the project to output binaries to a new directory.
267259
// Specifying --output and --no-dependencies breaks the build (because the previous build was not done using the custom output path),
268260
// so we don't include it if we're building no-deps (only supported for integration tests).
269-
internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool useArtifactsPath, bool isRestore = false, bool excludeOutput = false)
261+
internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool isRestore = false, bool excludeOutput = false)
270262
=> excludeOutput
271263
? stringBuilder
272264
: stringBuilder
273265
// Use AltDirectorySeparatorChar so it's not interpreted as an escaped quote `\"`.
274-
.AppendArgument(useArtifactsPath
275-
// We set ArtifactsPath for dotnet sdk 8+, fallback to IntermediateOutputPath for older sdks.
276-
? $"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}\""
277-
// This is technically incorrect (#2664, #2425), but it's the best we can do for older sdks.
278-
// MSBuild does not support setting BaseIntermediateOutputPath from command line. https://github.com/dotnet/sdk/issues/2003#issuecomment-369408964
279-
: $"/p:IntermediateOutputPath=\"{artifactsPaths.IntermediateDirectoryPath}{Path.AltDirectorySeparatorChar}\""
280-
)
266+
.AppendArgument($"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
281267
.AppendArgument($"/p:OutDir=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
282268
// OutputPath is legacy, per-project version of OutDir. We set both just in case. https://github.com/dotnet/msbuild/issues/87
283269
.AppendArgument($"/p:OutputPath=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs

+2-18
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using System.Text;
77
using System.Text.RegularExpressions;
88
using BenchmarkDotNet.Detectors;
9-
using BenchmarkDotNet.Environments;
109
using BenchmarkDotNet.Extensions;
1110
using BenchmarkDotNet.Helpers;
1211
using BenchmarkDotNet.Jobs;
@@ -56,24 +55,9 @@ public static DotNetCliCommandResult Execute(DotNetCliCommand parameters)
5655
}
5756
}
5857

59-
internal static bool DotNetSdkSupportsArtifactsPath(string? customDotNetCliPath)
58+
internal static string? GetDotNetSdkVersion()
6059
{
61-
var version = string.IsNullOrEmpty(customDotNetCliPath)
62-
? HostEnvironmentInfo.GetCurrent().DotNetSdkVersion.Value
63-
: GetDotNetSdkVersion(customDotNetCliPath);
64-
if (string.IsNullOrEmpty(version))
65-
{
66-
return false;
67-
}
68-
version = CoreRuntime.GetParsableVersionPart(version);
69-
return Version.TryParse(version, out var semVer) && semVer.Major >= 8;
70-
}
71-
72-
internal static string? GetDotNetSdkVersion() => GetDotNetSdkVersion(null);
73-
74-
internal static string? GetDotNetSdkVersion(string? customDotNetCliPath)
75-
{
76-
using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
60+
using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath: null, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
7761
using (new ConsoleExitHandler(process, NullLogger.Instance))
7862
{
7963
try

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs

+2-7
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,13 @@ public abstract class DotNetCliGenerator : GeneratorBase
2020

2121
protected bool IsNetCore { get; }
2222

23-
// Whether to use ArtifactsPath or IntermediateOutputPath. ArtifactsPath is only supported in dotnet sdk 8+.
24-
private protected readonly bool _useArtifactsPath;
25-
2623
[PublicAPI]
2724
protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore)
2825
{
2926
TargetFrameworkMoniker = targetFrameworkMoniker;
3027
CliPath = cliPath;
3128
PackagesPath = packagesPath;
3229
IsNetCore = isNetCore;
33-
34-
_useArtifactsPath = DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(cliPath);
3530
}
3631

3732
protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe";
@@ -106,8 +101,8 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
106101
protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
107102
{
108103
var content = new StringBuilder(300)
109-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, _useArtifactsPath)}")
110-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, _useArtifactsPath)}")
104+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition)}")
105+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition)}")
111106
.ToString();
112107

113108
File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);

‎src/BenchmarkDotNet/Toolchains/GeneratorBase.cs

+4-12
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,18 @@ public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger log
4444
/// </summary>
4545
[PublicAPI] protected abstract string GetBuildArtifactsDirectoryPath(BuildPartition assemblyLocation, string programName);
4646

47-
/// <summary>
48-
/// returns a path where the publish directory should be found after the build (usually \publish)
49-
/// </summary>
50-
[PublicAPI]
51-
protected virtual string GetPublishDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
52-
=> Path.Combine(buildArtifactsDirectoryPath, "publish");
53-
5447
/// <summary>
5548
/// returns a path where executable should be found after the build (usually \bin)
5649
/// </summary>
5750
[PublicAPI] protected virtual string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
5851
=> buildArtifactsDirectoryPath;
5952

6053
/// <summary>
61-
/// returns a path where intermediate files should be found after the build (usually \obj)
54+
/// returns a path where the publish directory should be found after the build (usually \publish)
6255
/// </summary>
6356
[PublicAPI]
64-
protected virtual string GetIntermediateDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
65-
=> string.Empty;
57+
protected virtual string GetPublishDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
58+
=> Path.Combine(buildArtifactsDirectoryPath, "publish");
6659

6760
/// <summary>
6861
/// returns OS-specific executable extension
@@ -144,9 +137,8 @@ private ArtifactsPaths GetArtifactsPaths(BuildPartition buildPartition, string r
144137
return new ArtifactsPaths(
145138
rootArtifactsFolderPath: rootArtifactsFolderPath,
146139
buildArtifactsDirectoryPath: buildArtifactsDirectoryPath,
147-
publishDirectoryPath: GetPublishDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration),
148140
binariesDirectoryPath: binariesDirectoryPath,
149-
intermediateDirectoryPath: GetIntermediateDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration),
141+
publishDirectoryPath: GetPublishDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration),
150142
programCodePath: Path.Combine(buildArtifactsDirectoryPath, $"{programName}{codeFileExtension}"),
151143
appConfigPath: $"{executablePath}.config",
152144
nuGetConfigPath: Path.Combine(buildArtifactsDirectoryPath, "NuGet.config"),

0 commit comments

Comments
 (0)