-
Notifications
You must be signed in to change notification settings - Fork 205
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
Fix quoting of git ssh arguments #5947
Conversation
I tested this locally |
@@ -32,7 +32,7 @@ sub _run_cmd ($self, $args, $options = {}) { | |||
my $include_git_path = $options->{include_git_path} // 1; | |||
my $ssh_batchmode = $options->{ssh_batchmode} // 0; | |||
my @cmd; | |||
push @cmd, 'env', 'GIT_SSH_COMMAND="ssh -oBatchMode=yes"' if $ssh_batchmode; | |||
push @cmd, 'env', 'GIT_SSH_COMMAND=ssh -oBatchMode=yes' if $ssh_batchmode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have unit test coverage for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but no good idea yet.
e.g. if we actually ran git on temporary git repositories created during unit tests, that would be an improvement, but we would use local git repositories in paths, not remote ones, I guess, so the ssh ...
stuff would never be called.
Ideas welcome.
In any case, we should deploy this fix.
ok, I need to fix the tests of course. edit: pushed |
Error detecting remote default branch name for "some git": "ssh -oBatchMode=yes": ssh -oBatchMode=yes: command not found Since we are passing an array ref to IPC::Run::run, we should not add quotes. git is trying to call `"ssh ..."` including the quotes. Issues: * https://progress.opensuse.org/issues/164898 * https://progress.opensuse.org/issues/164886
fd38c0e
to
9bc1ffa
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5947 +/- ##
=======================================
Coverage 98.74% 98.75%
=======================================
Files 396 396
Lines 38967 38967
=======================================
+ Hits 38479 38482 +3
+ Misses 488 485 -3 ☔ View full report in Codecov by Sentry. |
Since we are passing an array ref to IPC::Run::run, we should not add quotes. git is trying to call
"ssh ..."
including the quotes.Issues: