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.
Unified · Split
| @@ -120,7 +120,7 @@ mod prepare { · 119 unmodified lines | |||
| 120 | self | 120 | self |
| 121 | } | 121 | } |
| 122 | 122 | ||
| 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. |
| 124 | /// | 124 | /// |
| 125 | /// If that's not the case, use a shell instead. | 125 | /// If that's not the case, use a shell instead. |
| 126 | pub fn with_shell_allow_argument_splitting(mut self) -> Self { | 126 | pub fn with_shell_allow_argument_splitting(mut self) -> Self { |
| @@ -202,7 +202,14 @@ mod prepare { · 75 unmodified lines | |||
| 202 | let mut cmd = Command::new(if cfg!(windows) { "sh" } else { "/bin/sh" }); | 202 | let mut cmd = Command::new(if cfg!(windows) { "sh" } else { "/bin/sh" }); |
| 203 | cmd.arg("-c"); | 203 | cmd.arg("-c"); |
| 204 | if !prep.args.is_empty() { | 204 | 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 | } | ||
| 206 | } | 213 | } |
| 207 | cmd.arg(prep.command); | 214 | cmd.arg(prep.command); |
| 208 | cmd.arg("--"); | 215 | cmd.arg("--"); |
| @@ -324,10 +324,20 @@ mod prepare { · 323 unmodified lines | |||
| 324 | "this always needs a shell as we need tilde expansion" | 324 | "this always needs a shell as we need tilde expansion" |
| 325 | ); | 325 | ); |
| 326 | } | 326 | } |
| 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 | } | ||
| 327 | } | 338 | } |
| 328 | 339 | ||
| 329 | mod spawn { | 340 | mod spawn { |
| 330 | #[cfg(unix)] | ||
| 331 | use bstr::ByteSlice; | 341 | use bstr::ByteSlice; |
| 332 | 342 | ||
| 333 | #[test] | 343 | #[test] |
| @@ -360,6 +370,19 @@ mod spawn { · 26 unmodified lines | |||
| 360 | Ok(()) | 370 | Ok(()) |
| 361 | } | 371 | } |
| 362 | 372 | ||
| 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 | |||
| 363 | #[test] | 386 | #[test] |
| 364 | fn direct_command_execution_searches_in_path() -> crate::Result { | 387 | fn direct_command_execution_searches_in_path() -> crate::Result { |
| 365 | assert!(gix_command::prepare(if cfg!(unix) { "ls" } else { "dir.exe" }) | 388 | assert!(gix_command::prepare(if cfg!(unix) { "ls" } else { "dir.exe" }) |