Skip to content

Commit 7895395

Browse files
Ghost93Copilot
Ghost93
and
Copilot
committed
Fix Grpc.Net.ClientFactory version range check
Normalize NuGet version range metadata before calling VersionLessThan in the Http.Resilience buildTransitive target. Add regression coverage for bracket-pinned versions across all target inputs. Fixes #7565 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d508eee commit 7895395

2 files changed

Lines changed: 323 additions & 11 deletions

File tree

‎src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
<PropertyGroup>
33
<_GrpcNetClientFactory>Grpc.Net.ClientFactory</_GrpcNetClientFactory>
44
<_CompatibleGrpcNetClientFactoryVersion>2.64.0</_CompatibleGrpcNetClientFactoryVersion>
5+
<_GrpcNetClientFactoryComparableVersionPattern>^\s*[\[\(]?\s*(\d+(?:\.\d+){1,3})(?=\s*(?:[,)\]]|$))</_GrpcNetClientFactoryComparableVersionPattern>
56
<_GrpcNetClientFactoryVersionIsIncorrect>Grpc.Net.ClientFactory 2.63.0 or earlier could cause issues when used together with Microsoft.Extensions.Http.Resilience. For more details, see https://learn.microsoft.com/dotnet/core/resilience/http-resilience#known-issues. Consider using Grpc.Net.ClientFactory $(_CompatibleGrpcNetClientFactoryVersion) or later. To suppress the warning set SuppressCheckGrpcNetClientFactoryVersion=true.</_GrpcNetClientFactoryVersionIsIncorrect>
67
</PropertyGroup>
78

@@ -15,40 +16,47 @@
1516
Condition=" '$(SuppressCheckGrpcNetClientFactoryVersion)' != 'true' ">
1617
<ItemGroup>
1718
<!-- Find the package in the .csproj file. -->
18-
<_GrpcNetClientFactoryPackageReference Include="@(PackageReference)" Condition=" '%(PackageReference.Identity)' == '$(_GrpcNetClientFactory)' " />
19+
<_GrpcNetClientFactoryPackageReference Include="@(PackageReference)" Condition=" '%(PackageReference.Identity)' == '$(_GrpcNetClientFactory)' ">
20+
<_ComparableVersion>$([System.Text.RegularExpressions.Regex]::Match('%(PackageReference.Version)', '$(_GrpcNetClientFactoryComparableVersionPattern)').Groups[1].Value)</_ComparableVersion>
21+
<_ComparableVersionOverride>$([System.Text.RegularExpressions.Regex]::Match('%(PackageReference.VersionOverride)', '$(_GrpcNetClientFactoryComparableVersionPattern)').Groups[1].Value)</_ComparableVersionOverride>
22+
</_GrpcNetClientFactoryPackageReference>
1923

2024
<!-- Find the version of the package in the Central Package Source. The solution uses the Central Package Management. -->
21-
<_GrpcNetClientFactoryPackageVersion Include="@(PackageVersion)" Condition=" '%(PackageVersion.Identity)' == '$(_GrpcNetClientFactory)' " />
25+
<_GrpcNetClientFactoryPackageVersion Include="@(PackageVersion)" Condition=" '%(PackageVersion.Identity)' == '$(_GrpcNetClientFactory)' ">
26+
<_ComparableVersion>$([System.Text.RegularExpressions.Regex]::Match('%(PackageVersion.Version)', '$(_GrpcNetClientFactoryComparableVersionPattern)').Groups[1].Value)</_ComparableVersion>
27+
</_GrpcNetClientFactoryPackageVersion>
2228

2329
<!-- The package is added to the project as a transitive dependency. -->
24-
<_GrpcNetClientFactoryTransitiveDependency Include="@(ReferencePath)" Condition=" '%(ReferencePath.NuGetPackageId)' == '$(_GrpcNetClientFactory)' " />
30+
<_GrpcNetClientFactoryTransitiveDependency Include="@(ReferencePath)" Condition=" '%(ReferencePath.NuGetPackageId)' == '$(_GrpcNetClientFactory)' ">
31+
<_ComparableVersion>$([System.Text.RegularExpressions.Regex]::Match('%(ReferencePath.NuGetPackageVersion)', '$(_GrpcNetClientFactoryComparableVersionPattern)').Groups[1].Value)</_ComparableVersion>
32+
</_GrpcNetClientFactoryTransitiveDependency>
2533
</ItemGroup>
2634

