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

Spaces in command arguments break things #88

Closed
Devmeon opened this issue Sep 23, 2020 · 5 comments · Fixed by #119
Closed

Spaces in command arguments break things #88

Devmeon opened this issue Sep 23, 2020 · 5 comments · Fixed by #119
Assignees

Comments

@Devmeon
Copy link

Devmeon commented Sep 23, 2020

Command line:
braid add --verbose ssh://git@url.git Externe/project

Output:

Braid: Executing `git --version` in D:/folder
Braid: Executing `git rev-parse --is-inside-work-tree` in D:/folder
Braid: Executing `git rev-parse --show-prefix` in D:/folder
Braid: Executing `git status` in D:/folder
Braid: Executing `git rev-parse HEAD` in D:/folder
Braid: Executing `git add .braids.json` in D:/folder
Braid: Adding mirror of 'ssh://git@url.git'.
Braid: Executing `git --version` in D:/folder
Braid: Executing `git rev-parse --is-inside-work-tree` in D:/folder
Braid: Executing `git rev-parse --show-prefix` in D:/folder
Braid: Executing `git config remote.master/braid/Externe/project.url` in D:/folder
Braid: Setup: Creating remote for 'Externe/project'.
Braid: Executing `git remote add master/braid/Externe/project C:/Users/HeadQuarter n°1/.braid/cache/ssh___git_url.git` in D:/folder
Braid: Shell error: usage: git remote add [<options>] <name> <url>

braid version : braid 1.1.3
git version : git version 2.27.0.windows.1

@mattmccutchen mattmccutchen changed the title Braid: Shell error: usage: git remote add [<options>] <name> <url> Spaces in command arguments break things Sep 23, 2020
@mattmccutchen
Copy link
Collaborator

mattmccutchen commented Sep 23, 2020

Without testing, it looks like the problem is that you have a space in your username and Braid joins command arguments with spaces and passes the result to the shell without any quoting. ☹️ Hopefully we can fix this by moving everything to the forms of Open3.capture3, etc. that take a list of arguments.

However, it's generally advisable (even on unix-like OSes) to avoid spaces in file paths for software development work because lots of less-used development tools have such bugs, although as they get more used, the bugs tend to get fixed (as is happening here with Braid). Indeed, in some programming environments such as makefiles, trying to defend against all characters treated specially by the shell entails so much clutter in the code that it's considered better to require users not to use those characters; maybe we should be moving off of those environments, but realistically we will be stuck with them for many years. In fact, I think the removal of spaces from a lot of default folder names in Windows Vista (e.g., Documents and Settings -> Users) was at least partly for this reason, though I haven't confirmed this.

@Devmeon
Copy link
Author

Devmeon commented Sep 23, 2020

Your are right, the problem was the space in my username.
After renaming my username, it went smooth as expected.

@Devmeon Devmeon closed this as completed Sep 23, 2020
@mattmccutchen
Copy link
Collaborator

I'm glad to have confirmation of what the problem was and that you worked around it. This is still a bug in Braid.

@Saragani
Copy link

Saragani commented Jun 7, 2022

I'll be glad to see this issue fixed. In my scenario, it was just one folder, that I had to rename and add an _ instead of space, but on some cases, there are several directories one inside the other where there are spaces in few of them.
On existing projects, renaming a folder (In my case, the solution has over 110 projects), might break things.

@mattmccutchen mattmccutchen self-assigned this Jun 20, 2022
@mattmccutchen
Copy link
Collaborator

I'm working on getting the Braid::Operations::Git class statically typed, which is going to require extensive rework and review of that class and all calls to it anyway, and I'm hoping it will be efficient to fix the problems with spaces at the same time.

mattmccutchen added a commit that referenced this issue Jan 1, 2023
(Since each of these changes requires extensive reworking of
operations.rb and review of all its callers, it's efficient to make both
changes together.)

To fix the handling of spaces (and perhaps other characters as well) in
shell command arguments, we just keep the arguments as a list of strings
all the way to an Open3.capture3 (or similar) that doesn't use the
shell, instead of joining the arguments with spaces.  I added one
integration test of each Braid command with spaces in the file paths.

I'm still mostly avoiding nontrivial changes to Braid's runtime
behavior, preferring to leave them to a separate pull request.  A
notable exception: when generating a remote name, Braid now replaces a
much larger set of characters with underscores.  For the new tests to
pass, I had to at least replace spaces, and I decided to go all the way.

Fixes #88.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants