cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.91k stars 729 forks source link

Working directory with a space #1898

Open jords1987 opened 7 years ago

jords1987 commented 7 years ago

What You Are Seeing? When trying to debug a cake script in vs code I am seeing the "More than one build script specified" error. I think it has something to do with my working directory having a space in a folder name as when I run without a build script specified I get an IO error and it seems to be combining my working path with the path after the space (see repro steps).

What is Expected? I am expecting to be able to debug my cake script.

What version of Cake are you using? 0.23.0

Are you running on a 32 or 64 bit system? 64

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

How Did You Get This To Happen? (Steps to Reproduce)

Clone the example repo https://github.com/mholo65/cake-vscode-debug-example into a foder path with a space in the first folder i.e "c:\spaced folder\cakedebug\" and then hit F5 on vs code or, use PS:

  1. cd "c:\spaced folder\cakedebug"
  2. ./build.ps1
  3. Observe error "More than one build script specified"

At this point you can take the space out of the base folder and rerun build.ps1, watch the build script run.

After this put the space back in the folder name and:

  1. cd "c:\spaced folder\cakedebug"
  2. dotnet tools/Cake.CoreCLR/Cake.dll --verbosity=diagnostic
  3. Observe error: Module directory does not exist. Error: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\spaced folder\cakedebug\folder\cakedebug\tools\Cake.CoreCLR'. at System.IO.Win32FileSystem.SetCurrentDirectory(String fullPath) at System.IO.Directory.SetCurrentDirectory(String path) at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments) at Cake.Commands.BuildCommand.Execute(CakeOptions options) at Cake.CakeApplication.Run(CakeOptions options) at Cake.Program.Main()
SIkebe commented 7 years ago

GetCommandLine() is called from Program.cs. But the arguments are split by space because they are not quoted properly.

https://github.com/cake-build/cake/blob/b66ac025002d35288583e1b725155aedc0d3f8f9/src/Cake/Program.cs#L39-L42

https://github.com/cake-build/cake/blob/08907d1a5d97b66f58c01ae82506280882dcfacc/src/Cake/Polyfill/EnvironmentHelper.cs#L11-L19

SIkebe commented 7 years ago

So each argument should be quoted using this?

https://github.com/cake-build/cake/blob/08907d1a5d97b66f58c01ae82506280882dcfacc/src/Cake.Core/Extensions/StringExtensions.cs#L20-L27

jnm2 commented 7 years ago

So each argument should be quoted using this?

No. Please see https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/.

If the path ever happens to end in a backslash, it escapes the ending quote and you're in big trouble.

See https://gist.github.com/jnm2/c5c840bf317605a40f5f56f944db4892 for a safe implementation of argv quoting with tests.

See also https://github.com/dotnet/corefx/blob/5d3e42f831ec3d4c5964040e235824f779d5db53/src/Common/src/System/PasteArguments.cs.

See dotnet/corefx#23592 for a discussion of how to handle args in a cross-platform way.

jnm2 commented 7 years ago

In my opinion, Cake should deprecate AppendQuoted and autoquote everything passed to Append if necessary. The problem has been rampant every time I've looked. My EscapeProcessArgument implementation can either quote only if necessary or always.

Again, simply appending and prepending a quote character is dangerous and incorrect.

SIkebe commented 7 years ago

@jnm2 Thanks! Can you share your thoughts on this issue?

jnm2 commented 7 years ago

dotnet/corefx#23592 is a very insightful discussion and I hope Cake takes notice.

Windows is weird in that all the arguments are stuffed in a single string. Linux actually passes processes the equivalent of string[], not string. So my thought is that EnvironmentHelper.GetCommandLine() should be deleted and everything should rely on Environment.GetCommandLineArgs(). If you don't do this, it means on Linux you're joining and resplitting and you're bound to get it wrong.

That's for consuming arguments. For producing arguments, Cake should keep everything as string[] and wait until the last possible moment before serializing to an argv string and quoting if need be. One day we will have ProcessStartInfo.ArgumentList and Cake should no longer let itself be responsible for serializing the array to a single string. In the meantime, Cake should follow https://github.com/dotnet/corefx/blob/5d3e42f831ec3d4c5964040e235824f779d5db53/src/Common/src/System/PasteArguments.cs#L16 rather than naive quoting.

jnm2 commented 7 years ago

This has been an ongoing concern of mine with the entirety of Cake. I started work towards addressing the naive quoting issue but my plate is way overfull. I sincerely hope someone does address it. It's one of the few things that I strongly believe Cake needs to fix ASAP. It's hard to fix properly because poor assumptions about quoting are baked into the Append and AppendQuoted etc API when what we should have been using from day one was Append() that auto quoted when necessary.

And remember, one day, it wouldn't even be responsible for the quoting. It would just pass the list to ProcessStartInfo.ArgumentList so that Linux passes the list of args to the process with full fidelity; it would never appear even once as a single string. And on Windows, the BCL would then take care of serializing into a single string by following the argv rules so that the arguments are deserialized back into a string[] in the target process with full fidelity.

ktenedios commented 6 years ago

I too would like to see this resolved, as it has taken me the best part of a day to work out that this limitation exists in Cake.CoreCLR. If anyone is interested in a workaround (a.k.a. hack), here is a PowerShell sample that will convert the Cake.dll path to the old-style DOS 8:3 path format:

$cakeDll = "C:\Path with spaces\cake.coreclr\0.27.2\Cake.dll"
$fso = New-Object -ComObject Scripting.FileSystemObject
$cakeDll = $fso.GetFile($cakeDll).ShortPath
& dotnet $cakeDll

Note that this has only been tested in Windows. I have not done a Linux/MacOS Bash equivalent, as I have not explored this as yet.