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.

Sign up for GitHub

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

various improvements #1800

Merged
merged 2 commits into from
Jan 23, 2025
Merged

various improvements #1800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
667de43
fix!: rename `with_shell()` to `command_may_be_shell_script()`, add `…
Byron Jan 23, 2025
28f712e
adapt to changes in `gix-command`
Byron Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 30 additions & 14 deletions gix-command/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,27 +93,41 @@ mod prepare {

/// Builder
impl Prepare {
/// If called, the command will not be executed directly, but with `sh`, but only if the
/// command passed to [`prepare`](super::prepare()) requires this.
/// If called, the command will be checked for characters that are typical for shell scripts, and if found will use `sh` to execute it
/// or whatever is set as [`with_shell_program()`](Self::with_shell_program()).
/// If the command isn't valid UTF-8, a shell will always be used.
///
/// This also allows to pass shell scripts as command, or use commands that contain arguments which are subsequently
/// parsed by `sh`.
pub fn with_shell(mut self) -> Self {
/// If a shell is used, then arguments given here with [arg()](Self::arg) or [args()](Self::args) will be substituted via `"$@"` if it's not
/// already present in the command.
///
/// If this method is not called, commands are always executed verbatim, without the use of a shell.
pub fn command_may_be_shell_script(mut self) -> Self {
self.use_shell = self.command.to_str().map_or(true, |cmd| {
cmd.as_bytes().find_byteset(b"|&;<>()$`\\\"' \t\n*?[#~=%").is_some()
});
self
}

