Skip to content

Commit 7664f5d

Browse files
Copilotbaronfel
andcommitted
Add EnableDefaultItems=false optimization for restore operations
Co-authored-by: baronfel <[email protected]>
1 parent e05e92a commit 7664f5d

File tree

4 files changed

+226
-1
lines changed

4 files changed

+226
-1
lines changed

src/Cli/Microsoft.DotNet.Cli.Utils/Constants.cs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Runtime.InteropServices;
5+
46
namespace Microsoft.DotNet.Cli.Utils;
57

68
public static class Constants
@@ -29,4 +31,57 @@ public static class Constants
2931

3032
public static readonly string RestoreInteractiveOption = "--interactive";
3133
public static readonly string workloadSetVersionFileName = "workloadVersion.txt";
34+
35+
/// <summary>
36+
/// Adds performance optimizations to restore by disabling default item globbing
37+
/// if the user hasn't already specified EnableDefaultItems.
38+
/// </summary>
39+
public static IEnumerable<string> AddRestoreOptimizations(IEnumerable<string> msbuildArgs)
40+
{
41+
var args = msbuildArgs.ToList();
42+
43+
// Check if user has already specified EnableDefaultItems
44+
bool userSpecifiedEnableDefaultItems = HasUserSpecifiedProperty(args, EnableDefaultItems);
45+
46+
if (!userSpecifiedEnableDefaultItems)
47+
{
48+
// Add EnableDefaultItems=false to improve restore performance by disabling default item globbing
49+
args.Insert(0, $"-property:{EnableDefaultItems}=false");
50+
}
51+
52+
return args;
53+
}
54+
55+
/// <summary>
56+
/// Checks if the user has already specified a given property in the MSBuild arguments.
57+
/// </summary>
58+
private static bool HasUserSpecifiedProperty(IEnumerable<string> args, string propertyName)
59+
{
60+
foreach (var arg in args)
61+
{
62+
// Check for -property:PropertyName=, -p:PropertyName=, --property:PropertyName=, etc.
63+
if (arg.StartsWith("-property:", StringComparison.OrdinalIgnoreCase) ||
64+
arg.StartsWith("-p:", StringComparison.OrdinalIgnoreCase) ||
65+
arg.StartsWith("--property:", StringComparison.OrdinalIgnoreCase))
66+
{
67+
// Extract the property part after the prefix
68+
var colonIndex = arg.IndexOf(':');
69+
if (colonIndex > 0 && colonIndex < arg.Length - 1)
70+
{
71+
var propertyPart = arg.Substring(colonIndex + 1);
72+
var equalsIndex = propertyPart.IndexOf('=');
73+
if (equalsIndex > 0)
74+
{
75+
var propName = propertyPart.Substring(0, equalsIndex);
76+
if (string.Equals(propName, propertyName, StringComparison.OrdinalIgnoreCase))
77+
{
78+
return true;
79+
}
80+
}
81+
}
82+
}
83+
}
84+
85+
return false;
86+
}
3287
}

