BenchmarkDotNet icon indicating copy to clipboard operation
BenchmarkDotNet copied to clipboard

feat: Add logics to escape MSBuildArgument text representation when contains MSBuild special chars

Open filzrev opened this issue 5 months ago • 7 comments

This PR intended to fix issue #2719.

There is existing PR(#2729) that trying to resolve issue by escaping MSBuild special chars

Instead, this PR try to resolve issue by surrounding MSBuild parameter value by quotes. (See: https://github.com/dotnet/sdk/issues/8792#issuecomment-393756980 for details)

What's changed in this PR

1. Argument.cs Add internal method GetEscapedTextRepresentation to MsBuildArgument. This method returns platform-dependent text representation that is used for script file.

  1. DotNetCliCommand.cs Modify code to pass GetEscapedTextRepresentation value instead of TextRepresentation

  2. MsBuildArgumentTests.cs Add unit tests for various argument patterns.

Additional integration tests

I've confirmed benchmarks works as expected both Windows/Ubuntu(on WSL) environments.

public class CustomConfig : ManualConfig
{
    public CustomConfig()
    {
        WithOptions(ConfigOptions.DisableOptimizationsValidator);
        WithOptions(ConfigOptions.StopOnFirstError);
        WithOptions(ConfigOptions.KeepBenchmarkFiles);
        WithOptions(ConfigOptions.GenerateMSBuildBinLog);

        WithUnionRule(DefaultConfig.Instance.UnionRule);

        var baseJob = Job.ShortRun
            .WithStrategy(RunStrategy.Monitoring)
            .DontEnforcePowerPlan()
            .Freeze();

        AddJob(baseJob.WithArguments([new MsBuildArgument("/p:DefineConstants2=V1;V111")]).WithId("V1"));
        AddJob(baseJob.WithArguments([new MsBuildArgument("/p:DefineConstants2=V2;V222")]).WithId("V2"));

        AddLogger(ConsoleLogger.Default);

        AddExporter(DefaultConfig.Instance.GetExporters().ToArray());
        AddAnalyser(DefaultConfig.Instance.GetAnalysers().ToArray());
        AddColumnProvider(DefaultColumnProviders.Instance);
    }

filzrev avatar May 15 '25 11:05 filzrev