Static Graph RestoreEx: NuGet.Build.Tasks.Console.exe crash with wildcards and UNC paths #9405
Comments
|
Rainer thinks this is an MSBuild, rather than NuGet, bug. It's happening in static graph restore because that sets MSBuild config to disable glob expansion, and thus passes the * into the normalize method. That should have the same behavior as elsewhere. |
|
I GOT THE MINIMAL REPRO!!!! Just build this bad boy with
|
|
Since the actual exception happens in NuGet.Build.Tasks.Console.exe, I found it's easier to debug this by starting the .exe directly:
and passing it these args: Have to start it under debugger, of course, for example by opening the .exe as a project in Visual Studio, editing the project properties to pass the argument string, and then pressing F5. Remember to disable Just My Code and enable all first-chance exceptions. |
|
Just to be sure - the expected behavior is for the attempted glob expansion to silently fail and |
Because @jeffkl has been around the block a time or two (complimentary) you can also set an environment variable
Sort of; in this case glob expansion is disabled via I think I see the fix. |
|
Hm. Well. Almost! diff --git a/src/Build/Evaluation/LazyItemEvaluator.cs b/src/Build/Evaluation/LazyItemEvaluator.cs
index bd34997b83..4fa4f02291 100644
--- a/src/Build/Evaluation/LazyItemEvaluator.cs
+++ b/src/Build/Evaluation/LazyItemEvaluator.cs
@@ -444,7 +444,7 @@ namespace Microsoft.Build.Evaluation
{
for (int i = 0; i < items.Count; i++)
{
- string fullPath = FileUtilities.GetFullPath(items[i].Item.EvaluatedIncludeEscaped, items[i].Item.ProjectDirectory);
+ string fullPath = FileUtilities.NormalizePathForComparisonNoThrow(items[i].Item.EvaluatedIncludeEscaped, items[i].Item.ProjectDirectory);
if (itemsWithNoWildcards.TryGetValue(fullPath, out UpdateOperation op))
{
items[i] = op.UpdateItem(items[i]);Gets past the crash but still fails with |
|
@KirillOsenkov to work around in your original scenario can you condition the globbed item on |
This can be worked around with ❯ artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\amd64\MSBuild.exe S:\repro\dotnet\msbuild\issues\9405\fail.csproj -t:restore -p:SetLinkMetadataAutomatically=false
Restore complete (1.6s)
Build succeeded in 2.1s |
|
Do we have a documented way for someone to generate that Link metadata on-demand if they need it? One thing I see with a lot of our automatic behavior toggles is that what users have to do to recreate that automatic behavior is not always documented or clear. |
|
"Bake it into the project like VS used to" . . . but yeah I don't think there are docs. |
|
Thanks Rainer!
I'd say let's just ship that diff you've pasted above and I'll work around the rest of it in my solution. Food for thought is that this is another cursed case of |
Yes! #7029 added a message about this, that can be elevated to a break via |
An Item `Update` operation may apply to items that are defined via wildcard. In normal operation, the wildcard will have been expanded by the time Update applies, and it's fine for Update to assume that everything is a valid file path. But in `MSBuildSkipEagerWildCardEvaluationRegexes` mode, wildcards may not be expanded. Fixes dotnet#9405 by using a more appropriate method to normalize probably-but-not-necessarily paths for comparisons.
works for me! #9409 |
rainersigwald
added
bug
Feature - Globbing
Area: Engine
I have a situation where normal restore works fine, but switching to static graph restore crashes MSBuild with:
The UNC path should be of the form \\server\share.The specific process that crashes is
NuGet.Build.Tasks.Console.exewith the following stack:The actual problem is this call to
Path.GetFullPath:msbuild/src/Build/Evaluation/LazyItemEvaluator.cs
Line 447 in c36a54e
It is being passed the value of
\\csc.*, which results in GetFullPath throwing an ArgumentException:The UNC path should be of the form \\server\share.Unfortunately I've failed to isolate a standalone repro for this exact scenario. I do have an internal repo at commit
bd36c7032f0950eeca634d7aa34fcdf441e89ed9that's easy to reproduce this on. Just contact me on Teams and we can take a look together.However I did build a repro that hits a very similar case, with slightly different behavior. It no longer crashes, but fails the build with this error:
To repro, create this fail.csproj file:
and build with
msbuild /t:Restore fail.csproj.In this repro, the crash happens here:
msbuild/src/Build/Evaluation/LazyItemEvaluator.cs
Line 429 in c36a54e
There's a good try/catch around this callstack, so it doesn't bring down the process, because this catch block catches it:
msbuild/src/Shared/Modifiers.cs
Line 602 in c36a54e
There's another catch block here:
msbuild/src/Build/Evaluation/Expander.cs
Line 982 in c36a54e
However in my actual case, there isn't such a good catch block around this codepath:
msbuild/src/Build/Evaluation/LazyItemEvaluator.cs
Line 418 in c36a54e
I wasn't able to recreate a standalone repro which takes that path (line 418) instead of the regular path (line 429). Ping me on Teams for a real-life repro of the actual issue.
I suspect the problem is that wildcards are not expected to be in the item spec
\\csc.*but they are, and the item spec starts with\\because the $(Pkg*) property evaluates to an empty string at the time of restore eval, and thenPath.GetFullPath()crashes on it, but there's no good catch block to catch it around line 418, so it is more fatal.The text was updated successfully, but these errors were encountered: