Skip to content

chore: Add validator to check .NET SDK as minimum requirements #3194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
filzrev wants to merge 2 commits into
base: master
Choose a base branch
from
+70 −31
Open

chore: Add validator to check .NET SDK as minimum requirements #3194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44aa07e
chore: add validation for net8 sdk as minimum requirements
filzrev Jul 10, 2026
a309aa0
chore: use CoreRuntime.GetParsableVersionPart to parse version
filzrev Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,7 @@ private static async ValueTask<Summary[]> RunCore(BenchmarkRunInfo[] benchmarkRu
var buildPartitions = BenchmarkPartitioner.CreateForBuild(supportedBenchmarks, resolver);
eventProcessor.OnStartBuildStage(buildPartitions);

var sequentialBuildPartitions = buildPartitions.Where(partition =>
partition.Benchmarks.Any(x => x.Config.Options.IsSet(ConfigOptions.DisableParallelBuild))
// .Net SDK 8+ supports ArtifactsPath for proper parallel builds.
// Older SDKs may produce builds with incorrect bindings if more than 1 partition is built in parallel.
|| (partition.RepresentativeBenchmarkCase.GetToolchain().Generator is DotNetCliGenerator
&& partition.RepresentativeBenchmarkCase.GetRuntime().RuntimeMoniker.GetRuntimeVersion().Major < 8)
)
.ToArray();
var sequentialBuildPartitions = buildPartitions.Where(partition => partition.Benchmarks.Any(x => x.Config.Options.IsSet(ConfigOptions.DisableParallelBuild))).ToArray();
var parallelBuildPartitions = buildPartitions.Except(sequentialBuildPartitions).ToArray();

Dictionary<BuildPartition, BuildResult> buildResults = parallelBuildPartitions.Length > 0
Expand Down
65 changes: 42 additions & 23 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,52 +148,52 @@ public Task<DotNetCliCommandResult> PublishNoRestoreAsync(CancellationToken canc
cancellationToken);

internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string filePath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
=> new StringBuilder()
=> new StringBuilder(256)
.AppendArgument("restore")
.AppendArgument($"\"{filePath}\"")
.AppendArgument($"\"{filePath.ToRelativePath(artifactsPaths)}\"")
// restore doesn't support -f argument.
.AppendArgument(artifactsPaths.PackagesDirectoryName.IsBlank() ? 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)
.MaybeAppendExtraArguments(artifactsPaths, isRestore: true, excludeOutput: excludeOutput)
.ToString();

internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string filePath, string tfm, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
=> new StringBuilder()
=> new StringBuilder(256)
.AppendArgument("build")
.AppendArgument($"\"{filePath}\"")
.AppendArgument($"\"{filePath.ToRelativePath(artifactsPaths)}\"")
.AppendArgument($"-f {tfm}")
.AppendArgument($"-c {buildPartition.BuildConfiguration}")
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
.AppendArgument(extraArguments)
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
.AppendArgument(artifactsPaths.PackagesDirectoryName.IsBlank() ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
.MaybeAppendOutputPaths(artifactsPaths, excludeOutput: excludeOutput)
.MaybeAppendExtraArguments(artifactsPaths, isBuild: true, excludeOutput: excludeOutput)
.ToString();

internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string filePath, string tfm, string? extraArguments = null, string? binLogSuffix = null)
=> new StringBuilder()
=> new StringBuilder(256)
.AppendArgument("publish")
.AppendArgument($"\"{filePath}\"")
.AppendArgument($"\"{filePath.ToRelativePath(artifactsPaths)}\"")
.AppendArgument($"-f {tfm}")
.AppendArgument($"-c {buildPartition.BuildConfiguration}")
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
.AppendArgument(extraArguments)
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
.AppendArgument(artifactsPaths.PackagesDirectoryName.IsBlank() ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
.MaybeAppendOutputPaths(artifactsPaths)
.MaybeAppendExtraArguments(artifactsPaths, isPublish: true)
.ToString();

private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string? suffix)
{
if (!buildPartition.GenerateMSBuildBinLog || suffix.IsBlank())
return string.Empty;

return $"\"-bl:{buildPartition.ProgramName}-{suffix}.binlog\"";
return $"-bl:\"{buildPartition.ProgramName}-{suffix}.binlog\"";
}

private static string GetCustomMsBuildArguments(BenchmarkCase benchmarkCase, IResolver resolver)
Expand Down Expand Up @@ -228,18 +228,37 @@ 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)
=> excludeOutput
? stringBuilder
: stringBuilder
// Use AltDirectorySeparatorChar so it's not interpreted as an escaped quote `\"`.
// Use a subdirectory for ArtifactsPath so that DefaultItemExcludes (which the SDK
// sets to $(ArtifactsPath)/**) doesn't cover project-level files like wwwroot/.
.AppendArgument($"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}.artifacts{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}\"");
internal static StringBuilder MaybeAppendExtraArguments(
this StringBuilder stringBuilder,
ArtifactsPaths artifactsPaths,
bool isRestore = false,
bool isBuild = false,

@timcassell timcassell Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused param

@filzrev filzrev Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is being added for API consistency between commands.

It'll be removed on another PR if it's not expected to be used.

bool isPublish = false,
bool excludeOutput = false)
{
if (!excludeOutput)
{
// Use a subdirectory for ArtifactsPath so that DefaultItemExcludes (which the SDK
// sets to $(ArtifactsPath)/**) doesn't cover project-level files like wwwroot/.
stringBuilder.AppendArgument($"--artifacts-path .artifacts");

if (!isRestore)
stringBuilder.AppendArgument($"--output \"{artifactsPaths.BinariesDirectoryPath.ToRelativePath(artifactsPaths)}\"");

@timcassell timcassell Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we don't need to pass --output with artifacts path?

@filzrev filzrev Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also agree --output argument should be removed from command line.
(This setting output all project dependencies output to single directory and cause confilecs)

Though, it will be handled on another PR.

artifactsPaths.BinariesDirectoryPath value is set on early stage of GenerateProjectAsync.
and this path also used on GenerateAppConfigAsync
So BinariesDirectoryPath setting itself will not be changed.

Instead, following steps are expected.

  1. Gets TargetDir property(.artifacts\bin\BenchmarkDotNet.Autogenerated\release_net8.0) on project build phase.
  2. Copy output files from TargetDir to BinariesDirectoryPath when build succeeded.

}

if (isPublish)
stringBuilder.AppendArgument($"/p:PublishDir=\"{artifactsPaths.PublishDirectoryPath}\"");

@timcassell timcassell Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can also omit PublishDir since we're always using artifacts path, the toolchains that use it can update their location (.artifacts/publish instead of bin/publish).

@filzrev filzrev Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Publish target directory also needs MSBuild project evaluation to determine actual value.
(e.g. it's resolved to artifacts\publish\ConsoleApp7\release_net10.0)

So it seems better to use fixed path that relative to BuildArtifactsDirectoryPath.

@timcassell timcassell Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the same issue with non-publishing toolchains for the bin directory. If we're removing --output, we should unify the logic for both. It seems pretty straight forward for us to construct that path since we're generating the project name, and we have the tfm and configuration.

@timcassell timcassell Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we could also add <ArtifactsPivots></ArtifactsPivots> property to the csproj to strip the release_net10.0 part without affecting dependencies, so we can just build the path from the generated project name (we could also set ArtifactsProjectName, but it'd be redundant).

@filzrev filzrev Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the same issue with non-publishing toolchains for the bin directory. If we're removing --output, we should unify the logic for both.

It'll be expected to be handled on another PR.

It looks like we could also add property to the csproj

I'll create another PR to supports custom props/targets on benchmark project build.
/p:PublishDir setting will be removed by using this feature.

Additionally, ErrorOnDuplicatePublishOutputFiles setting also should be removed.

@timcassell timcassell Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be expected to be handled on another PR.

Let's just include the full fix in a single PR, please.


return stringBuilder;
}

internal static string ToRelativePath(this string path, ArtifactsPaths artifactsPaths)
{
var buildArtifactsDirectoryPath = $"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.DirectorySeparatorChar}";
if (path.StartsWith(buildArtifactsDirectoryPath))
return path.Substring(buildArtifactsDirectoryPath.Length);

return path;
}
}
}
27 changes: 27 additions & 0 deletions src/BenchmarkDotNet/Validators/DotNetSdkValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using System.ComponentModel;
Expand All @@ -20,10 +21,19 @@ public static IEnumerable<ValidationError> ValidateCoreSdks(string? customDotNet
yield return cliPathError;
yield break;
}

