Skip to content

Commit d441488

Browse files
timcassellclaude
andcommitted
Fix output-path regressions in the csproj build plumbing
Three defects found reviewing #3235, originally verified against a console (exe) benchmark project on NativeAotToolchain -- the shape CI cannot reach, since BenchmarkDotNet.IntegrationTests is a library. 1. DotNetCliCommand: --no-dependencies was widened to every autogenerated build via a filename check. The generated project keeps its ProjectReference, and RIDs propagate to exe references but not library ones, so the reference resolved to a RID-specific ref assembly that nothing produced (CS0006, plus MSB3243 against the gathered Reference). Reverted to the previous ForcedNoDependenciesForIntegrationTests gate. 2. GeneratorBase: GetPublishDirectoryPath defaulted to <artifacts>/publish while the generators expect the executable in their binaries directory. That was masked while --output overrode PublishDir on the command line; once PublishDir became authoritative, publish wrote somewhere BenchmarkDotNet never looks. Default it to the binaries directory. 3. CsProjGenerator: OutputPath was derived by chopping TFM.Length + 1 characters off the binaries path, which is wrong for generators whose path ends in the RID. Set AppendTargetFrameworkToOutputPath=false in the generated props instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 649e8a9 commit d441488

4 files changed

Lines changed: 15 additions & 11 deletions

File tree

src/BenchmarkDotNet/Templates/BenchmarkDotNet.Build.props.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,7 @@
77
<PropertyGroup Condition="'$(MSBuildProjectFile)' == '$AUTOGENERATED_PROJ$'">
88
<OutputPath>$OUTPUT_PATH$</OutputPath>
99
<PublishDir>$PUBLISH_DIR$</PublishDir>
10+
<!-- OutputPath already ends with the moniker, so MSBuild must not append it again. -->
11+
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
1012
</PropertyGroup>
1113
</Project>

src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,12 @@ protected override string GetBinariesDirectoryPath(string buildArtifactsDirector
6969

7070
protected override async ValueTask GenerateCustomBuildHooksAsync(BuildPartition buildPartition, ArtifactsPaths artifactsPaths, ILogger logger, CancellationToken cancellationToken)
7171
{
72-
var benchmark = buildPartition.RepresentativeBenchmarkCase;
73-
var benchmarkProjectFile = GetProjectFilePath(benchmark.Descriptor.Type, logger);
74-
7572
var buildArtifactsDirectoryPath = artifactsPaths.BuildArtifactsDirectoryPath;
7673

77-
// Trim tfm part from binariesDirectoryPath on IntegrationTests. (When ArtifactsPath is not used, it's appended automatically)
78-
var binariesDirectoryPath = buildPartition.ForcedNoDependenciesForIntegrationTests
79-
? artifactsPaths.BinariesDirectoryPath.Substring(0, artifactsPaths.BinariesDirectoryPath.Length - Settings.TargetFrameworkMoniker.Length - 1)
80-
: artifactsPaths.BinariesDirectoryPath;
74+
// The generator already puts the moniker (and for some toolchains the RID) into these
75+
// paths, so MSBuild must not append it a second time. It does that whenever the
76+
// artifacts layout is not in use, which is the case for our own integration tests.
77+
var binariesDirectoryPath = artifactsPaths.BinariesDirectoryPath;
8178

8279
var propsPath = Path.Combine(buildArtifactsDirectoryPath, "BenchmarkDotNet.Build.props");
8380
var props = GetTemplateContent("BenchmarkDotNet.Build.props.txt");

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using BenchmarkDotNet.Loggers;
55
using BenchmarkDotNet.Portability;
66
using BenchmarkDotNet.Running;
7-
using BenchmarkDotNet.Toolchains.CsProj;
87
using BenchmarkDotNet.Toolchains.Results;
98
using JetBrains.Annotations;
109
using System.Text;
@@ -54,8 +53,12 @@ public DotNetCliCommand WithArguments(string arguments)
5453
public DotNetCliCommand WithCliPath(FileInfo? cliPath)
5554
=> new(cliPath, FilePath, TargetFrameworkMoniker, Arguments, GenerateResult, Logger, BuildPartition, EnvironmentVariables, Timeout, LogOutput);
5655

56+
// Building without dependencies skips building the referenced benchmark project, so the
57+
// generated project's ProjectReference resolves to output that was never produced. That is
58+
// only safe for our own integration tests, where the gatherer is skipped and the referenced
59+
// project is a library built by the test run itself.
5760
private bool UseNoDependencies
58-
=> BuildPartition.IsCustomBuildConfiguration || Path.GetFileName(FilePath) == CsProjGenerator.AutogeneratedProjectName;
61+
=> BuildPartition.ForcedNoDependenciesForIntegrationTests;
5962

6063
[PublicAPI]
6164
public async Task<BuildResult> RestoreThenBuildAsync(CancellationToken cancellationToken = default)

src/BenchmarkDotNet/Toolchains/GeneratorBase.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ protected virtual string GetBinariesDirectoryPath(string buildArtifactsDirectory
6262
=> buildArtifactsDirectoryPath;
6363

6464
/// <summary>
65-
/// returns a path where the publish directory should be found after the build (usually \publish)
65+
/// returns a path where `dotnet publish` should write its output. It has to contain the
66+
/// executable that <see cref="GetExecutablePath"/> points at, so by default it matches the
67+
/// binaries directory.
6668
/// </summary>
6769
[PublicAPI]
6870
protected virtual string GetPublishDirectoryPath(string buildArtifactsDirectoryPath, string configuration)
69-
=> Path.Combine(buildArtifactsDirectoryPath, "publish");
71+
=> GetBinariesDirectoryPath(buildArtifactsDirectoryPath, configuration);
7072

7173
/// <summary>
7274
/// returns OS-specific executable extension

0 commit comments

Comments
 (0)