Bootstrap fails when destination path has a space

[alexeckermann]

This refers to a twitter conversation with @Reinmar https://twitter.com/alexeckermann/status/933241249548582912

Running mgit bootstrap threw a consecutive series of errors with the description fatal: Too many arguments..

With the guidance from @Reinmar I printed to STDOUT the command being executed and it was immediately apparent that the destinationPath in cksource/mgit2/lib/commands/bootstrap.js was not escaping space characters in the command.

Example: git clone --progress git@github.com:ckeditor/ckeditor5-widget.git /Users/alexeckermann/Documents/Projects/Open Source/ckeditor5/packages/ckeditor5-widget && cd /Users/alexeckermann/Documents/Projects/Open Source/ckeditor5/packages/ckeditor5-widget && git checkout --quiet master

Which should be: git clone --progress git@github.com:ckeditor/ckeditor5-widget.git /Users/alexeckermann/Documents/Projects/Open\ Source/ckeditor5/packages/ckeditor5-widget && cd /Users/alexeckermann/Documents/Projects/Open\ Source/ckeditor5/packages/ckeditor5-widget && git checkout --quiet master

I will put together a PR and associate with this issue.

[alexeckermann]

Method to resolve:

• Traced source of path where whitespace was not escaped and breaking ./commands/bootstrap.js.
• Identified ./utils/getoptions.js as the source of the path causing issue.
• Based on other example resolutions online, used a simple string replace method to replace spaces with an escape character.
• No tests in package so no additions made there.

These changes made directly on the source of the installed package on my machine resolved the issue.

[alexeckermann]

Upon further inspection this may be better suited for cwd to have the whitespace escape feature applied to it since it is the deeper source of that path and would have a wider application across the whole package. Let me know if this is the preferred approach.

[Reinmar]

Thanks for the analysis :)

I think that the fact that this value https://github.com/cksource/mgit2/blob/7c2a2ee6e2b7d3e4c569332fab65bf8dd9d97518/lib/commands/bootstrap.js#L24 may have spaces in it is fine. It's path to a directory and e.g. if we wanted to print it out to the console, we wouldn't like (I think) to have it escaped there. E.g. pwd itself doesn't escape it when printing:

So, it'd be good to keep these values unescaped up to the point where we pass them to bash. The issue is exactly there – in the place where we create bash commands out of the input we get. In fact, there must be a lib for that :D

So, a quick fix will be to move replace() calls to such places: https://github.com/cksource/mgit2/blob/7c2a2ee6e2b7d3e4c569332fab65bf8dd9d97518/lib/commands/bootstrap.js#L34. A real fix though, would be to create/find a tool for generating commands out of args.

[pomek]

The fastest and correct fix for that is use " for paths that contain spaces: git clone gitRepository.git "path/to clone/repository". We can assume that every path can contain the space so we should wrap in the " every command that requires a path to work.

I would like to fix that issue during work on #73.

[Reinmar]

#73 will take a significant amount of time. Let's not block the quick fix because the tool is now broken for other people.

[alexeckermann]

Just realised I didn't create a PR. Happy that the referenced quick fix is adequate?

Just went and did it — #75. Reject it if there's a better way.