var requiredSdkVersion = benchmark.GetRuntime().RuntimeMoniker.GetRuntimeVersion();
if (!GetInstalledDotNetSdks(customDotNetCliPath).Any(sdk => sdk >= requiredSdkVersion))
{
yield return new ValidationError(true, $"The required .NET Core SDK version {requiredSdkVersion} or higher for runtime moniker {benchmark.Job.Environment.Runtime!.RuntimeMoniker} is not installed.", benchmark);
yield break;
}

// Validate actual .NET SDK version. (.NET 8 SDK is minimum requirement to use ArtifactsPath)
if (TryGetDotNetSdkVersion(customDotNetCliPath, out string rawVersionText, out Version? sdkVersion))
{
if (sdkVersion.Major < 8)
yield return new ValidationError(true, $"The .NET 8 SDK is the minimum requirement for building the project. Resolved SDK version: {rawVersionText}", benchmark);
}
}

Expand Down Expand Up @@ -182,5 +192,22 @@ private static string CheckFor45PlusVersion(int releaseKey)

return "";
}

private static bool TryGetDotNetSdkVersion(
string? customDotNetCliPath,
out string rawSdkVersion,
[NotNullWhen(true)] out Version? sdkVersion)
{
string exePath = customDotNetCliPath.IsBlank()
? "dotnet"
: customDotNetCliPath!;

rawSdkVersion = ProcessHelper.RunAndReadOutput(exePath, "--version") ?? "";
if (Version.TryParse(CoreRuntime.GetParsableVersionPart(rawSdkVersion), out sdkVersion))
return true;

sdkVersion = null;
return false;
}
}
}