src/Cli/dotnet/Commands/Restore/RestoreCommand.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ public static CommandBase FromParseResult(ParseResult result, string? msbuildPat
4646

4747
public static MSBuildForwardingApp CreateForwarding(IEnumerable<string> msbuildArgs, string? msbuildPath = null)
4848
{
49-
var forwardingApp = new MSBuildForwardingApp(msbuildArgs, msbuildPath);
49+
var argsWithOptimizations = Constants.AddRestoreOptimizations(msbuildArgs);
50+
var forwardingApp = new MSBuildForwardingApp(argsWithOptimizations, msbuildPath);
5051
NuGetSignatureVerificationEnabler.ConditionallyEnable(forwardingApp);
5152
return forwardingApp;
5253
}

src/Cli/dotnet/Commands/Restore/RestoringCommand.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.DotNet.Cli.Commands.MSBuild;
77
using Microsoft.DotNet.Cli.Commands.Workload.Install;
88
using Microsoft.DotNet.Configurer;
9+
using Microsoft.DotNet.Cli.Utils;
910

1011
namespace Microsoft.DotNet.Cli.Commands.Restore;
1112

@@ -65,6 +66,9 @@ private static MSBuildForwardingApp GetSeparateRestoreCommand(
6566
(var newArgumentsToAdd, var existingArgumentsToForward) = ProcessForwardedArgumentsForSeparateRestore(arguments);
6667
restoreArguments = [.. restoreArguments, .. newArgumentsToAdd, .. existingArgumentsToForward];
6768

69+
// Add restore performance optimizations
70+
restoreArguments = Constants.AddRestoreOptimizations(restoreArguments);
71+
6872
return RestoreCommand.CreateForwarding(restoreArguments, msbuildPath);
6973
}
7074

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
#nullable disable
5+
6+
using Microsoft.DotNet.Cli.Commands.Restore;
7+
using Microsoft.DotNet.Cli.Utils;
8+
using Microsoft.DotNet.Cli.Commands.Build;
9+
10+
namespace Microsoft.NET.Restore.Tests
11+
{
12+
public class GivenThatWeWantRestoreOptimizations : SdkTest
13+
{
14+
public GivenThatWeWantRestoreOptimizations(ITestOutputHelper log) : base(log)
15+
{
16+
}
17+
18+
[Fact]
19+
public void It_adds_EnableDefaultItems_false_by_default()
20+
{
21+
// Arrange
22+
var originalArgs = new[] { "-target:Restore", "MyProject.csproj" };
23+
24+
// Act
25+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
26+
27+
// Assert
28+
optimizedArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false");
29+
}
30+
31+
[Fact]
32+
public void It_does_not_add_EnableDefaultItems_false_when_user_specified_EnableDefaultItems_true()
33+
{
34+
// Arrange
35+
var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=true", "MyProject.csproj" };
36+
37+
// Act
38+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
39+
40+
// Assert
41+
optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
42+
optimizedArgs.Should().Contain("-property:EnableDefaultItems=true");
43+
}
44+
45+
[Fact]
46+
public void It_does_not_add_EnableDefaultItems_false_when_user_specified_EnableDefaultItems_false()
47+
{
48+
// Arrange
49+
var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=false", "MyProject.csproj" };
50+
51+
// Act
52+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
53+
54+
// Assert
55+
var enableDefaultItemsArgs = optimizedArgs.Where(arg => arg.Contains("EnableDefaultItems")).ToList();
56+
enableDefaultItemsArgs.Should().ContainSingle();
57+
enableDefaultItemsArgs.Single().Should().Be("-property:EnableDefaultItems=false");
58+
}
59+
60+
[Fact]
61+
public void It_respects_user_EnableDefaultItems_with_short_property_syntax()
62+
{
63+
// Arrange
64+
var originalArgs = new[] { "-target:Restore", "-p:EnableDefaultItems=true", "MyProject.csproj" };
65+
66+
// Act
67+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
68+
69+
// Assert
70+
optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
71+
optimizedArgs.Should().Contain("-p:EnableDefaultItems=true");
72+
}
73+
74+
[Fact]
75+
public void It_respects_user_EnableDefaultItems_with_double_dash_syntax()
76+
{
77+
// Arrange
78+
var originalArgs = new[] { "-target:Restore", "--property:EnableDefaultItems=true", "MyProject.csproj" };
79+
80+
// Act
81+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
82+
83+
// Assert
84+
optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
85+
optimizedArgs.Should().Contain("--property:EnableDefaultItems=true");
86+
}
87+
88+
[Fact]
89+
public void It_handles_case_insensitive_property_names()
90+
{
91+
// Arrange
92+
var originalArgs = new[] { "-target:Restore", "-property:enabledefaultitems=true", "MyProject.csproj" };
93+
94+
// Act
95+
var optimizedArgs = Constants.AddRestoreOptimizations(originalArgs);
96+
97+
// Assert
98+
optimizedArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
99+
optimizedArgs.Should().Contain("-property:enabledefaultitems=true");
100+
}
101+
102+
[Fact]
103+
public void RestoreCommand_CreateForwarding_includes_optimization()
104+
{
105+
// Arrange
106+
var originalArgs = new[] { "-target:Restore", "MyProject.csproj" };
107+
108+
// Act
109+
var restoreCommand = RestoreCommand.CreateForwarding(originalArgs);
110+
111+
// Assert
112+
var msbuildArgs = restoreCommand.MSBuildArguments;
113+
msbuildArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false");
114+
}
115+
116+
[Fact]
117+
public void RestoreCommand_CreateForwarding_respects_user_EnableDefaultItems()
118+
{
119+
// Arrange
120+
var originalArgs = new[] { "-target:Restore", "-property:EnableDefaultItems=true", "MyProject.csproj" };
121+
122+
// Act
123+
var restoreCommand = RestoreCommand.CreateForwarding(originalArgs);
124+
125+
// Assert
126+
var msbuildArgs = restoreCommand.MSBuildArguments;
127+
msbuildArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
128+
msbuildArgs.Should().Contain("-property:EnableDefaultItems=true");
129+
}
130+
131+
[Fact]
132+
public void SeparateRestoreCommand_includes_optimization_when_needed()
133+
{
134+
// This tests the optimization in the separate restore command path
135+
// which is used when certain properties are excluded from the main restore
136+
137+
// Arrange - use args that trigger separate restore (like specifying TargetFramework)
138+
var args = new[] { "-f", "net6.0", "MyProject.csproj" };
139+
140+
// Act
141+
var buildCommand = (RestoringCommand)BuildCommand.FromArgs(args, "msbuildpath");
142+
143+
// Assert
144+
buildCommand.SeparateRestoreCommand.Should().NotBeNull();
145+
var restoreArgs = buildCommand.SeparateRestoreCommand.MSBuildArguments;
146+
restoreArgs.Should().Contain($"-property:{Constants.EnableDefaultItems}=false");
147+
}
148+
149+
[Fact]
150+
public void SeparateRestoreCommand_respects_user_EnableDefaultItems()
151+
{
152+
// Arrange - use args that trigger separate restore and specify EnableDefaultItems
153+
var args = new[] { "-f", "net6.0", "-property:EnableDefaultItems=true", "MyProject.csproj" };
154+
155+
// Act
156+
var buildCommand = (RestoringCommand)BuildCommand.FromArgs(args, "msbuildpath");
157+
158+
// Assert
159+
buildCommand.SeparateRestoreCommand.Should().NotBeNull();
160+
var restoreArgs = buildCommand.SeparateRestoreCommand.MSBuildArguments;
161+
restoreArgs.Should().NotContain($"-property:{Constants.EnableDefaultItems}=false");
162+
restoreArgs.Should().Contain("-property:EnableDefaultItems=true");
163+
}
164+
}
165+
}

0 commit comments

Comments
 (0)