Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wildcard expansion is silently disabled when a glob expansion encounters an I/O exception #406

Open
rainersigwald opened this issue Dec 16, 2015 · 34 comments
Labels
Breaking Change bug Feature - Globbing Feature: Warning Waves Warnings to enable in opt-in waves. Formerly "strict mode". User Experience

Comments

@rainersigwald
Copy link
Member

This was reported as a Connect issue.

The repro is simple:

<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <ItemGroup>
        <MyItem Include="MyDir\**\*.*" Exclude="MyDir\node_modules\**\*.*;MyDir\tmp\**\*.*" />
    </ItemGroup>

    <Target Name="TestInputs">
        <Warning Text="Inputs = @(MyItem)" />
    </Target>
</Project>

Where there are files in MyDir\node_modules that exceed MAX_PATH. That returns:

C:\Users\raines\Downloads>msbuild test.proj
Microsoft (R) Build Engine version 14.0.24720.0
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 12/16/2015 12:46:15 PM.
Project "C:\Users\raines\Downloads\test.proj" on node 1 (default targets).
C:\Users\raines\Downloads\test.proj(8,3): warning : Inputs = MyDir\**\*.*
Done Building Project "C:\Users\raines\Downloads\test.proj" (default targets).

Build succeeded.

Note that the wildcards weren't actually expanded.

The original issue mentions an expectation that the Exclude element would prevent this from happening because that directory and all its contents are excluded. The reason that doesn't work is because we build the Include list fully first, then build the Exclude list, then do a subtraction. The failure occurs when building the Include list, so Exclude has no effect.

@rainersigwald
Copy link
Member Author

