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

Cake.CoreCLR can't handle whitespace in path #2507

Closed
Roadrunner67 opened this Issue Mar 23, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@Roadrunner67
Copy link
Contributor

commented Mar 23, 2019

I know about #2214, and that seems to have been solved, but there is more to it:

Steps to reproduce:

mkdir 'My Folder'
pushd 'My Folder'
nuget install Cake.CoreCLR -o tools
dotnet new console -n HelloWorld
dotnet build .\HelloWorld\
echo 'DotNetCoreExecute("./HelloWorld/bin/Debug/netcoreapp2.2/Helloworld.dll");' > build.cake
dotnet .\tools\Cake.CoreCLR.0.32.1\Cake.dll
popd

What You Are Seeing?

No executable found matching command "dotnet-C:/Temp/My"
Error: One or more errors occurred. (.NET Core CLI: Process returned an error (exit code 1).)
.NET Core CLI: Process returned an error (exit code 1).

What is Expected?

Hello World!

What version of Cake are you using?

0.32.1

Are you running on a 32 or 64 bit system?

64

What environment are you running on? Windows? Linux? Mac?

Windows

If 'My Folder' is changed to 'MyFolder', everything works as expected.

@Roadrunner67

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

The problem seems to be in DotNetCoreExecutor.GetArguments(...)
where:

    assemblyPath = assemblyPath.IsRelative ? assemblyPath.MakeAbsolute(_environment) : assemblyPath;
    builder.Append(assemblyPath.FullPath);

should be:

    builder.AppendQuoted(assemblyPath.MakeAbsolute(_environment).FullPath);

This is how ProcessArgumentBuilder normally adds FullPaths.

(edited after closer study)

Roadrunner67 added a commit to Roadrunner67/cake that referenced this issue Mar 25, 2019

@Roadrunner67

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

The PR failed during tests against other OS'es than Windows.
I think modifying the tests to expect quoted paths will also benefit the other OS'es, but I might not be the one to judge.
@mholo65 what is your take on this?

Roadrunner67 added a commit to Roadrunner67/cake that referenced this issue Mar 25, 2019

@mholo65

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

@Roadrunner67 I think qouting paths is generally a good idea, we do this in other places as well! I see tests are now passing on other OS’s in your PR.

devlead added a commit to Roadrunner67/cake that referenced this issue Mar 27, 2019

devlead added a commit that referenced this issue Mar 27, 2019

Merge branch 'Roadrunner67-Roadrunner67-issue-#2507' into develop
* Roadrunner67-Roadrunner67-issue-#2507:
  Testing for paths with whitespace
  Expect quoted paths in unit tests
  Made tolerant to whitespace in paths (#2507)

@devlead devlead added the Improvement label Mar 27, 2019

@devlead devlead added this to the v0.33.0 milestone Mar 27, 2019

@devlead

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

fixed by #2511

@devlead devlead closed this Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.