/// Set the name or path to the shell `program` to use, to avoid using the default shell which is `sh`.
/// If called, unconditionally use a shell to execute the command and its arguments, and `sh` to execute it,
/// or whatever is set as [`with_shell_program()`](Self::with_shell_program()).
///
/// If a shell is used, then arguments given here with [arg()](Self::arg) or [args()](Self::args) will be substituted via `"$@"` if it's not
/// already present in the command.
///
/// If this method is not called, commands are always executed verbatim, without the use of a shell.
pub fn with_shell(mut self) -> Self {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this cause code targeting the previous API and calling with_shell without actually requiring a shell to be used to continue to build and appear to work, but to work differently by using a shell in circumstances where it wasn't before? If so, I wonder if that might introduce bugs, or worsen existing bugs, in code that uses gix-command.

Copy link
Member Author

@Byron Byron Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was aware of this (and agree that it could cause unwanted effects downstream) and probably didn't even mark it as fix: which is what I thought about when doing so.
with_shell() now actually does what it says, as opposed to before when it surprisingly only used a shell when the command seemed to need it. Especially on Windows this matters so I really thought of it as a fix for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is true that it could fix bugs where people assumed use_shell had a more intuitive meaning. An alternative could be to abandon the method name use_shell altogether and call all the methods something different. That would break anything that should be broken. It might also break things that need not break.

On the other hand, I recognize it's possible to take that too far and accumulate lots of names that one does use in the future even when they would be the best description available. So I don't know what's best.

In this project itself, I think the changed methods may be useful for future bug investigation. I suspect we have some invocations that treat the command name as a shell script when, to match the expectations set by Git's behavior and the guarantees in Git's documentation, it would be necessary to treat the command as a shell script when it was obtained from some sources, while being necessary to treat it not as a shell script (i.e. just as a path) when it was obtained from other sources.

The specific case of this that I most strongly suspect the SSH client. When obtained from the environment GIT_SSH, it must not be run or parsed as a shell script (regardless of its value). When obtained from GIT_SSH_COMMAND, or the value of the core.sshCommand configuration variable, it is a shell script and must either be run as one or (in sufficiently simple cases) parsed as one. These requirements are explicated in git(1).

I have not checked recently--and maybe not at all, in terms of running experiments to verify it--that we are doing the wrong thing here. When I have a chance to test it, I'll open an issue if we are. I recall it having seemed so when I was working on #1342, and that it seemed like a contributing factor to the bug reported in #1432 and discussed further in connection with some related topics in #1585 (comment). This was the suspected aspect of #1432 highlighted in #1432 (comment).

If we don't yet have an issue for it, then I'll try to open one once I get around to checking the effect in a way that facilitates direct comparison to Git behavior. Some invocations of the SSH client occur only under some circumstances; for example, it is only invoked to figure out what kind of SSH client it is if that cannot be inferred from its name (and is not overridden by configuration), and I think that is one of the places where GIT_SSH may be being wrongly treated like GIT_SSH_COMMAND.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this up!

I was aware of the difference between GIT_SSH and GIT_SSH_COMMAND at the time and should have implemented it correctly. Due to me renaming with_shell() to its new name, in this codebase the semantics are preserved, too, so I'd think there is no issue, still.

However, there I support and value an investigation to assure there aren't any subtle bugs around that in the hopes that this also helps the API of gix-command to mature. After all, it's vital to make it as bullet-proof and unambiguous to use as possible.

Copy link
Member

@EliahKagan EliahKagan Jan 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to me renaming with_shell() to its new name, in this codebase the semantics are preserved, too, so I'd think there is no issue, still.

I very much agree that this PR introduces no new incorrect behavior--related to the distinction between GIT_SSH and GIT_SSH_COMMAND, or otherwise. But I think there is probably a preexisting issue, and if so, it remains intact. (One of the ways this PR is significant to that is that it calls attention to such possible things, which is a good thing about this PR, not a bad thing.)

On Windows, \ is the primary directory separator, so it often appears in paths that are otherwise unremarkable, like C:\Windows\System32\OpenSSH\ssh.exe. But \ is also taken to mean that we must run a command in a shell if we are allowed to do so (since \ is a shell metacharacter). Specifically:

  • The method now called command_may_be_shell_script is meant to conditionally refrain from planning to run unremarkable paths using a shell, but the presence of \ causes it to go ahead and plan that by setting the use_shell field to true:

    gitoxide/gix-command/src/lib.rs

    Lines 108 to 110 in f3dc83b

    self.use_shell = self.command.to_str().map_or(true, |cmd| {
    cmd.as_bytes().find_byteset(b"|&;<>()$`\\\"' \t\n*?[#~=%").is_some()
    });

  • When the use_shell field is set to true, using a shell is planned, but it may still be averted if the allow_manual_arg_splitting field is set to true, which is its default value on Windows. However, \ is also one of the characters that suppresses the effect of allow_manual_arg_splitting:

    gitoxide/gix-command/src/lib.rs

    Lines 222 to 234 in f3dc83b

    let split_args = prep
    .allow_manual_arg_splitting
    .then(|| {
    if gix_path::into_bstr(std::borrow::Cow::Borrowed(prep.command.as_ref()))
    .find_byteset(b"\\|&;<>()$`\n*?[#~%")
    .is_none()
    {
    prep.command
    .to_str()
    .and_then(|args| shell_words::split(args).ok().map(Vec::into_iter))
    } else {
    None
    }

    Where, matching on split_args:

    gitoxide/gix-command/src/lib.rs

    Lines 243 to 244 in f3dc83b

    None => {
    let mut cmd = Command::new(

Therefore, if there are places where the function now called command_may_be_shell_script is called when it shouldn't be called, on Windows this will often have the effect of running a command in a shell that should not be run that way--this effect is more likely on Windows, due to \ being a common character in paths.

To be clear, this behavior is not in any way exacerbated by the changes in this PR. My thinking was that, as far as this sort of thing goes, the changes here are a boon, because they make it easier to reason about what is going on.

However, I just now realize that there is actually a sense in which the changes in this PR exacerbate this. Before, because the method had the name with_shell, it conveyed the impression that a shell would be used in more circumstances than it was intended to convey. But as detailed above, it seems that a shell was also used, on Windows, in more circumstances than it was intended to be used. These partially "cancelled out": the impression it gave was, on Windows, closer to the truth than it probably is now.

I don't suggest that as a reason to undo anything from this PR. Instead, the specific effect of \ in a command on those two methods on Windows, in light of the special significance of \ on Windows, can modified, documented, or both; and any code in this project that calls command_may_be_shell_script when it shouldn't can be identified and fixed.

I was aware of the difference between GIT_SSH and GIT_SSH_COMMAND at the time and should have implemented it correctly.

As far as I can tell, in the main code path used to decide how to invoke an SSH client to actually connect to a remote, this is implemented perfectly. In gix::repository::config::ssh_connect_options, the fallback_active flag keeps track of where the SSH client command is configured from:

gitoxide/gix/src/repository/config/mod.rs

Lines 63 to 70 in f3dc83b

let mut fallback_active = false;
let ssh_command = config
.string_filter(Core::SSH_COMMAND, &mut trusted)
.or_else(|| {
fallback_active = true;
config.string_filter(gitoxide::Ssh::COMMAND_WITHOUT_SHELL_FALLBACK, &mut trusted)
})
.map(|cmd| gix_path::from_bstr(cmd).into_owned().into());

This is used to decide, I believe correctly, whether to disallow the use of a shell:

gitoxide/gix/src/repository/config/mod.rs

Lines 71 to 79 in f3dc83b

let opts = gix_protocol::transport::client::ssh::connect::Options {
disallow_shell: fallback_active,
command: ssh_command,
kind: config
.string_filter("ssh.variant", &mut trusted)
.and_then(|variant| Ssh::VARIANT.try_into_variant(variant).transpose())
.transpose()
.with_leniency(self.options.lenient_config)?,
};

These options are ultimately passed to gix_transport::client::blocking_io::ssh::connect. In most cases, this connect function arranges a scenario in which the SSH client will run once. This happens through the last thing this connect function does, which to unconditionally call gix_transport::client::blocking_io::file::SpawnProcessOnDemand::new_ssh. That call relates to actually using the SSH client to make a connection to a server and transfer data. It properly passes along the disallow_shell flag:

gitoxide/gix-transport/src/client/blocking_io/ssh/mod.rs

Lines 136 to 144 in f3dc83b

Ok(blocking_io::file::SpawnProcessOnDemand::new_ssh(
url,
ssh_cmd,
path,
kind,
options.disallow_shell,
desired_version,
trace,
))

However, before this connect function does that, it figures out how the SSH client will be invoked to do that. It checks if this is configured, and otherwise tries to ascertain what SSH implementation it is or behaves like. Under some conditions, it runs the SSH client in order to do this. I am unclear at this time on how often or exactly under what circumstances that happens. The logic is:

gitoxide/gix-transport/src/client/blocking_io/ssh/mod.rs

Lines 112 to 113 in f3dc83b

let mut kind = options.kind.unwrap_or_else(|| ProgramKind::from(ssh_cmd));
if options.kind.is_none() && kind == ProgramKind::Simple {

But when it does it, it runs a command that it sets up like this:

gitoxide/gix-transport/src/client/blocking_io/ssh/mod.rs

Lines 115 to 125 in f3dc83b

gix_command::prepare(ssh_cmd)
.stderr(Stdio::null())
.stdout(Stdio::null())
.stdin(Stdio::null())
.command_may_be_shell_script()
.arg("-G")
.arg(match url.host_as_argument() {
Usable(host) => host,
Dangerous(host) => Err(Error::AmbiguousHostName { host: host.into() })?,
Absent => panic!("BUG: host should always be present in SSH URLs"),
}),

(In #1342, I modified part of this, but not the part that is relevant to the use of a shell. I wondered why it was calling what was, back then, named with_shell, but this was not part of what was necessary to modify to achieve the goals of that PR.)

That code unconditionally calls the command_may_be_shell_script method, and does not subsequently do anything to reverse its effects. It runs the command immediately. Specifically, the above expression is passed to:

gitoxide/gix-transport/src/client/blocking_io/ssh/mod.rs

Line 114 in f3dc83b

let mut cmd = std::process::Command::from(

And the subsequent code is:

gitoxide/gix-transport/src/client/blocking_io/ssh/mod.rs

Lines 127 to 132 in f3dc83b

gix_features::trace::debug!(cmd = ?cmd, "invoking `ssh` for feature check");
kind = if cmd.status().ok().is_some_and(|status| status.success()) {
ProgramKind::Ssh
} else {
ProgramKind::Simple
};

Consequently, when gix-transport pre-runs an SSH client to figure out what kind of SSH client it is, it seems to me that it runs it in a way that allows a shell to be used and, combined with the aforementioned effect of \ suppressing the conditional shell-avoidance behaviors of both command_may_be_shell_script and allow_manual_arg_splitting, this will often run it in a (Bourne-style) shell on Windows, where it will typically fail or otherwise malfunction because the shell treats \ as the escape character.

(Separately from that, I am unclear on why gix_transport::client::blocking_io::file::SpawnProcessOnDemand::new_local always passes a false value of ssh_disallow_shell. But since, if I understand correctly, that is not using SSH, my guess is that it is okay.)

Edit: I've opened #1814, which would fix the second part of what I've described here--that is, the place in gix-transport where disallow_shell should be followed but is not, when running the SSH client to figure out what implementation it is. I'm not sure how good shape that PR is in yet, though (as described there).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for sharing! I see now how this PR made it easier to realize that on Windows, the shell would actually run more often than it would on Unix.

(Separately from that, I am unclear on why gix_transport::client::blocking_io::file::SpawnProcessOnDemand::new_local always passes a false value of ssh_disallow_shell. But since, if I understand correctly, that is not using SSH, my guess is that it is okay.)

Memory does not serve, unfortunately, but given the importance of not messing this up (i.e. URLs could be crafted to use the shell to execute arbitrary commands) I'd hope that was intentional. However, I'd think it's better to document why this is false given that shells ideally are avoided entirely.

self.use_shell = true;
self
}
/// Set the name or path to the shell `program` to use if a shell is to be used, to avoid using the default shell which is `sh`.
pub fn with_shell_program(mut self, program: impl Into<OsString>) -> Self {
self.shell_program = Some(program.into());
self
}

/// Unconditionally turn off using the shell when spawning the command.
/// Note that not using the shell is the default so an effective use of this method
/// is some time after [`with_shell()`][Prepare::with_shell()] was called.
/// is some time after [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()) was called.
pub fn without_shell(mut self) -> Self {
self.use_shell = false;
self
Expand All @@ -128,18 +142,20 @@ mod prepare {
self
}

/// Use a shell, but try to split arguments by hand if this can be safely done without a shell.
/// Like [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()), but try to split arguments by hand if this can be safely done without a shell.
///
/// If that's not the case, use a shell instead.
pub fn with_shell_allow_manual_argument_splitting(mut self) -> Self {
/// This is useful on platforms where spawning processes is slow, or where many processes have to be spawned in a raw which should be sped up.
/// Manual argument splitting is enabled by default on Windows only.
pub fn command_may_be_shell_script_allow_manual_argument_splitting(mut self) -> Self {
self.allow_manual_arg_splitting = true;
self.with_shell()
self.command_may_be_shell_script()
}

/// Use a shell, but prohibit splitting arguments by hand even if this could be safely done without a shell.
pub fn with_shell_disallow_manual_argument_splitting(mut self) -> Self {
/// Like [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()), but don't allow to bypass the shell even if manual argument splitting
/// can be performed safely.
pub fn command_may_be_shell_script_disallow_manual_argument_splitting(mut self) -> Self {
self.allow_manual_arg_splitting = false;
self.with_shell()
self.command_may_be_shell_script()
}

/// Configure the process to use `stdio` for _stdin.
Expand Down
74 changes: 55 additions & 19 deletions gix-command/tests/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ mod prepare {
#[test]
fn multiple_arguments_in_one_line_with_auto_split() {
let cmd = std::process::Command::from(
gix_command::prepare("echo first second third").with_shell_allow_manual_argument_splitting(),
gix_command::prepare("echo first second third")
.command_may_be_shell_script_allow_manual_argument_splitting(),
);
assert_eq!(
format!("{cmd:?}"),
Expand All @@ -267,7 +268,8 @@ mod prepare {

#[test]
fn single_and_multiple_arguments_as_part_of_command_with_shell() {
let cmd = std::process::Command::from(gix_command::prepare("ls first second third").with_shell());
let cmd =
std::process::Command::from(gix_command::prepare("ls first second third").command_may_be_shell_script());
assert_eq!(
format!("{cmd:?}"),
if cfg!(windows) {
Expand All @@ -283,7 +285,7 @@ mod prepare {
fn single_and_multiple_arguments_as_part_of_command_with_given_shell() {
let cmd = std::process::Command::from(
gix_command::prepare("ls first second third")
.with_shell()
.command_may_be_shell_script()
.with_shell_program("/somepath/to/bash"),
);
assert_eq!(
Expand All @@ -299,7 +301,11 @@ mod prepare {

#[test]
fn single_and_complex_arguments_as_part_of_command_with_shell() {
let cmd = std::process::Command::from(gix_command::prepare("ls --foo \"a b\"").arg("additional").with_shell());
let cmd = std::process::Command::from(
gix_command::prepare("ls --foo \"a b\"")
.arg("additional")
.command_may_be_shell_script(),
);
assert_eq!(
format!("{cmd:?}"),
if cfg!(windows) {
Expand All @@ -314,7 +320,7 @@ mod prepare {
#[test]
fn single_and_complex_arguments_with_auto_split() {
let cmd = std::process::Command::from(
gix_command::prepare("ls --foo=\"a b\"").with_shell_allow_manual_argument_splitting(),
gix_command::prepare("ls --foo=\"a b\"").command_may_be_shell_script_allow_manual_argument_splitting(),
);
assert_eq!(
format!("{cmd:?}"),
Expand All @@ -326,15 +332,29 @@ mod prepare {
#[test]
fn single_and_complex_arguments_without_auto_split() {
let cmd = std::process::Command::from(
gix_command::prepare("ls --foo=\"a b\"").with_shell_disallow_manual_argument_splitting(),
gix_command::prepare("ls --foo=\"a b\"").command_may_be_shell_script_disallow_manual_argument_splitting(),
);
assert_eq!(format!("{cmd:?}"), quoted(&[SH, "-c", r#"ls --foo=\"a b\""#, "--"]));
}

#[test]
fn single_and_simple_arguments_without_auto_split_with_shell() {
let cmd = std::process::Command::from(
gix_command::prepare("ls")
.arg("--foo=a b")
.command_may_be_shell_script_disallow_manual_argument_splitting()
.with_shell(),
);
assert_eq!(
format!("{cmd:?}"),
quoted(&[SH, "-c", "ls \\\"$@\\\"", "--", "--foo=a b"])
);
}

#[test]
fn single_and_complex_arguments_will_not_auto_split_on_special_characters() {
let cmd = std::process::Command::from(
gix_command::prepare("ls --foo=~/path").with_shell_allow_manual_argument_splitting(),
gix_command::prepare("ls --foo=~/path").command_may_be_shell_script_allow_manual_argument_splitting(),
);
assert_eq!(
format!("{cmd:?}"),
Expand All @@ -345,7 +365,8 @@ mod prepare {

#[test]
fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() {
let cmd = std::process::Command::from(gix_command::prepare("~/bin/exe --foo \"a b\"").with_shell());
let cmd =
std::process::Command::from(gix_command::prepare("~/bin/exe --foo \"a b\"").command_may_be_shell_script());
assert_eq!(
format!("{cmd:?}"),
format!(r#""{SH}" "-c" "~/bin/exe --foo \"a b\"" "--""#),
Expand All @@ -355,7 +376,11 @@ mod prepare {

#[test]
fn script_with_dollar_at() {
let cmd = std::process::Command::from(gix_command::prepare("echo \"$@\" >&2").with_shell().arg("store"));
let cmd = std::process::Command::from(
gix_command::prepare("echo \"$@\" >&2")
.command_may_be_shell_script()
.arg("store"),
);
assert_eq!(
format!("{cmd:?}"),
format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#),
Expand All @@ -374,7 +399,7 @@ mod spawn {
let out = gix_command::prepare("echo $FIRST $SECOND")
.env("FIRST", "first")
.env("SECOND", "second")
.with_shell()
.command_may_be_shell_script()
.spawn()?
.wait_with_output()?;
assert_eq!(out.stdout.as_bstr(), "first second\n");
Expand All @@ -385,12 +410,15 @@ mod spawn {
#[cfg(unix)]
fn disallow_shell() -> crate::Result {
let out = gix_command::prepare("PATH= echo hi")
.with_shell()
.command_may_be_shell_script()
.spawn()?
.wait_with_output()?;
assert_eq!(out.stdout.as_bstr(), "hi\n");

let mut cmd: std::process::Command = gix_command::prepare("echo hi").with_shell().without_shell().into();
let mut cmd: std::process::Command = gix_command::prepare("echo hi")
.command_may_be_shell_script()
.without_shell()
.into();
assert!(
cmd.env_remove("PATH").spawn().is_err(),
"no command named 'echo hi' exists"
Expand All @@ -400,9 +428,13 @@ mod spawn {

#[test]
fn script_with_dollar_at() -> crate::Result {
let out = std::process::Command::from(gix_command::prepare("echo \"$@\"").with_shell().arg("arg"))
.spawn()?
.wait_with_output()?;
let out = std::process::Command::from(
gix_command::prepare("echo \"$@\"")
.command_may_be_shell_script()
.arg("arg"),
)
.spawn()?
.wait_with_output()?;
assert_eq!(
out.stdout.to_str_lossy().trim(),
"arg",
Expand Down Expand Up @@ -433,7 +465,7 @@ mod spawn {
#[test]
fn command_in_path_with_args() -> crate::Result {
assert!(gix_command::prepare(if cfg!(unix) { "ls -l" } else { "dir.exe -a" })
.with_shell()
.command_may_be_shell_script()
.spawn()?
.wait()?
.success());
Expand All @@ -442,14 +474,18 @@ mod spawn {

#[test]
fn sh_shell_specific_script_code() -> crate::Result {
assert!(gix_command::prepare(":;:;:").with_shell().spawn()?.wait()?.success());
assert!(gix_command::prepare(":;:;:")
.command_may_be_shell_script()
.spawn()?
.wait()?
.success());
Ok(())
}

#[test]
fn sh_shell_specific_script_code_with_single_extra_arg() -> crate::Result {
let out = gix_command::prepare("printf")
.with_shell()
.command_may_be_shell_script()
.arg("1")
.spawn()?
.wait_with_output()?;
Expand All @@ -461,7 +497,7 @@ mod spawn {
#[test]
fn sh_shell_specific_script_code_with_multiple_extra_args() -> crate::Result {
let out = gix_command::prepare("printf")
.with_shell()
.command_may_be_shell_script()
.arg("%s")
.arg("arg")
.spawn()?
Expand Down
4 changes: 2 additions & 2 deletions gix-credentials/src/program/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ impl Program {
args.insert_str(0, git_program.to_string_lossy().as_ref());
gix_command::prepare(gix_path::from_bstr(args.as_bstr()).into_owned())
.arg(action.as_arg(true))
.with_shell_allow_manual_argument_splitting()
.command_may_be_shell_script_allow_manual_argument_splitting()
.into()
}
Kind::ExternalShellScript(for_shell)
| Kind::ExternalPath {
path_and_args: for_shell,
} => gix_command::prepare(gix_path::from_bstr(for_shell.as_bstr()).as_ref())
.with_shell()
.command_may_be_shell_script()
.arg(action.as_arg(true))
.into(),
};
Expand Down
2 changes: 1 addition & 1 deletion gix-diff/src/blob/pipeline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ impl Driver {
let cmd = gix_command::prepare(gix_path::from_bstr(command).into_owned())
// TODO: Add support for an actual Context, validate it *can* match Git
.with_context(Default::default())
.with_shell()
.command_may_be_shell_script()
.stdin(Stdio::null())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
Expand Down
2 changes: 1 addition & 1 deletion gix-filter/src/driver/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fn spawn_driver(
context: &gix_command::Context,
) -> Result<(std::process::Child, std::process::Command), Error> {
let mut cmd: std::process::Command = gix_command::prepare(gix_path::from_bstr(cmd).into_owned())
.with_shell()
.command_may_be_shell_script()
.with_context(context.clone())
.stdin(Stdio::piped())
.stdout(Stdio::piped())
Expand Down
2 changes: 1 addition & 1 deletion gix-merge/src/blob/platform/merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ pub(super) mod inner {
Ok(merge::Command {
cmd: gix_command::prepare(gix_path::from_bstring(cmd))
.with_context(context)
.with_shell()
.command_may_be_shell_script()
.stdin(Stdio::null())
.stdout(Stdio::inherit())
.stderr(Stdio::inherit())
Expand Down
2 changes: 1 addition & 1 deletion gix-transport/src/client/blocking_io/ssh/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub fn connect(
.stderr(Stdio::null())
.stdout(Stdio::null())
.stdin(Stdio::null())
.with_shell()
.command_may_be_shell_script()
.arg("-G")
.arg(match url.host_as_argument() {
Usable(host) => host,
Expand Down
2 changes: 1 addition & 1 deletion gix-transport/src/client/blocking_io/ssh/program_kind.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl ProgramKind {
desired_version: Protocol,
disallow_shell: bool,
) -> Result<gix_command::Prepare, ssh::invocation::Error> {
let mut prepare = gix_command::prepare(ssh_cmd).with_shell();
let mut prepare = gix_command::prepare(ssh_cmd).command_may_be_shell_script();
if disallow_shell {
prepare.use_shell = false;
}
Expand Down
Loading