dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

`build.sh` fails when run in a directory that contains spaces #73327

Open MichalPetryka opened 2 years ago

MichalPetryka commented 2 years ago

Running in such directory results in:

<path with a space>/runtime/eng/build.sh: line 156: <path with a space up to the space>: No such file or directory

It looks like this line is the cause: https://github.com/dotnet/runtime/blob/main/eng/build.sh#L156

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries See info in area-owners.md if you want to be subscribed.

Issue Details
Running in such directory results in: ``` /runtime/eng/build.sh: line 156: : No such file or directory ``` It looks like this line is the cause: https://github.com/dotnet/runtime/blob/main/eng/build.sh#L156
Author: MichalPetryka
Assignees: -
Labels: `area-Infrastructure-libraries`
Milestone: -
MichalPetryka commented 2 years ago

It seems like fixing that line leads to more errors later on.

ViktorHofer commented 2 years ago

Thanks for brining this to our attention. Would you mind submitting a PR with the fix? And can you please post the failures that happen when this line is fixed?

jkotas commented 2 years ago

Related: https://github.com/dotnet/runtime/issues/42397 https://github.com/dotnet/runtime/issues/13740

MichalPetryka commented 2 years ago

Thanks for brining this to our attention. Would you mind submitting a PR with the fix? And can you please post the failures that happen when this line is fixed?

I've already moved my repo to a path without spaces and closed the terminal so I can't tell exactly but it was an error when invoking msbuild, so changing that one line for sure is not enough. I'll try fixing it again tomorrow.

MichalPetryka commented 2 years ago

Thanks for brining this to our attention. Would you mind submitting a PR with the fix? And can you please post the failures that happen when this line is fixed?

Looked into this again, the following error appears after fixing the first one, I don't really know how to fix it though:

MSBUILD : error MSB1008: Only one project can be specified.
 Full command line: „/media/ewa/Samsung USB/runtime/.dotnet/sdk/7.0.100-preview.7.22377.5/MSBuild.dll -maxcpucount -verbosity:m /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/ewa/.nuget/packages/microsoft.dotnet.arcade.sdk/7.0.0-beta.22416.1/tools/Build.proj /p:Configuration=Release /p:RepoRoot=/media/ewa/Samsung USB/runtime/ /p:Restore=true /p:Build=true /p:ArcadeBuildFromSource=false /p:Rebuild=false /p:Test=false /p:Pack=false /p:IntegrationTest=false /p:PerformanceTest=false /p:Sign=false /p:Publish=false /p:Subset=clr+libs /p:TargetArchitecture=x64 /p:BuildArchitecture=x64 /p:CMakeArgs="" -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/media/ewa/Samsung USB/runtime/.dotnet/sdk/7.0.100-preview.7.22377.5/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/media/ewa/Samsung USB/runtime/.dotnet/sdk/7.0.100-preview.7.22377.5/dotnet.dll“
 Switches appended by response files: 
Switch: USB/runtime/

For switch syntax, type "MSBuild -help"
jkotas commented 2 years ago

Try to wrap https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/eng/common/build.ps1#L113 in quotes, like "/p:RepoRoot=$RepoRoot"

MichalPetryka commented 2 years ago

Try to wrap

https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/eng/common/build.ps1#L113

in quotes, like "/p:RepoRoot=$RepoRoot"

Already tried, the same error.

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/runtime-infrastructure See info in area-owners.md if you want to be subscribed.

Issue Details
Running in such directory results in: ``` /runtime/eng/build.sh: line 156: : No such file or directory ``` It looks like this line is the cause: https://github.com/dotnet/runtime/blob/main/eng/build.sh#L156
Author: MichalPetryka
Assignees: -
Labels: `area-Infrastructure`
Milestone: Future
ivdiazsa commented 2 years ago

The MSBuild error is just part of the problem. Bash also doesn't like paths with spaces when sourcing other scripts.

For example, build.sh --help also fails, albeit with a different error. It treats the path as up to the first space, and consequently, there is no directory under that name. This one is fixed by enclosing "$scriptroot" with quotes. Most of our scripts already did that but there were a few unquoted instances.

Currently looking into the MSBuild failure as Jan's suggestion unfortunately didn't work as expected.

EDIT: Tested on Windows and builds also fail there.

ivdiazsa commented 2 years ago

After some more investigation, and reading through the issues pointed by @jkotas here https://github.com/dotnet/runtime/issues/73327#issuecomment-1204400327, I can conclude this issue goes far deeper than meets the eye. It might actually be a bug with the MSBuild parser rather than our runtime, since it just ignores my quoted paths and splits in spaces. Additionally, I got two different repros in MacOS/Linux and Windows:

Linux/MacOS

$ ./build.sh -s clr -c Release

__DistroRid: osx-arm64
MSBUILD : error MSB1008: Only one project can be specified.
    Full command line: '/usr/local/share/dotnet/sdk/7.0.100-preview.7.22377.5/MSBuild.dll -maxcpucount -verbosity:m /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /Path/To/.nuget/packages/microsoft.dotnet.arcade.sdk/7.0.0-beta.22419.1/tools/Build.proj /p:Configuration=Release /p:RepoRoot="/Path/To/Runtime With Spaces/" /p:Restore=true /p:Build=true /p:ArcadeBuildFromSource=false /p:Rebuild=false /p:Test=false /p:Pack=false /p:IntegrationTest=false /p:PerformanceTest=false /p:Sign=false /p:Publish=false /p:Subset=clr /p:TargetArchitecture=arm64 /p:BuildArchitecture=arm64 /p:CMakeArgs="" -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/usr/local/share/dotnet/sdk/7.0.100-preview.7.22377.5/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/usr/local/share/dotnet/sdk/7.0.100-preview.7.22377.5/dotnet.dll'
  Switches appended by response files:
Switch: With

For switch syntax, type "MSBuild -help"
Build failed with exit code 1. Check errors above.

Check out the /p:RepoRoot flag. Also, out of curiosity, I passed the path with just a starting quote, but no closing one, and it failed with the exact same error. Which means it ignores them. Single quotes and no quotes yielded the same result.

Windows

$> .\build.cmd -s clr -c Release

C:\Path\To\Runtime With Spaces\src\coreclr\runtime.proj(61,5): error MSB3073: The command ""C:\Path\To\Runtime With Spaces\src\coreclr\build-runtime.cmd" -x64 -release -os windows -pgodatapath "C:\Path\To\.nuget\packages\optimization.windows_nt-x64.pgo.coreclr\1.0.0-prerelease.22415.6"" exited with code 1.

Build FAILED.

C:\Path\To\Runtime With Spaces\src\coreclr\runtime.proj(61,5): error MSB3073: The command ""C:\Path\To\Runtime With Spaces\src\coreclr\build-runtime.cmd" -x64 -release -os windows -pgodatapath "C:\Path\To\.nuget\packages\optimization.windows_nt-x64.pgo.coreclr\1.0.0-prerelease.22415.6"" exited with code 1.
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:13.20
Build failed with exit code 1. Check errors above.

I found the offending line in build-runtime.cmd and fixed it (there was a call statement without quoting the script path name), and then it failed later on somewhere else. My hypothesis is that the problem will keep propagating to other places (our scripts' problem) until it eventually lands on the MSBuild problem like it happens in MacOS and Linux.

trylek commented 2 years ago

This is a bottomless Pandora's box. Manish also spent some time on this but to my knowledge no-one has yet made this work end to end. I think that in general it's a worthwhile effort so if you have some fixes that let the build proceed further, just get them merged in; once we're sure we're no longer able to make progress due to internal issues in components like MSBuild, we can follow up with the partner teams.

danmoseley commented 2 years ago

Building with spaces seems to fail about once a year. Which tells me that the developers working on this day to day just avoid it and it will keep breaking periodically unless we make CI work this way.

We could just document that we don't support it...

trylek commented 2 years ago

Sure, we all strive to avoid it, but some people are less lucky - in many institutions, universities, companies, IT may force a policy that requires people to use root folders with names beyond their control (e.g. based on their names that may contain a plethora of accent signs and spaces). While it's a sad reality that today this doesn't work, I think our north star should be ultimately making this fully functional, at the very least as a nod to our inclusiveness principle.

trylek commented 2 years ago

And, after all, the default VS installation folder contains at least four blanks (C:\Program Files (x86)\Microsoft Visual Studio).

MichalPetryka commented 2 years ago

Building with spaces seems to fail about once a year. Which tells me that the developers working on this day to day just avoid it and it will keep breaking periodically unless we make CI work this way.

We could just document that we don't support it...

There could be some weekly CI job that'd build and test from a directory that contains spaces and non-ASCII characters, this'd keep things working in check.

trylek commented 2 years ago

(In practice, even this is non-trivial as AzDO has its own rules regarding directory naming, I believe we looked into that in the past and there was no easy way to name the AzDO folder for a particular run "šílený raťafák" so that would be one more thing to follow up on with the AzDO team.)

danmoseley commented 2 years ago

You're right, now I recall in the past at least one of the folks hitting this had no other option, because their user name was chosen to have a space and they could not access any other folder.

akoeplinger commented 2 years ago

One workaround for those users could be to build in a VM/Docker container.

danmoseley commented 2 years ago

It's possible the machine is locked down such that they can't do that kind of thing?

ivdiazsa commented 2 years ago

This is a bottomless Pandora's box. Manish also spent some time on this but to my knowledge no-one has yet made this work end to end. I think that in general it's a worthwhile effort so if you have some fixes that let the build proceed further, just get them merged in; once we're sure we're no longer able to make progress due to internal issues in components like MSBuild, we can follow up with the partner teams.

Alright then it's settled! I'll get a PR with fixes on our part, i.e. until the failure falls onto MSBuild.

ivdiazsa commented 2 years ago

It's possible the machine is locked down such that they can't do that kind of thing?

This is a thing in some companies so unfortunately that workaround wouldn't work for them. It's not a huge amount of places, but there are some locked down out there.