Skip to content

Commit 88c2309

Browse files
brettfobrettfo
authored
Merge pull request #14868 from dependabot/dev/brettfo/nuget-rsp
force all invocations of `dotnet msbuild` to ignore response files
2 parents 3bbd97e + 1229994 commit 88c2309

6 files changed

Lines changed: 84 additions & 13 deletions

File tree

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.Project.cs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,5 +894,65 @@ await TestDiscoveryAsync(
894894
}
895895
);
896896
}
897+
898+
[Fact]
899+
public async Task MSBuildResponseFileDoesNotCauseDiscoveryFailure()
900+
{
901+
await TestDiscoveryAsync(
902+
packages:
903+
[
904+
MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net9.0"),
905+
],
906+
workspacePath: "",
907+
files: [
908+
("myproj.csproj", """
909+
<Project Sdk="Microsoft.NET.Sdk">
910+
<PropertyGroup>
911+
<TargetFramework>net9.0</TargetFramework>
912+
</PropertyGroup>
913+
<ItemGroup>
914+
<PackageReference Include="Some.Package" />
915+
</ItemGroup>
916+
</Project>
917+
"""),
918+
("Directory.Build.props", "<Project />"),
919+
("Directory.Build.targets", "<Project />"),
920+
("Directory.Packages.props", """
921+
<Project>
922+
<PropertyGroup>
923+
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
924+
</PropertyGroup>
925+
<ItemGroup>
926+
<PackageVersion Include="Some.Package" Version="1.0.0" />
927+
</ItemGroup>
928+
</Project>
929+
"""),
930+
("Directory.Build.rsp", """
931+
/this-is-not-a-supported-switch-and-would-normally-cause-a-discovery-failure
932+
""")
933+
],
934+
expectedResult: new()
935+
{
936+
Path = "",
937+
Projects = [
938+
new()
939+
{
940+
FilePath = "myproj.csproj",
941+
Dependencies = [
942+
new("Some.Package", "1.0.0", DependencyType.PackageReference, TargetFrameworks: ["net9.0"]),
943+
],
944+
TargetFrameworks = ["net9.0"],
945+
ReferencedProjectPaths = [],
946+
ImportedFiles = [
947+
"Directory.Build.props",
948+
"Directory.Build.targets",
949+
"Directory.Packages.props",
950+
],
951+
AdditionalFiles = [],
952+
},
953+
],
954+
}
955+
);
956+
}
897957
}
898958
}

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/MockNuGetPackage.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ private static byte[] CreateAssembly(string assemblyName, string assemblyVersion
318318
</Project>
319319
"""
320320
);
321-
var (exitCode, stdout, stderr) = ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(["msbuild", projectPath, "/t:_ReportCurrentSdkVersion"], projectDir.FullName).Result;
321+
var (exitCode, stdout, stderr) = ProcessEx.RunDotnetMSBuildSafelyAsync([projectPath, "/t:_ReportCurrentSdkVersion"], projectDir.FullName).Result;
322322
if (exitCode != 0)
323323
{
324324
throw new Exception($"Failed to report the current SDK version:\n{stdout}\n{stderr}");

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public static async Task<ImmutableArray<ProjectDiscoveryResult>> DiscoverAsync(s
144144
try
145145
{
146146
// when using single restore, we can directly invoke the relevant targets...
147-
var args = new List<string>() { "msbuild", startingProjectPath };
147+
var args = new List<string>() { startingProjectPath };
148148

149149
// ...but determining what the relevant targets are can be complicated
150150

@@ -195,15 +195,15 @@ public static async Task<ImmutableArray<ProjectDiscoveryResult>> DiscoverAsync(s
195195
args.Add("/p:MSBuildTreatWarningsAsErrors=false");
196196
args.Add($"/bl:{binLogPath}");
197197

198-
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(args, startingProjectDirectory);
198+
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(args, startingProjectDirectory);
199199
if (exitCode != 0 && stdOut.Contains("error : Object reference not set to an instance of an object."))
200200
{
201201
// https://github.com/NuGet/Home/issues/11761#issuecomment-1105218996
202202
// Due to a bug in NuGet, there can be a null reference exception thrown and adding this command line argument will work around it,
203203
// but this argument can't always be added; it can cause problems in other instances, so we're taking the approach of not using it
204204
// unless we have to.
205205
args.Add("/RestoreProperty:__Unused__=__Unused__");
206-
(exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(args, startingProjectDirectory);
206+
(exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(args, startingProjectDirectory);
207207
}
208208

209209
MSBuildHelper.ThrowOnError(stdOut);

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ ILogger logger
118118
// generate project.assets.json
119119
var parsedTargetFramework = NuGetFramework.Parse(targetFramework);
120120
var tempProject = await MSBuildHelper.CreateTempProjectAsync(tempDir, repoRoot, projectPath, targetFramework, topLevelDependencies, logger, importDependencyTargets: true);
121-
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(["msbuild", tempProject, "/t:Restore,GenerateBuildDependencyFile"], tempDir.FullName);
121+
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync([tempProject, "/t:Restore,GenerateBuildDependencyFile"], tempDir.FullName);
122122
var assetsJsonPath = Path.Join(tempDir.FullName, "obj", "project.assets.json");
123123
var assetsJsonContent = await File.ReadAllTextAsync(assetsJsonPath);
124124

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,8 @@ internal static async Task<ImmutableArray<string>> GetTargetFrameworkValuesFromP
406406
var (exitCode, stdOut, stdErr) = await HandleGlobalJsonAsync(projectDirectory, repoRoot, async () =>
407407
{
408408
var targetsHelperPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "TargetFrameworkReporter.targets");
409-
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(
409+
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(
410410
[
411-
"msbuild",
412411
projectPath,
413412
"/t:ReportTargetFramework",
414413
$"/p:CustomAfterMicrosoftCommonCrossTargetingTargets={targetsHelperPath}",
@@ -524,11 +523,10 @@ public static async Task<HashSet<string>> GetProjectTargetsAsync(string projectP
524523
var projectDirectory = Path.GetDirectoryName(projectPath)!;
525524
var args = new[]
526525
{
527-
"msbuild",
528526
projectPath,
529527
"-targets"
530528
};
531-
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(args, projectDirectory);
529+
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(args, projectDirectory);
532530
if (exitCode != 0)
533531
{
534532
logger.Warn($"Unable to determine targets for project [{projectPath}]:\nSTDOUT:\n{stdOut}\nSTDERR:\n{stdErr}\n");
@@ -555,12 +553,11 @@ public static async Task<HashSet<string>> GetProjectTargetsAsync(string projectP
555553
var projectDirectory = Path.GetDirectoryName(projectPath)!;
556554
var args = new[]
557555
{
558-
"msbuild",
559556
projectPath,
560557
$"-getProperty:{propertyName}"
561558
};
562559

563-
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(args, projectDirectory);
560+
var (exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(args, projectDirectory);
564561
if (exitCode != 0)
565562
{
566563
if (stdOut.Contains("error MSB1001: Unknown switch."))
@@ -580,12 +577,11 @@ public static async Task<HashSet<string>> GetProjectTargetsAsync(string projectP
580577

581578
// do it
582579
args = [
583-
"msbuild",
584580
projectPath,
585581
$"/p:CustomAfterMicrosoftCommonTargets={tempTargetsPath}",
586582
"/t:_Dependabot_GetProperty",
587583
];
588-
(exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetWithoutMSBuildEnvironmentVariablesAsync(args, projectDirectory);
584+
(exitCode, stdOut, stdErr) = await ProcessEx.RunDotnetMSBuildSafelyAsync(args, projectDirectory);
589585
if (exitCode == 0)
590586
{
591587
var match = Regex.Match(stdOut, "__PROPERTY_VALUE:(?<PropertyValue>[^$]*)$", RegexOptions.Multiline);

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/ProcessExtensions.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,21 @@ namespace NuGetUpdater.Core;
55

66
public static class ProcessEx
77
{
8+
/// <summary>
9+
/// Run `dotnet msbuild` with the given additional arguments. The argument `-noAutoResponse` is always appended to
10+
/// suppress `Directory.Build.rsp` inclusion. This new set of arguments is then passed to the function
11+
/// `RunDotnetWithoutMSBuildEnvironmentVariablesAsync` to prevent MSBuild from inheriting certain environment
12+
/// variables.
13+
/// </summary>
14+
public static Task<(int ExitCode, string Output, string Error)> RunDotnetMSBuildSafelyAsync(
15+
IEnumerable<string> arguments,
16+
string workingDirectory,
17+
IEnumerable<(string Name, string? Value)>? extraEnvironmentVariables = null
18+
)
19+
{
20+
return RunDotnetWithoutMSBuildEnvironmentVariablesAsync(["msbuild", .. arguments, "-noAutoResponse"], workingDirectory, extraEnvironmentVariables);
21+
}
22+
823
/// <summary>
924
/// Run the `dotnet` command with the given values. This will exclude all `MSBuild*` environment variables from the execution.
1025
/// </summary>

0 commit comments

Comments
 (0)