There are a couple of questions here.

  1. We have (until we figure out MSBuild should handle paths longer than MAX_PATH #53 for full-framework MSBuild) nice errors that we are supposed to emit when something goes over MAX_PATH. Why didn't they fire here?
  2. Should we precompute the impact of the exclude list as this user seems to expect?

For 2, I think the answer is "no". Being able to do pattern-based set exclusions seems like an awful lot of work for fairly minimal gain.

@shift-evgeny
Copy link

Thanks for troubleshooting this! You're right about my expectations. At minimum MSBuild should report a meaningful error here, but yes, I do expect that excluding the directory will avoid any problems with it.

"Precomputing the impact of the exclude list" is an implementation detail. From the user point of view, the ItemList is "everything in MyDir, except node_modules" - same as manually listing every other sub-directory in the include list (which works).

Yes, it is a problem with NPM that it builds a directory structure that exceeds MAX_PATH, but I have no control over that. All I can do is exclude the directory.

@rainersigwald
Copy link
Member Author

Thinking about it a bit more, it may not be that difficult to consider the Exclude list inline with Include, but I haven't looked at that corner of the code in a while. If it's easy we should do it.

If it's not easy, I think fixing the long-path behavior would be sufficient to avoid this problem--it's inefficient to add and then remove things from a list but would produce the desired behavior.

@rainersigwald
Copy link
Member Author

@shift-evgeny Exclude should now behave as you expect, thanks to improvements made for lazy item evaluation (#770). MSBuild now considers the exclude list while recursing through the include list and will stop when it hits an excluded directory.

This should already be available in VS "15" Preview 5.

I'm not closing this issue because the bad doesn't-expand-wildcards behavior persists if you don't have an exclude. But at least the obvious workaround for node_modules works now.

@Daniel15
Copy link

Daniel15 commented May 10, 2017

The built-in EmbeddedResource stuff totally breaks because of this because it uses a wildcard like **/*.resx in %ProgramFiles%\Microsoft Visual Studio\2017\community\msbuild\Sdks\Microsoft.NET.Sdk\build\Microsoft.NET.Sdk.DefaultItems.props:

<EmbeddedResource Include="**/*.resx" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" Condition=" '$(EnableDefaultEmbeddedResourceItems)' == 'true' " />

Resulting in the GenerateResource task throwing this error:

Microsoft.Common.CurrentVersion.targets(2867,5): error MSB3552: Resource file "**/*.resx" cannot be found.

https://github.com/dotnet/cli/issues/6561

If it's difficult to properly fix this, could you please at least update the resx stuff in Microsoft.Common.CurrentVersion.targets so it doesn't fail due to this problem?

Daniel15 added a commit to reactjs/React.NET that referenced this issue May 11, 2017
Daniel15 added a commit to reactjs/React.NET that referenced this issue May 11, 2017
* Upgrade to Visual Studio 2017 + csproj tooling

* Include workaround for NuGet/Home#4337 (explicitly specify version when restoring NuGet packages)

* Work around dotnet/cli-migrate#11

* Upgrade to .NET Core 1.1.x

* Remove VersionPrefix from all csproj files

* Legacy NuGet restore is no longer needed

* Disable CS1701 warning for sample projects

* Turns out we actually do need the legacy NuGet restore

* Use VS2017 AppVeyor image

* Remove project dependencies to work around NuGet/Home#5193 and NuGet/Home#4578

* Use MSBuild 15 on AppVeyor

* Enforce .NET Core tools 1.0.0 in global.json

* Use newer npm on AppVeyor (as a workaround for https://github.com/dotnet/cli/issues/6561 and dotnet/msbuild#406), and run correct test command
@RehanSaeed
Copy link

RehanSaeed commented Jul 2, 2017

Just started getting this error from Travis CI here. My path doesn't even seem that long.

/usr/share/dotnet/sdk/1.0.1/Microsoft.Common.CurrentVersion.targets(2865,5): error MSB3552: Resource file "**/*.resx" cannot be found. [/home/travis/build/RehanSaeed/Schema.NET/Source/Schema.NET/Schema.NET.csproj]

I guess my solution is to somehow tell Travis CI to put my code in folder at a lower level?

@Daniel15
Copy link

Daniel15 commented Jul 2, 2017 via email

@RehanSaeed
Copy link

Not using npm, just a simple class library.

@Daniel15
Copy link

Daniel15 commented Jul 2, 2017

Try running find . in your build script (which will recursively list all the files in the directory) and see if there's any very long paths. A long path anywhere in the directory will break the wildcard expansion.

@RehanSaeed
Copy link

@Daniel15 Thanks for the suggestion. Just tried that and this is what I found:

The longest path from the project causing the build error is 110 characters:

/home/travis/build/RehanSaeed/Schema.NET/Source/Schema.NET/health-lifesci/MedicalGuidelineContraindication.cs

The longest path from another project in the solution is 118 characters:

/home/travis/build/RehanSaeed/Schema.NET/Source/Schema.NET.Tool/Overrides/AddNumberTypeToMediaObjectHeightAndWidth.cs

These paths are well short of the 260 character max path, unless I'm missing something about Linux or MacOS where I'm running the dotnet build command.

@cellvia
Copy link

cellvia commented Sep 8, 2017

i am totally blocked by this. I am also on osx and having the same results as @RehanSaeed
looks like msbuild / dotnet core are not actually ready for unix :-/

@cellvia
Copy link

cellvia commented Sep 8, 2017

@RehanSaeed for what its worth, it seems like some issue with the hidden keys folder thats generated when launching debug session.

to solve this: i created a keys folder (not hidden), and then within that a file with the same filename as the one that had been generated in the hidden .keys folder. i left the file empty.

now every time i build / launch it works fine...

@rashidovich
Copy link

VM Ubuntu 17.04
.net core 2.0

dotnet build throws same issue.

basically i cannot deploy to production.
on local machine (OSX) builds fine.

i don't see any '.keys' folder.

Thanks in advance.

dustinsoftware added a commit to dustinsoftware/React.NET that referenced this issue Oct 11, 2017
This fixes building within VS 2017 community.
node_modules can sometimes contain paths that are very long, which can
trigger an msbuild bug

dotnet/msbuild#406
Daniel15 pushed a commit to reactjs/React.NET that referenced this issue Oct 19, 2017
This fixes building within VS 2017 community.
node_modules can sometimes contain paths that are very long, which can
trigger an msbuild bug

dotnet/msbuild#406
@dasjestyr
Copy link

Having same issue on both osx and ubuntu. Getting the latest version of npm did not help. Moving the project to root in order to shorten the path also did not work. No keys file either. Won't build locally or as a docker build on osx or ubuntu, but builds fine in a windows environment both locally and as docker build.

@radical
Copy link
Member

radical commented Dec 21, 2017

Can you try running find . -name '*\\*' in your project directory?

@dasjestyr
Copy link

@radical not sure if you were talking to me, but if I run it, it just returns a single file which is an NServiceBus log file.

@radical
Copy link
Member

radical commented Dec 21, 2017

@dasjestyr I hit this issue too and have a workaround. The problem is that the wildcard expansion is skipped if there is any error while trying to expand the wildcard. And the \ in the path here gets converted to / on !windows and that causes a failure later. My workaround really just skips such a directory, and so doesn't really solve the issue. https://gist.github.com/radical/27ab744e235badd5f7b24dbfc85ec9af
I will try to look at a proper solution later. But anyone else should feel free to take a stab at it meanwhile :)

@radical
Copy link
Member

radical commented Dec 21, 2017

@dasjestyr um btw, was it a file or a directory for you? My patch might need to be fixed to handle files too. But this is a hack anyway.

@livarcocc livarcocc added this to the MSBuild 16.4 milestone Aug 22, 2019
@livarcocc livarcocc modified the milestones: MSBuild 16.4, MSBuild 16.5 Oct 8, 2019
@livarcocc livarcocc added the Feature: Warning Waves Warnings to enable in opt-in waves. Formerly "strict mode". label Oct 8, 2019
@livarcocc livarcocc modified the milestones: MSBuild 16.5, Backlog Oct 8, 2019
radical pushed a commit to radical/msbuild that referenced this issue Apr 21, 2020
…0331.1 (dotnet#406)

- Microsoft.Dotnet.Toolset.Internal - 3.1.202-servicing.20181.1

Dependency coherency updates

- Microsoft.DotNet.Cli.Runtime - 3.1.202-servicing.20181.3 (parent: Microsoft.Dotnet.Toolset.Internal)

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
@ghidalgo3
Copy link

If MSBuild can detect when an item group evaluation has reached MAX_PATH, at least log a normal or diagnostic log saying "ItemGroup evaluation for X reached MAX_PATH at C:\longpath... and will be discarded" so that the user sees this is happening to them.

We ran into this issue internally due to a misbehaving program that generated a very long path which then causes MSBuild to suddenly stop building inexplicably because it couldn't expand a dirs.proj with an <ProjectReferences Include="src/**/*.csproj">. I wish I could get back a day of trying to figure why my build suddenly broke :)

@ghidalgo3
Copy link

@rainersigwald Would you be opposed to adding diagnostic logs when this situation is detected? I can make the change and raise a PR if you don't have any objections and if it would not be a breaking change.

@rainersigwald
Copy link
Member Author

@ghidalgo3 No, that sounds good. But I'm not sure the logging context is available at all the right places. Feel free to take a shot at it and put up a draft PR.

A message wouldn't be a breaking change, but we'll have to balance chattiness/usefulness. For MAX_PATH it's likely always nice. For "couldn't expand and falling back to literal" in general, a lot of times that's intentional and we shouldn't always message.

toptaldev92 pushed a commit to toptaldev92/React.NET that referenced this issue Jul 28, 2021
* Upgrade to Visual Studio 2017 + csproj tooling

* Include workaround for NuGet/Home#4337 (explicitly specify version when restoring NuGet packages)

* Work around dotnet/cli-migrate#11

* Upgrade to .NET Core 1.1.x

* Remove VersionPrefix from all csproj files

* Legacy NuGet restore is no longer needed

* Disable CS1701 warning for sample projects

* Turns out we actually do need the legacy NuGet restore

* Use VS2017 AppVeyor image

* Remove project dependencies to work around NuGet/Home#5193 and NuGet/Home#4578

* Use MSBuild 15 on AppVeyor

* Enforce .NET Core tools 1.0.0 in global.json

* Use newer npm on AppVeyor (as a workaround for https://github.com/dotnet/cli/issues/6561 and dotnet/msbuild#406), and run correct test command
toptaldev92 added a commit to toptaldev92/React.NET that referenced this issue Jul 28, 2021
This fixes building within VS 2017 community.
node_modules can sometimes contain paths that are very long, which can
trigger an msbuild bug

dotnet/msbuild#406
@mjsabby
Copy link

mjsabby commented Feb 16, 2023

Let's reboot this issue given there is support beyond Win32 MAX PATH now in .NET

cc @jaredpar

@jaredpar
Copy link
Member

Curious: this issue seems to predate us having pretty solid long path support in .NET. Now though it's a fairly established feature and supported by our build tools. Given the current state of our tech is there a reason why this still needs to be unsupported? Basically can wildcards now expand beyond MAX_PATH? Or is there some limitation here that stops that even with us having good long path support?

@rainersigwald
Copy link
Member Author

@mjsabby what made you comment on this? Exceeding MAX_PATH in a glob should not hit this problem any more (in a context where long paths are supported, i.e. on modern OS with long paths enabled and anywhere outside of Visual Studio's devenv.exe).

@rainersigwald
Copy link
Member Author

Basically can wildcards now expand beyond MAX_PATH?

Yes, in any case where the long-path support works, which is still not everywhere.

This issue is still open because any other I/O exception still triggers the silent assume-they-meant-a-literal-string-with-*-in-it behavior. I'll rename for clarity.

@rainersigwald rainersigwald changed the title Wildcard expansion is silently disabled when a wildcard includes a file over MAX_PATH Wildcard expansion is silently disabled when a glob expansion encounters an I/O exception Feb 16, 2023
@danmoseley
Copy link
Member

cc @JeremyKuhne as both wildcards and max path are his thing.

@rokonec
Copy link
Contributor

rokonec commented Oct 24, 2023

We have discussed in internally and would like to consider this approach:

If exception is thrown during globing consider in "non-glob" only if that patterns does not fit "high-confident-glob-pattern"

High confident glob patterns could consist of most common patterns which indicates intent was almost sure a glob. Examples:

  • ***.ext
  • subFolder***.ext
  • subFolder*.*
  • etc...

So if ItemSpec will fits high-confident-glob-pattern and it will fail we can log error leading customer to self-solving it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change bug Feature - Globbing Feature: Warning Waves Warnings to enable in opt-in waves. Formerly "strict mode". User Experience
Projects
None yet
Development

No branches or pull requests