diff --git a/src/Cli/Microsoft.DotNet.Cli.Utils/Constants.cs b/src/Cli/Microsoft.DotNet.Cli.Utils/Constants.cs index b98e32204600..c8137f9272c1 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Utils/Constants.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Utils/Constants.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Runtime.InteropServices; + namespace Microsoft.DotNet.Cli.Utils; public static class Constants @@ -29,4 +31,57 @@ public static class Constants public static readonly string RestoreInteractiveOption = "--interactive"; public static readonly string workloadSetVersionFileName = "workloadVersion.txt"; + + /// + /// Adds performance optimizations to restore by disabling default item globbing + /// if the user hasn't already specified EnableDefaultItems. + /// + public static IEnumerable AddRestoreOptimizations(IEnumerable msbuildArgs) + { + var args = msbuildArgs.ToList(); + + // Check if user has already specified EnableDefaultItems + bool userSpecifiedEnableDefaultItems = HasUserSpecifiedProperty(args, EnableDefaultItems); + + if (!userSpecifiedEnableDefaultItems) + { + // Add EnableDefaultItems=false to improve restore performance by disabling default item globbing + args.Insert(0, $"-property:{EnableDefaultItems}=false"); + } + + return args; + } + + /// + /// Checks if the user has already specified a given property in the MSBuild arguments. + /// + private static bool HasUserSpecifiedProperty(IEnumerable args, string propertyName) + { + foreach (var arg in args) + { + // Check for -property:PropertyName=, -p:PropertyName=, --property:PropertyName=, etc. + if (arg.StartsWith("-property:", StringComparison.OrdinalIgnoreCase) || + arg.StartsWith("-p:", StringComparison.OrdinalIgnoreCase) || + arg.StartsWith("--property:", StringComparison.OrdinalIgnoreCase)) + { + // Extract the property part after the prefix + var colonIndex = arg.IndexOf(':'); + if (colonIndex > 0 && colonIndex < arg.Length - 1) + { + var propertyPart = arg.Substring(colonIndex + 1); + var equalsIndex = propertyPart.IndexOf('='); + if (equalsIndex > 0) + { + var propName = propertyPart.Substring(0, equalsIndex); + if (string.Equals(propName, propertyName, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } + } + } + + return false; + } } diff --git a/src/Cli/dotnet/Commands/Restore/RestoreCommand.cs b/src/Cli/dotnet/Commands/Restore/RestoreCommand.cs index f7251959888f..6020ce6eb9fd 100644 --- a/src/Cli/dotnet/Commands/Restore/RestoreCommand.cs +++ b/src/Cli/dotnet/Commands/Restore/RestoreCommand.cs @@ -46,7 +46,8 @@ public static CommandBase FromParseResult(ParseResult result, string? msbuildPat public static MSBuildForwardingApp CreateForwarding(IEnumerable msbuildArgs, string? msbuildPath = null) { - var forwardingApp = new MSBuildForwardingApp(msbuildArgs, msbuildPath); + var argsWithOptimizations = Constants.AddRestoreOptimizations(msbuildArgs); + var forwardingApp = new MSBuildForwardingApp(argsWithOptimizations, msbuildPath); NuGetSignatureVerificationEnabler.ConditionallyEnable(forwardingApp); return forwardingApp; } diff --git a/src/Cli/dotnet/Commands/Restore/RestoringCommand.cs b/src/Cli/dotnet/Commands/Restore/RestoringCommand.cs index 69febb31fd32..557709859863 100644 --- a/src/Cli/dotnet/Commands/Restore/RestoringCommand.cs +++ b/src/Cli/dotnet/Commands/Restore/RestoringCommand.cs @@ -6,6 +6,7 @@ using Microsoft.DotNet.Cli.Commands.MSBuild; using Microsoft.DotNet.Cli.Commands.Workload.Install; using Microsoft.DotNet.Configurer; +using Microsoft.DotNet.Cli.Utils; namespace Microsoft.DotNet.Cli.Commands.Restore; @@ -65,6 +66,9 @@ private static MSBuildForwardingApp GetSeparateRestoreCommand( (var newArgumentsToAdd, var existingArgumentsToForward) = ProcessForwardedArgumentsForSeparateRestore(arguments); restoreArguments = [.. restoreArguments, .. newArgumentsToAdd, .. existingArgumentsToForward]; + // Add restore performance optimizations + restoreArguments = Constants.AddRestoreOptimizations(restoreArguments); + return RestoreCommand.CreateForwarding(restoreArguments, msbuildPath); } diff --git a/test/Microsoft.NET.Restore.Tests/GivenThatWeWantRestoreOptimizations.cs b/test/Microsoft.NET.Restore.Tests/GivenThatWeWantRestoreOptimizations.cs new file mode 100644 index 000000000000..285bd521e8d3 --- /dev/null +++ b/test/Microsoft.NET.Restore.Tests/GivenThatWeWantRestoreOptimizations.cs @@ -0,0 +1,186 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable disable + +using Microsoft.DotNet.Cli.Commands.Restore; +using Microsoft.DotNet.Cli.Utils; +using Microsoft.DotNet.Cli.Commands.Build; + +namespace Microsoft.NET.Restore.Tests +{ + public class GivenThatWeWantRestoreOptimizations : SdkTest + { + public GivenThatWeWantRestoreOptimizations(ITestOutputHelper log) : base(log) + { + } + + [Fact] + public void It_adds_EnableDefaultItems_false_by_default() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + optimizedArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false"); + } + + [Fact] + public void It_does_not_add_EnableDefaultItems_false_when_user_specified_EnableDefaultItems_true() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=true", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + optimizedArgs.Should().Contain("-property:EnableDefaultItems=true"); + } + + [Fact] + public void It_does_not_add_EnableDefaultItems_false_when_user_specified_EnableDefaultItems_false() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=false", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + var enableDefaultItemsArgs = optimizedArgs.Where(arg => arg.Contains("EnableDefaultItems")).ToList(); + enableDefaultItemsArgs.Should().ContainSingle(); + enableDefaultItemsArgs.Single().Should().Be("-property:EnableDefaultItems=false"); + } + + [Fact] + public void It_respects_user_EnableDefaultItems_with_short_property_syntax() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "-p:EnableDefaultItems=true", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + optimizedArgs.Should().Contain("-p:EnableDefaultItems=true"); + } + + [Fact] + public void It_respects_user_EnableDefaultItems_with_double_dash_syntax() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "--property:EnableDefaultItems=true", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + optimizedArgs.Should().Contain("--property:EnableDefaultItems=true"); + } + + [Fact] + public void It_handles_case_insensitive_property_names() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "-property:enabledefaultitems=true", "MyProject.csproj" }; + + // Act + var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs); + + // Assert + optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + optimizedArgs.Should().Contain("-property:enabledefaultitems=true"); + } + + [Fact] + public void RestoreCommand_CreateForwarding_includes_optimization() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "MyProject.csproj" }; + + // Act + var restoreCommand = RestoreCommand.CreateForwarding(originalArgs); + + // Assert + var msbuildArgs = restoreCommand.MSBuildArguments; + msbuildArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false"); + } + + [Fact] + public void RestoreCommand_CreateForwarding_respects_user_EnableDefaultItems() + { + // Arrange + var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=true", "MyProject.csproj" }; + + // Act + var restoreCommand = RestoreCommand.CreateForwarding(originalArgs); + + // Assert + var msbuildArgs = restoreCommand.MSBuildArguments; + msbuildArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + msbuildArgs.Should().Contain("-property:EnableDefaultItems=true"); + } + + [Fact] + public void SeparateRestoreCommand_includes_optimization_when_needed() + { + // This tests the optimization in the separate restore command path + // which is used when certain properties are excluded from the main restore + + // Arrange - use args that trigger separate restore (like specifying TargetFramework) + var args = new[] { "-f", "net6.0", "MyProject.csproj" }; + + // Act + var buildCommand = (RestoringCommand)BuildCommand.FromArgs(args, "msbuildpath"); + + // Assert + buildCommand.SeparateRestoreCommand.Should().NotBeNull(); + var restoreArgs = buildCommand.SeparateRestoreCommand.MSBuildArguments; + restoreArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false"); + } + + [Fact] + public void SeparateRestoreCommand_respects_user_EnableDefaultItems() + { + // Arrange - use args that trigger separate restore and specify EnableDefaultItems + var args = new[] { "-f", "net6.0", "-property:EnableDefaultItems=true", "MyProject.csproj" }; + + // Act + var buildCommand = (RestoringCommand)BuildCommand.FromArgs(args, "msbuildpath"); + + // Assert + buildCommand.SeparateRestoreCommand.Should().NotBeNull(); + var restoreArgs = buildCommand.SeparateRestoreCommand.MSBuildArguments; + restoreArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false"); + restoreArgs.Should().Contain("-property:EnableDefaultItems=true"); + } + + [Fact] + public void Optimization_only_affects_restore_operations() + { + // This test verifies that the optimization is only applied in restore contexts + // and doesn't affect other MSBuild operations + + // The optimization should only be applied in: + // 1. RestoreCommand.CreateForwarding (explicit restore) + // 2. RestoringCommand.GetSeparateRestoreCommand (implicit restore) + // + // It should NOT be applied to regular build commands that don't involve restore + + // For now, this is verified by code inspection since our optimization is only + // applied in restore-specific code paths. If in the future other commands + // start using Constants.AddRestoreOptimizations, we would need to ensure + // they only do so in restore contexts. + + // This test serves as documentation of the intended scope + Assert.True(true, "Optimization scope is currently limited to restore commands by design"); + } + } +} \ No newline at end of file