Skip to content

Commit 74631d4

Browse files
committed
fix: Add test to assure echo "$@" >&2 works as expected (#1284)
Previously it would print the arguments twice, now it only prints it once as we avoid adding the 'trick' to the script if it's already present. This helps with custom git credential helpers who endorse to to call them like `!credential-helper $@`, in which case the added arguments are already part of the script. See https://github.com/languitar/pass-git-helper for detailed documentation on how that should usually work. Git seems to manage to not automatically add '$@' when calling credential helpers, but it's something that is done by `gix-command` automatically when a command should be invoked that receives arguments *and* has to be evaluated by a shell. The current implementation is very naive, but should also work for 99.9% of the cases out there.
1 parent face359 commit 74631d4

File tree

2 files changed

+33
-3
lines changed

2 files changed

+33
-3
lines changed

gix-command/src/lib.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ mod prepare {
120120
self
121121
}
122122

123-
/// Use a shell, but try to split arguments by hand if this be safely done without a shell.
123+
/// Use a shell, but try to split arguments by hand if this can be safely done without a shell.
124124
///
125125
/// If that's not the case, use a shell instead.
126126
pub fn with_shell_allow_argument_splitting(mut self) -> Self {
@@ -202,7 +202,14 @@ mod prepare {
202202
let mut cmd = Command::new(if cfg!(windows) { "sh" } else { "/bin/sh" });
203203
cmd.arg("-c");
204204
if !prep.args.is_empty() {
205-
prep.command.push(" \"$@\"")
205+
if prep.command.to_str().map_or(true, |cmd| !cmd.contains("$@")) {
206+
prep.command.push(" \"$@\"");
207+
} else {
208+
gix_trace::debug!(
209+
"Will not add '$@' to '{:?}' as it seems to contain it already",
210+
prep.command
211+
);
212+
}
206213
}
207214
cmd.arg(prep.command);
208215
cmd.arg("--");

gix-command/tests/command.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,20 @@ mod prepare {
324324
"this always needs a shell as we need tilde expansion"
325325
);
326326
}
327+
328+
#[test]
329+
fn script_with_dollar_at() {
330+
let cmd = std::process::Command::from(gix_command::prepare("echo \"$@\" >&2").with_shell().arg("store"));
331+
assert_eq!(
332+
format!("{cmd:?}"),
333+
format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#),
334+
"this is how credential helpers have to work as for some reason they don't get '$@' added in Git.\
335+
We deal with it by not doubling the '$@' argument, which seems more flexible."
336+
);
337+
}
327338
}
328339

329340
mod spawn {
330-
#[cfg(unix)]
331341
use bstr::ByteSlice;
332342

333343
#[test]
@@ -360,6 +370,19 @@ mod spawn {
360370
Ok(())
361371
}
362372

373+
#[test]
374+
fn script_with_dollar_at() -> crate::Result {
375+
let out = std::process::Command::from(gix_command::prepare("echo \"$@\"").with_shell().arg("arg"))
376+
.spawn()?
377+
.wait_with_output()?;
378+
assert_eq!(
379+
out.stdout.to_str_lossy().trim(),
380+
"arg",
381+
"the argument is just mentioned once"
382+
);
383+
Ok(())
384+
}
385+
363386
#[test]
364387
fn direct_command_execution_searches_in_path() -> crate::Result {
365388
assert!(gix_command::prepare(if cfg!(unix) { "ls" } else { "dir.exe" })

0 commit comments

Comments
 (0)