2735
<!-- The version of the package is included in the .csproj file. -->
2836
<Warning Condition=" @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
29-
AND '%(_GrpcNetClientFactoryPackageReference.Version)' != ''
30-
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference.Version)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
37+
AND '%(_GrpcNetClientFactoryPackageReference._ComparableVersion)' != ''
38+
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference._ComparableVersion)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
3139
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />
3240

3341
<!-- The solution uses the Central Package Management and the version of the package is overridden in the .csproj file using the VersionOverride property. -->
3442
<Warning Condition=" '$(ManagePackageVersionsCentrally)' == 'true'
3543
AND @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
36-
AND '%(_GrpcNetClientFactoryPackageReference.VersionOverride)' != ''
37-
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference.VersionOverride)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
44+
AND '%(_GrpcNetClientFactoryPackageReference._ComparableVersionOverride)' != ''
45+
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference._ComparableVersionOverride)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
3846
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />
3947

4048
<!-- The solution uses the Central Package Management and the version of the package is included in the Central Package Source. -->
4149
<Warning Condition=" '$(ManagePackageVersionsCentrally)' == 'true'
4250
AND @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
43-
AND '%(_GrpcNetClientFactoryPackageVersion.Version)' != ''
44-
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageVersion.Version)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
51+
AND '%(_GrpcNetClientFactoryPackageVersion._ComparableVersion)' != ''
52+
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageVersion._ComparableVersion)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
4553
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />
4654

4755
<!-- This condition handles a case when the package is added to the project as a transitive dependency. -->
4856
<Warning Condition=" @(_GrpcNetClientFactoryPackageReference->Count()) == 0
4957
AND @(_GrpcNetClientFactoryTransitiveDependency->Count()) &gt; 0
50-
AND '%(_GrpcNetClientFactoryTransitiveDependency.NuGetPackageVersion)' != ''
51-
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryTransitiveDependency.NuGetPackageVersion)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
58+
AND '%(_GrpcNetClientFactoryTransitiveDependency._ComparableVersion)' != ''
59+
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryTransitiveDependency._ComparableVersion)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
5260
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />
5361
</Target>
5462
</Project>

‎test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/BuildTransitive/GrpcNetClientFactoryVersionTargetTests.cs‎

Lines changed: 304 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,304 @@
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+
using System;
5+
using System.Diagnostics;
6+
using System.IO;
7+
using System.Runtime.InteropServices;
8+
using System.Security;
9+
using System.Text;
10+
using System.Threading.Tasks;
11+
using FluentAssertions;
12+
using Xunit;
13+
14+
namespace Microsoft.Extensions.Http.Resilience.Test.BuildTransitive;
15+
16+
public class GrpcNetClientFactoryVersionTargetTests
17+
{
18+
private const string WarningMessage = "Grpc.Net.ClientFactory 2.63.0 or earlier could cause issues";
19+
20+
public static TheoryData<string, string> CompatibleVersionSources => new()
21+
{
22+
{
23+
"PackageReference.Version",
24+
"""
25+
<ItemGroup>
26+
<PackageReference Include="Grpc.Net.ClientFactory" Version="[2.80.0]" />
27+
</ItemGroup>
28+
"""
29+
},
30+
{
31+
"PackageReference.VersionOverride",
32+
"""
33+
<PropertyGroup>
34+
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
35+
</PropertyGroup>
36+
<ItemGroup>
37+
<PackageReference Include="Grpc.Net.ClientFactory" VersionOverride="[2.80.0]" />
38+
</ItemGroup>
39+
"""
40+
},
41+
{
42+
"PackageVersion.Version",
43+
"""
44+
<PropertyGroup>
45+
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
46+
</PropertyGroup>
47+
<ItemGroup>
48+
<PackageReference Include="Grpc.Net.ClientFactory" />
49+
<PackageVersion Include="Grpc.Net.ClientFactory" Version="[2.80.0]" />
50+
</ItemGroup>
51+
"""
52+
},
53+
{
54+
"ReferencePath.NuGetPackageVersion",
55+
"""
56+
<ItemGroup>
57+
<ReferencePath Include="Grpc.Net.ClientFactory.dll" NuGetPackageId="Grpc.Net.ClientFactory" NuGetPackageVersion="[2.80.0]" />
58+
</ItemGroup>
59+
"""
60+
},
61+
};
62+
63+
public static TheoryData<string, string> IncompatibleVersionSources => new()
64+
{
65+
{
66+
"PackageReference.Version",
67+
"""
68+
<ItemGroup>
69+
<PackageReference Include="Grpc.Net.ClientFactory" Version="[2.63.0]" />
70+
</ItemGroup>
71+
"""
72+
},
73+
{
74+
"PackageReference.VersionOverride",
75+
"""
76+
<PropertyGroup>
77+
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
78+
</PropertyGroup>
79+
<ItemGroup>
80+
<PackageReference Include="Grpc.Net.ClientFactory" VersionOverride="[2.63.0]" />
81+
</ItemGroup>
82+
"""
83+
},
84+
{
85+
"PackageVersion.Version",
86+
"""
87+
<PropertyGroup>
88+
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
89+
</PropertyGroup>
90+
<ItemGroup>
91+
<PackageReference Include="Grpc.Net.ClientFactory" />
92+
<PackageVersion Include="Grpc.Net.ClientFactory" Version="[2.63.0]" />
93+
</ItemGroup>
94+
"""
95+
},
96+
{
97+
"ReferencePath.NuGetPackageVersion",
98+
"""
99+
<ItemGroup>
100+
<ReferencePath Include="Grpc.Net.ClientFactory.dll" NuGetPackageId="Grpc.Net.ClientFactory" NuGetPackageVersion="[2.63.0]" />
101+
</ItemGroup>
102+
"""
103+
},
104+
};
105+
106+
[Theory]
107+
[MemberData(nameof(CompatibleVersionSources))]
108+
public async Task CheckGrpcNetClientFactoryVersion_CompatibleBracketPinnedVersion_DoesNotWarnOrFail(
109+
string scenario,
110+
string projectItems)
111+
{
112+
var result = await RunTargetAsync(projectItems);
113+
114+
result.ExitCode.Should().Be(0, result.ToString());
115+
result.Output.Should().NotContain("MSB4184", scenario);
116+
result.Output.Should().NotContain(WarningMessage, scenario);
117+
}
118+
119+
[Theory]
120+
[MemberData(nameof(IncompatibleVersionSources))]
121+
public async Task CheckGrpcNetClientFactoryVersion_IncompatibleBracketPinnedVersion_Warns(
122+
string scenario,
123+
string projectItems)
124+
{
125+
var result = await RunTargetAsync(projectItems);
126+
127+
result.ExitCode.Should().Be(0, result.ToString());
128+
result.Output.Should().NotContain("MSB4184", scenario);
129+
result.Output.Should().Contain(WarningMessage, scenario);
130+
}
131+
132+
[Theory]
133+
[InlineData("(,2.80.0]")]
134+
[InlineData("[2.64.0-preview.1]")]
135+
public async Task CheckGrpcNetClientFactoryVersion_VersionWithoutComparableSystemVersion_DoesNotFail(string version)
136+
{
137+
var result = await RunTargetAsync($"""
138+
<ItemGroup>
139+
<PackageReference Include="Grpc.Net.ClientFactory" Version="{version}" />
140+
</ItemGroup>
141+
""");
142+
143+
result.ExitCode.Should().Be(0, result.ToString());
144+
result.Output.Should().NotContain("MSB4184");
145+
result.Output.Should().NotContain(WarningMessage);
146+
}
147+
148+
private static async Task<CommandResult> RunTargetAsync(string projectItems)
149+
{
150+
var tempDirectory = Path.Combine(Path.GetTempPath(), $"GrpcNetClientFactoryTargetTests_{Guid.NewGuid():N}");
151+
Directory.CreateDirectory(tempDirectory);
152+
153+
try
154+
{
155+
var projectPath = Path.Combine(tempDirectory, "test.proj");
156+
var project = $"""
157+
<Project>
158+
<Import Project="{EscapeXml(GetTargetPath())}" />
159+
{projectItems}
160+
</Project>
161+
""";
162+
163+
File.WriteAllText(projectPath, project);
164+
165+
return await RunDotNetAsync(tempDirectory, "msbuild", projectPath, "-nologo", "-v:minimal", "-t:_CheckGrpcNetClientFactoryVersion").ConfigureAwait(false);
166+
}
167+
finally
168+
{
169+
Directory.Delete(tempDirectory, recursive: true);
170+
}
171+
}
172+
173+
private static async Task<CommandResult> RunDotNetAsync(string workingDirectory, params string[] arguments)
174+
{
175+
var processStartInfo = new ProcessStartInfo(GetDotNetPath())
176+
{
177+
WorkingDirectory = workingDirectory,
178+
RedirectStandardOutput = true,
179+
RedirectStandardError = true,
180+
UseShellExecute = false,
181+
CreateNoWindow = true,
182+
Arguments = CreateArguments(arguments),
183+
};
184+
185+
using var process = Process.Start(processStartInfo) ?? throw new InvalidOperationException("Failed to start dotnet.");
186+
187+
var standardOutputTask = process.StandardOutput.ReadToEndAsync();
188+
var standardErrorTask = process.StandardError.ReadToEndAsync();
189+
190+
if (!process.WaitForExit((int)TimeSpan.FromSeconds(30).TotalMilliseconds))
191+
{
192+
process.Kill();
193+
throw new TimeoutException("Timed out while running dotnet msbuild.");
194+
}
195+
196+
var standardOutput = await standardOutputTask.ConfigureAwait(false);
197+
var standardError = await standardErrorTask.ConfigureAwait(false);
198+
199+
return new CommandResult(process.ExitCode, standardOutput, standardError);
200+
}
201+
202+
private static string CreateArguments(params string[] arguments)
203+
{
204+
return string.Join(" ", Array.ConvertAll(arguments, QuoteArgument));
205+
}
206+
207+
private static string QuoteArgument(string argument)
208+
{
209+
var quoted = new StringBuilder();
210+
quoted.Append('"');
211+
212+
var backslashCount = 0;
213+
foreach (var character in argument)
214+
{
215+
if (character == '\\')
216+
{
217+
backslashCount++;
218+
}
219+
else if (character == '"')
220+
{
221+
quoted.Append('\\', (backslashCount * 2) + 1);
222+
quoted.Append('"');
223+
backslashCount = 0;
224+
}
225+
else
226+
{
227+
quoted.Append('\\', backslashCount);
228+
quoted.Append(character);
229+
backslashCount = 0;
230+
}
231+
}
232+
233+
quoted.Append('\\', backslashCount * 2);
234+
quoted.Append('"');
235+
236+
return quoted.ToString();
237+
}
238+
239+
private static string GetTargetPath()
240+
{
241+
var repoRoot = GetRepoRoot();
242+
return Path.Combine(
243+
repoRoot,
244+
"src",
245+
"Libraries",
246+
"Microsoft.Extensions.Http.Resilience",
247+
"buildTransitive",
248+
"Microsoft.Extensions.Http.Resilience.targets");
249+
}
250+
251+
private static string GetDotNetPath()
252+
{
253+
var dotnetFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "dotnet.exe" : "dotnet";
254+
var repoDotnetPath = Path.Combine(GetRepoRoot(), ".dotnet", dotnetFileName);
255+
256+
return File.Exists(repoDotnetPath) ? repoDotnetPath : dotnetFileName;
257+
}
258+
259+
private static string GetRepoRoot()
260+
{
261+
var directory = new DirectoryInfo(AppContext.BaseDirectory);
262+
263+
while (directory is not null)
264+
{
265+
var targetPath = Path.Combine(
266+
directory.FullName,
267+
"src",
268+
"Libraries",
269+
"Microsoft.Extensions.Http.Resilience",
270+
"buildTransitive",
271+
"Microsoft.Extensions.Http.Resilience.targets");
272+
273+
if (File.Exists(targetPath))
274+
{
275+
return directory.FullName;
276+
}
277+
278+
directory = directory.Parent;
279+
}
280+
281+
throw new InvalidOperationException("Failed to locate the repository root.");
282+
}
283+
284+
private static string EscapeXml(string value)
285+
{
286+
return SecurityElement.Escape(value) ?? string.Empty;
287+
}
288+
289+
private sealed record CommandResult(int ExitCode, string StandardOutput, string StandardError)
290+
{
291+
public string Output => StandardOutput + StandardError;
292+
293+
public override string ToString()
294+
{
295+
return $"""
296+
Exit code: {ExitCode}
297+
Standard output:
298+
{StandardOutput}
299+
Standard error:
300+
{StandardError}
301+
""";
302+
}
303+
}
304+
}

0 commit comments

Comments
 (0)