diff --git a/docs/_changelog/header/v0.14.0.md b/docs/_changelog/header/v0.14.0.md new file mode 100644 index 0000000000..260ba4d47f --- /dev/null +++ b/docs/_changelog/header/v0.14.0.md @@ -0,0 +1,12 @@ +## Highlights + +* The default build toolchains have been updated to pass `IntermediateOutputPath`, `OutputPath`, and `OutDir` properties to the `dotnet build` command. This change forces all build outputs to be placed in a new directory generated by BenchmarkDotNet, and fixes many issues that have been reported with builds. You can also access these paths in your own `.csproj` and `.props` from those properties if you need to copy custom files to the output. + +## Bug fixes + +* Fixed multiple build-related bugs including passing MsBuildArguments and .Net 8's `UseArtifactsOutput`. + +## Breaking Changes + +* `DotNetCliBuilder` removed `retryFailedBuildWithNoDeps` constructor option. +* `DotNetCliCommand` removed `RetryFailedBuildWithNoDeps` property and `BuildNoRestoreNoDependencies()` and `PublishNoBuildAndNoRestore()` methods (replaced with `PublishNoRestore()`). \ No newline at end of file diff --git a/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs b/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs index 29ee184df3..734aa470e4 100644 --- a/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs +++ b/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs @@ -93,6 +93,7 @@ private static DisassembledMethod[] Disassemble(Settings settings, State state) { var result = new List(); + using var sourceCodeProvider = new SourceCodeProvider(); while (state.Todo.Count != 0) { var methodInfo = state.Todo.Dequeue(); @@ -101,7 +102,7 @@ private static DisassembledMethod[] Disassemble(Settings settings, State state) continue; // already handled if (settings.MaxDepth >= methodInfo.Depth) - result.Add(DisassembleMethod(methodInfo, state, settings)); + result.Add(DisassembleMethod(methodInfo, state, settings, sourceCodeProvider)); } return result.ToArray(); @@ -110,7 +111,7 @@ private static DisassembledMethod[] Disassemble(Settings settings, State state) private static bool CanBeDisassembled(ClrMethod method) => !((method.ILOffsetMap is null || method.ILOffsetMap.Length == 0) && (method.HotColdInfo is null || method.HotColdInfo.HotStart == 0 || method.HotColdInfo.HotSize == 0)); - private static DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state, Settings settings) + private static DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state, Settings settings, SourceCodeProvider sourceCodeProvider) { var method = methodInfo.Method; @@ -133,7 +134,7 @@ private static DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State var uniqueSourceCodeLines = new HashSet(new SharpComparer()); // for getting C# code we always use the original ILOffsetMap foreach (var map in method.ILOffsetMap.Where(map => map.StartAddress < map.EndAddress && map.ILOffset >= 0).OrderBy(map => map.StartAddress)) - foreach (var sharp in SourceCodeProvider.GetSource(method, map)) + foreach (var sharp in sourceCodeProvider.GetSource(method, map)) uniqueSourceCodeLines.Add(sharp); codes.AddRange(uniqueSourceCodeLines); diff --git a/src/BenchmarkDotNet.Disassembler.x64/SourceCodeProvider.cs b/src/BenchmarkDotNet.Disassembler.x64/SourceCodeProvider.cs index c9ac897570..f827369775 100644 --- a/src/BenchmarkDotNet.Disassembler.x64/SourceCodeProvider.cs +++ b/src/BenchmarkDotNet.Disassembler.x64/SourceCodeProvider.cs @@ -8,13 +8,34 @@ namespace BenchmarkDotNet.Disassemblers { - internal static class SourceCodeProvider + // This is taken from the Samples\FileAndLineNumbers projects from microsoft/clrmd, + // and replaces the previously-available SourceLocation functionality. + + internal class SourceLocation { - private static readonly Dictionary SourceFileCache = new Dictionary(); + public string FilePath; + public int LineNumber; + public int LineNumberEnd; + public int ColStart; + public int ColEnd; + } + + internal class SourceCodeProvider : IDisposable + { + private readonly Dictionary sourceFileCache = new Dictionary(); + private readonly Dictionary pdbReaders = new Dictionary(); + + public void Dispose() + { + foreach (var reader in pdbReaders.Values) + { + reader?.Dispose(); + } + } - internal static IEnumerable GetSource(ClrMethod method, ILToNativeMap map) + internal IEnumerable GetSource(ClrMethod method, ILToNativeMap map) { - var sourceLocation = method.GetSourceLocation(map.ILOffset); + var sourceLocation = GetSourceLocation(method, map.ILOffset); if (sourceLocation == null) yield break; @@ -39,16 +60,16 @@ internal static IEnumerable GetSource(ClrMethod method, ILToNativeMap map } } - private static string ReadSourceLine(string file, int line) + private string ReadSourceLine(string file, int line) { - if (!SourceFileCache.TryGetValue(file, out string[] contents)) + if (!sourceFileCache.TryGetValue(file, out string[] contents)) { // sometimes the symbols report some disk location from MS CI machine like "E:\A\_work\308\s\src\mscorlib\shared\System\Random.cs" for .NET Core 2.0 if (!File.Exists(file)) return null; contents = File.ReadAllLines(file); - SourceFileCache.Add(file, contents); + sourceFileCache.Add(file, contents); } return line - 1 < contents.Length @@ -84,29 +105,8 @@ private static string GetSmartPointer(string sourceLine, int? start, int? end) return new string(prefix); } - } - - - // This is taken from the Samples\FileAndLineNumbers projects from microsoft/clrmd, - // and replaces the previously-available SourceLocation functionality. - - internal class SourceLocation - { - public string FilePath; - public int LineNumber; - public int LineNumberEnd; - public int ColStart; - public int ColEnd; - } - - internal static class ClrSourceExtensions - { - // TODO Not sure we want this to be a shared dictionary, especially without - // any synchronization. Probably want to put this hanging off the Context - // somewhere, or inside SymbolCache. - private static readonly Dictionary s_pdbReaders = new Dictionary(); - internal static SourceLocation GetSourceLocation(this ClrMethod method, int ilOffset) + internal SourceLocation GetSourceLocation(ClrMethod method, int ilOffset) { PdbReader reader = GetReaderForMethod(method); if (reader == null) @@ -116,7 +116,7 @@ internal static SourceLocation GetSourceLocation(this ClrMethod method, int ilOf return FindNearestLine(function, ilOffset); } - internal static SourceLocation GetSourceLocation(this ClrStackFrame frame) + internal SourceLocation GetSourceLocation(ClrStackFrame frame) { PdbReader reader = GetReaderForMethod(frame.Method); if (reader == null) @@ -178,7 +178,7 @@ private static int FindIlOffset(ClrStackFrame frame) return last; } - private static PdbReader GetReaderForMethod(ClrMethod method) + private PdbReader GetReaderForMethod(ClrMethod method) { ClrModule module = method?.Type?.Module; PdbInfo info = module?.Pdb; @@ -186,7 +186,7 @@ private static PdbReader GetReaderForMethod(ClrMethod method) PdbReader? reader = null; if (info != null) { - if (!s_pdbReaders.TryGetValue(info, out reader)) + if (!pdbReaders.TryGetValue(info, out reader)) { SymbolLocator locator = GetSymbolLocator(module); string pdbPath = locator.FindPdb(info); @@ -207,7 +207,7 @@ private static PdbReader GetReaderForMethod(ClrMethod method) } } - s_pdbReaders[info] = reader; + pdbReaders[info] = reader; } } diff --git a/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs b/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs index 5a0ddccafb..16b3891d24 100644 --- a/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs @@ -126,6 +126,7 @@ private DisassembledMethod[] Disassemble(Settings settings, State state) var result = new List(); DisassemblySyntax syntax = (DisassemblySyntax)Enum.Parse(typeof(DisassemblySyntax), settings.Syntax); + using var sourceCodeProvider = new SourceCodeProvider(); while (state.Todo.Count != 0) { var methodInfo = state.Todo.Dequeue(); @@ -134,7 +135,7 @@ private DisassembledMethod[] Disassemble(Settings settings, State state) continue; // already handled if (settings.MaxDepth >= methodInfo.Depth) - result.Add(DisassembleMethod(methodInfo, state, settings, syntax)); + result.Add(DisassembleMethod(methodInfo, state, settings, syntax, sourceCodeProvider)); } return result.ToArray(); @@ -142,7 +143,7 @@ private DisassembledMethod[] Disassemble(Settings settings, State state) private static bool CanBeDisassembled(ClrMethod method) => method.ILOffsetMap.Length > 0 && method.NativeCode > 0; - private DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state, Settings settings, DisassemblySyntax syntax) + private DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state, Settings settings, DisassemblySyntax syntax, SourceCodeProvider sourceCodeProvider) { var method = methodInfo.Method; @@ -165,7 +166,7 @@ private DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state, var uniqueSourceCodeLines = new HashSet(new SharpComparer()); // for getting C# code we always use the original ILOffsetMap foreach (var map in method.ILOffsetMap.Where(map => map.StartAddress < map.EndAddress && map.ILOffset >= 0).OrderBy(map => map.StartAddress)) - foreach (var sharp in SourceCodeProvider.GetSource(method, map)) + foreach (var sharp in sourceCodeProvider.GetSource(method, map)) uniqueSourceCodeLines.Add(sharp); codes.AddRange(uniqueSourceCodeLines); diff --git a/src/BenchmarkDotNet/Disassemblers/SourceCodeProvider.cs b/src/BenchmarkDotNet/Disassemblers/SourceCodeProvider.cs index 777c24370e..beaeebfaa7 100644 --- a/src/BenchmarkDotNet/Disassemblers/SourceCodeProvider.cs +++ b/src/BenchmarkDotNet/Disassemblers/SourceCodeProvider.cs @@ -7,14 +7,21 @@ namespace BenchmarkDotNet.Disassemblers { - internal static class SourceCodeProvider + internal class SourceCodeProvider : IDisposable { - private static readonly Dictionary SourceFileCache = new Dictionary(); - private static readonly Dictionary SourceFilePathsCache = new Dictionary(); + private readonly Dictionary sourceFileCache = new Dictionary(); + private readonly Dictionary sourceFilePathsCache = new Dictionary(); + private readonly Dictionary pdbReaders = new Dictionary(); + private readonly SymbolReader symbolReader = new SymbolReader(TextWriter.Null) { SymbolPath = SymbolPath.MicrosoftSymbolServerPath }; - internal static IEnumerable GetSource(ClrMethod method, ILToNativeMap map) + public void Dispose() { - var sourceLocation = method.GetSourceLocation(map.ILOffset); + symbolReader.Dispose(); + } + + internal IEnumerable GetSource(ClrMethod method, ILToNativeMap map) + { + var sourceLocation = GetSourceLocation(method, map.ILOffset); if (sourceLocation == null) yield break; @@ -39,12 +46,12 @@ internal static IEnumerable GetSource(ClrMethod method, ILToNativeMap map } } - private static string GetFilePath(SourceFile sourceFile) - => SourceFilePathsCache.TryGetValue(sourceFile, out string filePath) ? filePath : sourceFile.Url; + private string GetFilePath(SourceFile sourceFile) + => sourceFilePathsCache.TryGetValue(sourceFile, out string filePath) ? filePath : sourceFile.Url; - private static string ReadSourceLine(SourceFile file, int line) + private string ReadSourceLine(SourceFile file, int line) { - if (!SourceFileCache.TryGetValue(file, out string[] contents)) + if (!sourceFileCache.TryGetValue(file, out string[] contents)) { // GetSourceFile method returns path when file is stored on the same machine // otherwise it downloads it from the Symbol Server and returns the source code ;) @@ -56,14 +63,14 @@ private static string ReadSourceLine(SourceFile file, int line) if (File.Exists(wholeFileOrJustPath)) { contents = File.ReadAllLines(wholeFileOrJustPath); - SourceFilePathsCache.Add(file, wholeFileOrJustPath); + sourceFilePathsCache.Add(file, wholeFileOrJustPath); } else { contents = wholeFileOrJustPath.Split(new string[] { Environment.NewLine }, StringSplitOptions.None); } - SourceFileCache.Add(file, contents); + sourceFileCache.Add(file, contents); } return line - 1 < contents.Length @@ -99,17 +106,8 @@ private static string GetSmartPointer(string sourceLine, int? start, int? end) return new string(prefix); } - } - - internal static class ClrSourceExtensions - { - // TODO Not sure we want this to be a shared dictionary, especially without - // any synchronization. Probably want to put this hanging off the Context - // somewhere, or inside SymbolCache. - private static readonly Dictionary s_pdbReaders = new Dictionary(); - private static readonly SymbolReader symbolReader = new SymbolReader(TextWriter.Null) { SymbolPath = SymbolPath.MicrosoftSymbolServerPath }; - internal static SourceLocation GetSourceLocation(this ClrMethod method, int ilOffset) + internal SourceLocation GetSourceLocation(ClrMethod method, int ilOffset) { var reader = GetReaderForMethod(method); if (reader == null) @@ -118,7 +116,7 @@ internal static SourceLocation GetSourceLocation(this ClrMethod method, int ilOf return reader.SourceLocationForManagedCode((uint)method.MetadataToken, ilOffset); } - internal static SourceLocation GetSourceLocation(this ClrStackFrame frame) + internal SourceLocation GetSourceLocation(ClrStackFrame frame) { var reader = GetReaderForMethod(frame.Method); if (reader == null) @@ -145,7 +143,7 @@ private static int FindIlOffset(ClrStackFrame frame) return last; } - private static ManagedSymbolModule GetReaderForMethod(ClrMethod method) + private ManagedSymbolModule GetReaderForMethod(ClrMethod method) { ClrModule module = method?.Type?.Module; PdbInfo info = module?.Pdb; @@ -153,7 +151,7 @@ private static ManagedSymbolModule GetReaderForMethod(ClrMethod method) ManagedSymbolModule? reader = null; if (info != null) { - if (!s_pdbReaders.TryGetValue(info, out reader)) + if (!pdbReaders.TryGetValue(info, out reader)) { string pdbPath = info.Path; if (pdbPath != null) @@ -173,7 +171,7 @@ private static ManagedSymbolModule GetReaderForMethod(ClrMethod method) } } - s_pdbReaders[info] = reader; + pdbReaders[info] = reader; } } diff --git a/src/BenchmarkDotNet/Jobs/JobExtensions.cs b/src/BenchmarkDotNet/Jobs/JobExtensions.cs index 2a236dc44f..2c778df589 100644 --- a/src/BenchmarkDotNet/Jobs/JobExtensions.cs +++ b/src/BenchmarkDotNet/Jobs/JobExtensions.cs @@ -436,5 +436,10 @@ private static Job WithCore(this Job job, Action updateCallback) updateCallback(newJob); return newJob; } + + internal static bool HasDynamicBuildCharacteristic(this Job job) => + job.HasValue(InfrastructureMode.NuGetReferencesCharacteristic) + || job.HasValue(InfrastructureMode.BuildConfigurationCharacteristic) + || job.HasValue(InfrastructureMode.ArgumentsCharacteristic); } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Running/BuildPartition.cs b/src/BenchmarkDotNet/Running/BuildPartition.cs index f89863215c..4ffc48d6c8 100644 --- a/src/BenchmarkDotNet/Running/BuildPartition.cs +++ b/src/BenchmarkDotNet/Running/BuildPartition.cs @@ -4,8 +4,12 @@ using BenchmarkDotNet.Characteristics; using BenchmarkDotNet.Configs; using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Helpers; using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Portability; +using BenchmarkDotNet.Toolchains; using BenchmarkDotNet.Toolchains.CsProj; +using BenchmarkDotNet.Toolchains.DotNetCli; using BenchmarkDotNet.Toolchains.MonoWasm; using BenchmarkDotNet.Toolchains.Roslyn; using JetBrains.Annotations; @@ -71,5 +75,20 @@ private static string GetResolvedAssemblyLocation(Assembly assembly) => // in case of SingleFile, location.Length returns 0, so we use GetName() and // manually construct the path. assembly.Location.Length == 0 ? Path.Combine(AppContext.BaseDirectory, assembly.GetName().Name) : assembly.Location; + + internal bool ForcedNoDependenciesForIntegrationTests + { + get + { + if (!XUnitHelper.IsIntegrationTest.Value || !RuntimeInformation.IsNetCore) + return false; + + var job = RepresentativeBenchmarkCase.Job; + if (job.GetToolchain().Builder is not DotNetCliBuilder) + return false; + + return !job.HasDynamicBuildCharacteristic(); + } + } } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs b/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs index 72faf5c956..0c097f5bb0 100644 --- a/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs +++ b/src/BenchmarkDotNet/Toolchains/ArtifactsPaths.cs @@ -4,11 +4,12 @@ namespace BenchmarkDotNet.Toolchains { public class ArtifactsPaths { - public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", ""); + public static readonly ArtifactsPaths Empty = new ArtifactsPaths("", "", "", "", "", "", "", "", "", "", "", ""); [PublicAPI] public string RootArtifactsFolderPath { get; } [PublicAPI] public string BuildArtifactsDirectoryPath { get; } [PublicAPI] public string BinariesDirectoryPath { get; } + [PublicAPI] public string IntermediateDirectoryPath { get; } [PublicAPI] public string ProgramCodePath { get; } [PublicAPI] public string AppConfigPath { get; } [PublicAPI] public string NuGetConfigPath { get; } @@ -22,6 +23,7 @@ public ArtifactsPaths( string rootArtifactsFolderPath, string buildArtifactsDirectoryPath, string binariesDirectoryPath, + string intermediateDirectoryPath, string programCodePath, string appConfigPath, string nuGetConfigPath, @@ -34,6 +36,7 @@ public ArtifactsPaths( RootArtifactsFolderPath = rootArtifactsFolderPath; BuildArtifactsDirectoryPath = buildArtifactsDirectoryPath; BinariesDirectoryPath = binariesDirectoryPath; + IntermediateDirectoryPath = intermediateDirectoryPath; ProgramCodePath = programCodePath; AppConfigPath = appConfigPath; NuGetConfigPath = nuGetConfigPath; diff --git a/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs b/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs index eaf1da4683..7e91c36ff4 100644 --- a/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs @@ -64,6 +64,9 @@ protected override string GetProjectFilePath(string buildArtifactsDirectoryPath) protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration) => Path.Combine(buildArtifactsDirectoryPath, "bin", configuration, TargetFrameworkMoniker); + protected override string GetIntermediateDirectoryPath(string buildArtifactsDirectoryPath, string configuration) + => Path.Combine(buildArtifactsDirectoryPath, "obj", configuration, TargetFrameworkMoniker); + [SuppressMessage("ReSharper", "StringLiteralTypo")] // R# complains about $variables$ protected override void GenerateProject(BuildPartition buildPartition, ArtifactsPaths artifactsPaths, ILogger logger) { diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs index e034279f89..7a7afc3184 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs @@ -14,15 +14,13 @@ public class DotNetCliBuilder : IBuilder private string CustomDotNetCliPath { get; } private bool LogOutput { get; } - private bool RetryFailedBuildWithNoDeps { get; } [PublicAPI] - public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPath = null, bool logOutput = false, bool retryFailedBuildWithNoDeps = true) + public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPath = null, bool logOutput = false) { TargetFrameworkMoniker = targetFrameworkMoniker; CustomDotNetCliPath = customDotNetCliPath; LogOutput = logOutput; - RetryFailedBuildWithNoDeps = retryFailedBuildWithNoDeps; } public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger) @@ -35,8 +33,7 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart buildPartition, Array.Empty(), buildPartition.Timeout, - logOutput: LogOutput, - retryFailedBuildWithNoDeps: RetryFailedBuildWithNoDeps) + logOutput: LogOutput) .RestoreThenBuild(); if (buildResult.IsBuildSuccess && buildPartition.RepresentativeBenchmarkCase.Job.Environment.LargeAddressAware) diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs index 9967b4ef92..3d4f2fefd6 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs @@ -4,7 +4,6 @@ using System.Text; using BenchmarkDotNet.Characteristics; using BenchmarkDotNet.Extensions; -using BenchmarkDotNet.Helpers; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Portability; @@ -32,11 +31,8 @@ public class DotNetCliCommand [PublicAPI] public bool LogOutput { get; } - [PublicAPI] public bool RetryFailedBuildWithNoDeps { get; } - public DotNetCliCommand(string cliPath, string arguments, GenerateResult generateResult, ILogger logger, - BuildPartition buildPartition, IReadOnlyList environmentVariables, TimeSpan timeout, bool logOutput = false, - bool retryFailedBuildWithNoDeps = true) + BuildPartition buildPartition, IReadOnlyList environmentVariables, TimeSpan timeout, bool logOutput = false) { CliPath = cliPath ?? DotNetCliCommandExecutor.DefaultDotNetCliPath.Value; Arguments = arguments; @@ -46,7 +42,6 @@ public DotNetCliCommand(string cliPath, string arguments, GenerateResult generat EnvironmentVariables = environmentVariables; Timeout = timeout; LogOutput = logOutput || (buildPartition is not null && buildPartition.LogBuildOutput); - RetryFailedBuildWithNoDeps = retryFailedBuildWithNoDeps; } public DotNetCliCommand WithArguments(string arguments) @@ -70,30 +65,29 @@ public BuildResult RestoreThenBuild() if (BuildPartition.IsCustomBuildConfiguration) return Build().ToBuildResult(GenerateResult); - var restoreResult = Restore(); - if (!restoreResult.IsSuccess) - return BuildResult.Failure(GenerateResult, restoreResult.AllInformation); - - // On our CI (Windows+.NET 7), Integration tests take to much time because each benchmark run rebuilds the BenchmarkDotNet itself. + // On our CI, Integration tests take too much time, because each benchmark run rebuilds BenchmarkDotNet itself. // To reduce the total duration of the CI workflows, we build all the projects without dependencies - bool forceNoDependencies = XUnitHelper.IsIntegrationTest.Value && - RuntimeInformation.IsWindows() && - RuntimeInformation.IsNetCore; - - DotNetCliCommandResult buildResult; - if (forceNoDependencies) - buildResult = BuildNoRestoreNoDependencies(); - else + if (BuildPartition.ForcedNoDependenciesForIntegrationTests) { - buildResult = BuildNoRestore(); - if (!buildResult.IsSuccess && RetryFailedBuildWithNoDeps) // if we fail to do the full build, we try with --no-dependencies - buildResult = BuildNoRestoreNoDependencies(); + var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments( + GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{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))) + .ToBuildResult(GenerateResult); } + else + { + var restoreResult = Restore(); + if (!restoreResult.IsSuccess) + return BuildResult.Failure(GenerateResult, restoreResult.AllInformation); - if (!buildResult.IsSuccess) - return BuildResult.Failure(GenerateResult, buildResult.AllInformation); - - return buildResult.ToBuildResult(GenerateResult); + // We no longer retry with --no-dependencies, because it fails with --output set at the same time, + // and the artifactsPaths.BinariesDirectoryPath is set before we try to build, so we cannot overwrite it. + return BuildNoRestore().ToBuildResult(GenerateResult); + } } [PublicAPI] @@ -115,14 +109,8 @@ public BuildResult RestoreThenBuildThenPublish() if (!restoreResult.IsSuccess) return BuildResult.Failure(GenerateResult, restoreResult.AllInformation); - var buildResult = BuildNoRestore(); - if (!buildResult.IsSuccess && RetryFailedBuildWithNoDeps) // if we fail to do the full build, we try with --no-dependencies - buildResult = BuildNoRestoreNoDependencies(); - - if (!buildResult.IsSuccess) - return BuildResult.Failure(GenerateResult, buildResult.AllInformation); - - return PublishNoBuildAndNoRestore().ToBuildResult(GenerateResult); + // We use the implicit build in the publish command. We stopped doing a separate build step because we set the --output. + return PublishNoRestore().ToBuildResult(GenerateResult); } public DotNetCliCommandResult AddPackages() @@ -151,22 +139,19 @@ public DotNetCliCommandResult BuildNoRestore() => DotNetCliCommandExecutor.Execute(WithArguments( GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "build-no-restore"))); - public DotNetCliCommandResult BuildNoRestoreNoDependencies() - => DotNetCliCommandExecutor.Execute(WithArguments( - GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps"))); - public DotNetCliCommandResult Publish() => DotNetCliCommandExecutor.Execute(WithArguments( GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "publish"))); - public DotNetCliCommandResult PublishNoBuildAndNoRestore() + // 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-build --no-restore", "publish-no-build-no-restore"))); + GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{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) + internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false) => new StringBuilder() .AppendArgument("restore") .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"--packages \"{artifactsPaths.PackagesDirectoryName}\"") @@ -174,9 +159,10 @@ internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPar .AppendArgument(extraArguments) .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration)) .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix)) + .MaybeAppendOutputPaths(artifactsPaths, true, excludeOutput) .ToString(); - internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null) + internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, 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)) @@ -184,6 +170,7 @@ 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) .ToString(); internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null) @@ -194,7 +181,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)) - .AppendArgument($"--output \"{artifactsPaths.BinariesDirectoryPath}\"") + .MaybeAppendOutputPaths(artifactsPaths) .ToString(); private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string suffix) @@ -262,4 +249,21 @@ private static string BuildAddPackageCommand(NuGetReference reference) return commandBuilder.ToString(); } } + + internal static class DotNetCliCommandExtensions + { + // Fix #1377 (see comments in #1773). + // 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 + .AppendArgument($"/p:IntermediateOutputPath=\"{artifactsPaths.IntermediateDirectoryPath}\"\\") + .AppendArgument($"/p:OutDir=\"{artifactsPaths.BinariesDirectoryPath}\"") + // 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}\"") + .AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}\""); + } } diff --git a/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs b/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs index 0d7ab30d7d..0b3aea02cd 100644 --- a/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs +++ b/src/BenchmarkDotNet/Toolchains/GeneratorBase.cs @@ -43,11 +43,18 @@ public GenerateResult GenerateProject(BuildPartition buildPartition, ILogger log [PublicAPI] protected abstract string GetBuildArtifactsDirectoryPath(BuildPartition assemblyLocation, string programName); /// - /// returns a path where executable should be found after the build + /// returns a path where executable should be found after the build (usually \bin) /// [PublicAPI] protected virtual string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration) => buildArtifactsDirectoryPath; + /// + /// returns a path where intermediate files should be found after the build (usually \obj) + /// + [PublicAPI] + protected virtual string GetIntermediateDirectoryPath(string buildArtifactsDirectoryPath, string configuration) + => string.Empty; + /// /// returns OS-specific executable extension /// @@ -128,6 +135,7 @@ private ArtifactsPaths GetArtifactsPaths(BuildPartition buildPartition, string r rootArtifactsFolderPath: rootArtifactsFolderPath, buildArtifactsDirectoryPath: buildArtifactsDirectoryPath, binariesDirectoryPath: binariesDirectoryPath, + intermediateDirectoryPath: GetIntermediateDirectoryPath(buildArtifactsDirectoryPath, buildPartition.BuildConfiguration), programCodePath: Path.Combine(buildArtifactsDirectoryPath, $"{programName}{codeFileExtension}"), appConfigPath: $"{executablePath}.config", nuGetConfigPath: Path.Combine(buildArtifactsDirectoryPath, "NuGet.config"), diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs index c2b3199d93..0686a322ac 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitArtifactsPath.cs @@ -12,6 +12,7 @@ public InProcessEmitArtifactsPath( baseArtifacts.RootArtifactsFolderPath, baseArtifacts.BuildArtifactsDirectoryPath, baseArtifacts.BinariesDirectoryPath, + baseArtifacts.IntermediateDirectoryPath, baseArtifacts.ProgramCodePath, baseArtifacts.AppConfigPath, baseArtifacts.NuGetConfigPath, diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs index c169c91903..18734fcf46 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/InProcessEmitGenerator.cs @@ -47,6 +47,7 @@ private ArtifactsPaths GetArtifactsPaths(BuildPartition buildPartition, string r rootArtifactsFolderPath: rootArtifactsFolderPath, buildArtifactsDirectoryPath: buildArtifactsDirectoryPath, binariesDirectoryPath: binariesDirectoryPath, + intermediateDirectoryPath: null, programCodePath: null, appConfigPath: null, nuGetConfigPath: null, diff --git a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs index d17fad7d00..cd64739ba9 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs @@ -59,6 +59,6 @@ protected override string GetExecutablePath(string binariesDirectoryPath, string : Path.Combine(binariesDirectoryPath, programName); protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration) - => Path.Combine(buildArtifactsDirectoryPath, "bin", TargetFrameworkMoniker, CustomDotNetCliToolchainBuilder.GetPortableRuntimeIdentifier(), "publish"); + => Path.Combine(buildArtifactsDirectoryPath, "bin", configuration, TargetFrameworkMoniker, CustomDotNetCliToolchainBuilder.GetPortableRuntimeIdentifier(), "publish"); } } diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs index acc1be303a..cc251abb0d 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs @@ -69,8 +69,6 @@ protected void GenerateProjectFile(BuildPartition buildPartition, ArtifactsPaths protected override string GetExecutablePath(string binariesDirectoryPath, string programName) => Path.Combine(binariesDirectoryPath, MainJS); protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration) - { - return Path.Combine(buildArtifactsDirectoryPath, "bin", TargetFrameworkMoniker, "browser-wasm", "AppBundle"); - } + => Path.Combine(buildArtifactsDirectoryPath, "bin", configuration, TargetFrameworkMoniker, "browser-wasm", "AppBundle"); } } diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs index 37f816bde9..c5ede03e49 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs @@ -49,8 +49,7 @@ public static IToolchain From(NetCoreAppSettings netCoreAppSettings) new DotNetCliBuilder(netCoreAppSettings.TargetFrameworkMoniker, netCoreAppSettings.CustomDotNetCliPath, // aot builds can be very slow - logOutput: netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm, - retryFailedBuildWithNoDeps: false), + logOutput: netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm), new WasmExecutor(), netCoreAppSettings.CustomDotNetCliPath); } diff --git a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs index 65eaceadfb..bbeb0113d8 100644 --- a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs +++ b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs @@ -73,7 +73,6 @@ protected override void GenerateBuildScript(BuildPartition buildPartition, Artif var content = new StringBuilder(300) .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, extraArguments)}") - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, extraArguments)}") .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, extraArguments)}") .ToString(); diff --git a/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs b/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs index 837c757cd7..c2872810fc 100644 --- a/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs +++ b/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs @@ -28,9 +28,7 @@ private static IToolchain GetToolchain(Job job, Descriptor descriptor) : GetToolchain( job.ResolveValue(EnvironmentMode.RuntimeCharacteristic, EnvironmentResolver.Instance), descriptor, - job.HasValue(InfrastructureMode.NuGetReferencesCharacteristic) - || job.HasValue(InfrastructureMode.BuildConfigurationCharacteristic) - || job.HasValue(InfrastructureMode.ArgumentsCharacteristic)); + job.HasDynamicBuildCharacteristic()); internal static IToolchain GetToolchain(this Runtime runtime, Descriptor? descriptor = null, bool preferMsBuildToolchains = false) { diff --git a/tests/BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks/MultipleFrameworksTest.cs b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks/MultipleFrameworksTest.cs index 2559034d2c..0e9eb6f80d 100644 --- a/tests/BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks/MultipleFrameworksTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks/MultipleFrameworksTest.cs @@ -6,7 +6,7 @@ using BenchmarkDotNet.Portability; using Xunit; -namespace BenchmarkDotNet.IntegrationTests +namespace BenchmarkDotNet.IntegrationTests.ManualRunning { // Note: To properly test this locally, modify // BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks.csproj, diff --git a/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/BenchmarkDotNet.IntegrationTests.ManualRunning.csproj b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/BenchmarkDotNet.IntegrationTests.ManualRunning.csproj index 77d82eb30e..b3fef215b4 100755 --- a/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/BenchmarkDotNet.IntegrationTests.ManualRunning.csproj +++ b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/BenchmarkDotNet.IntegrationTests.ManualRunning.csproj @@ -11,6 +11,11 @@ AnyCPU true + + + $(DefineConstants);CUSTOM_PROP + + diff --git a/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/MsBuildArgumentTests.cs b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/MsBuildArgumentTests.cs new file mode 100644 index 0000000000..f4619040cc --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.ManualRunning/MsBuildArgumentTests.cs @@ -0,0 +1,65 @@ +using System; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Jobs; +using Xunit; + +namespace BenchmarkDotNet.IntegrationTests.ManualRunning +{ + public class MsBuildArgumentTests : BenchmarkTestExecutor + { + private const string CustomPropEnvVarName = "CustomPropEnvVarName"; + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ProcessIsBuiltWithCustomProperty(bool setCustomProperty) + { + var config = ManualConfig.CreateEmpty() + .AddJob(Job.Dry + .WithArguments(new Argument[] { new MsBuildArgument($"/p:CustomProp={setCustomProperty}") }) + .WithEnvironmentVariable(CustomPropEnvVarName, setCustomProperty.ToString()) + ); + CanExecute(config); + } + + [Fact] + public void MultipleProcessesAreBuiltWithCorrectProperties() + { + var config = ManualConfig.CreateEmpty() + .AddJob(Job.Dry + .WithArguments(new Argument[] { new MsBuildArgument($"/p:CustomProp={true}") }) + .WithEnvironmentVariable(CustomPropEnvVarName, true.ToString()) + ) + .AddJob(Job.Dry + .WithRuntime(NativeAotRuntime.Net80) + .WithArguments(new Argument[] { new MsBuildArgument($"/p:CustomProp={true}") }) + .WithEnvironmentVariable(CustomPropEnvVarName, true.ToString()) + ) + .AddJob(Job.Dry + .WithEnvironmentVariable(CustomPropEnvVarName, false.ToString()) + ); + CanExecute(config); + } + + public class PropertyDefine + { + private const bool customPropWasSet = +#if CUSTOM_PROP + true; +#else + false; +#endif + + [Benchmark] + public void ThrowWhenWrong() + { + if (Environment.GetEnvironmentVariable(CustomPropEnvVarName) != customPropWasSet.ToString()) + { + throw new InvalidOperationException($"Custom property was not set properly, the expected value was {Environment.GetEnvironmentVariable(CustomPropEnvVarName)}"); + } + } + } + } +} \ No newline at end of file