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

Fix windows shell setting precedence #1306

Merged
merged 21 commits into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
with:
components: clippy, rustfmt
override: true
toolchain: 1.53.0
toolchain: 1.54.0

- uses: Swatinem/rust-cache@v1

Expand Down
16 changes: 15 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,8 @@ foo:
| `export` | boolean | Export all variables as environment variables. |
| `positional-arguments` | boolean | Pass positional arguments. |
| `shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. |
| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. |
| `windows-shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. |
| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. (Deprecated. Use `windows-shell` instead. |

Boolean settings can be written as:

Expand Down Expand Up @@ -775,6 +776,8 @@ hello:
Write-Host "Hello, world!"
```

See [powershell.just](https://github.com/casey/just/blob/master/examples/powershell.just) for a justfile that uses PowerShell on all platforms.

##### Windows PowerShell

*`set windows-powershell` uses the legacy `powershell.exe` binary, and is no longer recommended. See the `windows-shell` setting above for a more flexible way to control which shell is used on Windows.*
Expand Down Expand Up @@ -2126,6 +2129,17 @@ foo $argument:

This defeats `just`'s ability to catch typos, for example if you type `$argumant`, but works for all possible values of `argument`, including those with double quotes.

### Configuring the Shell

There are a number of ways to configure the shell for linewise recipes, which are the default when a recipe does not start with a `#!` shebang. Their precedence, from highest to lowest, is:

1. The `--shell` and `--shell-arg` command line options. Passing either of these will cause `just` to ignore any settings in the current justfile.
2. `set windows-shell := [...]`
3. `set windows-powershell` (deprecated)
4. `set shell := [...]`

Since `set windows-shell` has higher precedence than `set shell`, you can use `set windows-shell` to pick a shell on Windows, and `set shell` to pick a shell for all other platforms.

Changelog
---------

Expand Down
29 changes: 29 additions & 0 deletions examples/powershell.just
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Cross platform shebang:
shebang := if os() == 'windows' {
'powershell.exe'
} else {
'/usr/bin/env pwsh'
}

# Set shell for non-Windows OSs:
set shell := ["powershell", "-c"]

# Set shell for Windows OSs:
set windows-shell := ["powershell.exe", "-NoLogo", "-Command"]

# If you have PowerShell Core installed and want to use it,
# use `pwsh.exe` instead of `powershell.exe`

linewise:
Write-Host "Hello, world!"

shebang:
#!{{shebang}}
$PSV = $PSVersionTable.PSVersion | % {"$_" -split "\." }
$psver = $PSV[0] + "." + $PSV[1]
if ($PSV[2].Length -lt 4) {
$psver += "." + $PSV[2] + " Core"
} else {
$psver += " Desktop"
}
echo "PowerShell $psver"
106 changes: 46 additions & 60 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,50 +30,48 @@ impl<'src> Settings<'src> {
}

pub(crate) fn shell_command(&self, config: &Config) -> Command {
let mut cmd = Command::new(self.shell_binary(config));
let (command, args) = self.shell(config);

cmd.args(self.shell_arguments(config));
let mut cmd = Command::new(command);

cmd
}
cmd.args(args);

pub(crate) fn shell_binary<'a>(&'a self, config: &'a Config) -> &'a str {
let shell_or_args_present = config.shell.is_some() || config.shell_args.is_some();

if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
shell.command.cooked.as_ref()
} else if let Some(shell) = &config.shell {
shell
} else if let (true, Some(shell)) = (cfg!(windows), &self.windows_shell) {
shell.command.cooked.as_ref()
} else if cfg!(windows) && self.windows_powershell {
WINDOWS_POWERSHELL_SHELL
} else {
DEFAULT_SHELL
}
cmd
}

pub(crate) fn shell_arguments<'a>(&'a self, config: &'a Config) -> Vec<&'a str> {
let shell_or_args_present = config.shell.is_some() || config.shell_args.is_some();

if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
shell
.arguments
.iter()
.map(|argument| argument.cooked.as_ref())
.collect()
} else if let Some(shell_args) = &config.shell_args {
shell_args.iter().map(String::as_ref).collect()
} else if let (true, Some(shell)) = (cfg!(windows), &self.windows_shell) {
shell
.arguments
.iter()
.map(|argument| argument.cooked.as_ref())
.collect()
} else if cfg!(windows) && self.windows_powershell {
WINDOWS_POWERSHELL_ARGS.to_vec()
} else {
DEFAULT_SHELL_ARGS.to_vec()
pub(crate) fn shell<'a>(&'a self, config: &'a Config) -> (&'a str, Vec<&'a str>) {
match (&config.shell, &config.shell_args) {
(Some(shell), Some(shell_args)) => (shell, shell_args.iter().map(String::as_ref).collect()),
(Some(shell), None) => (shell, DEFAULT_SHELL_ARGS.to_vec()),
(None, Some(shell_args)) => (
DEFAULT_SHELL,
shell_args.iter().map(String::as_ref).collect(),
),
(None, None) => {
if let (true, Some(shell)) = (cfg!(windows), &self.windows_shell) {
(
shell.command.cooked.as_ref(),
shell
.arguments
.iter()
.map(|argument| argument.cooked.as_ref())
.collect(),
)
} else if cfg!(windows) && self.windows_powershell {
(WINDOWS_POWERSHELL_SHELL, WINDOWS_POWERSHELL_ARGS.to_vec())
} else if let Some(shell) = &self.shell {
(
shell.command.cooked.as_ref(),
shell
.arguments
.iter()
.map(|argument| argument.cooked.as_ref())
.collect(),
)
} else {
(DEFAULT_SHELL, DEFAULT_SHELL_ARGS.to_vec())
}
}
}
}
}
Expand All @@ -91,8 +89,7 @@ mod tests {
..testing::config(&[])
};

assert_eq!(settings.shell_binary(&config), "sh");
assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
assert_eq!(settings.shell(&config), ("sh", vec!["-cu"]));
}

#[test]
Expand All @@ -106,14 +103,12 @@ mod tests {
};

if cfg!(windows) {
assert_eq!(settings.shell_binary(&config), "powershell.exe");
assert_eq!(
settings.shell_arguments(&config),
vec!["-NoLogo", "-Command"]
settings.shell(&config),
("powershell.exe", vec!["-NoLogo", "-Command"])
);
} else {
assert_eq!(settings.shell_binary(&config), "sh");
assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
assert_eq!(settings.shell(&config), ("sh", vec!["-cu"]));
}
}

Expand All @@ -128,8 +123,7 @@ mod tests {
..testing::config(&[])
};

assert_eq!(settings.shell_binary(&config), "lol");
assert_eq!(settings.shell_arguments(&config), vec!["-nice"]);
assert_eq!(settings.shell(&config), ("lol", vec!["-nice"]));
}

#[test]
Expand All @@ -144,8 +138,7 @@ mod tests {
..testing::config(&[])
};

assert_eq!(settings.shell_binary(&config), "lol");
assert_eq!(settings.shell_arguments(&config), vec!["-nice"]);
assert_eq!(settings.shell(&config), ("lol", vec!["-nice"]));
}

#[test]
Expand All @@ -170,8 +163,7 @@ mod tests {
..testing::config(&[])
};

assert_eq!(settings.shell_binary(&config), "asdf.exe");
assert_eq!(settings.shell_arguments(&config), vec!["-nope"]);
assert_eq!(settings.shell(&config), ("asdf.exe", vec!["-nope"]));
}

#[test]
Expand All @@ -184,7 +176,7 @@ mod tests {
..testing::config(&[])
};

assert_eq!(settings.shell_binary(&config), "lol");
assert_eq!(settings.shell(&config).0, "lol");
}

#[test]
Expand All @@ -198,12 +190,6 @@ mod tests {
..testing::config(&[])
};

if cfg!(windows) {
assert_eq!(settings.shell_binary(&config), "powershell.exe");
} else {
assert_eq!(settings.shell_binary(&config), "sh");
}

assert_eq!(settings.shell_arguments(&config), vec!["-nice"]);
assert_eq!(settings.shell(&config), ("sh", vec!["-nice"]));
}
}
5 changes: 3 additions & 2 deletions src/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,10 @@ impl Subcommand {
let mut child = match result {
Ok(child) => child,
Err(io_error) => {
let (shell_binary, shell_arguments) = justfile.settings.shell(config);
return Err(Error::ChooserInvoke {
shell_binary: justfile.settings.shell_binary(config).to_owned(),
shell_arguments: justfile.settings.shell_arguments(config).join(" "),
shell_binary: shell_binary.to_owned(),
shell_arguments: shell_arguments.join(" "),
chooser,
io_error,
});
Expand Down
2 changes: 0 additions & 2 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,5 @@ mod subsequents;
mod tempdir;
mod undefined_variables;
#[cfg(target_family = "windows")]
mod windows_powershell;
#[cfg(target_family = "windows")]
mod windows_shell;
mod working_directory;
18 changes: 0 additions & 18 deletions tests/windows_powershell.rs

This file was deleted.

36 changes: 36 additions & 0 deletions tests/windows_shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,42 @@ fn windows_shell_setting() {
.justfile(
r#"
set windows-shell := ["pwsh.exe", "-NoLogo", "-Command"]
set shell := ["asdfasdfasdfasdf"]

foo:
Write-Output bar
"#,
)
.shell(false)
.stdout("bar\r\n")
.stderr("Write-Output bar\n")
.run();
}

#[test]
fn windows_powershell_setting_uses_powershell() {
Test::new()
.justfile(
r#"
set windows-powershell
set shell := ["asdfasdfasdfasdf"]

foo:
Write-Output bar
"#,
)
.shell(false)
.stdout("bar\r\n")
.stderr("Write-Output bar\n")
.run();
}

#[test]
fn windows_poweshell_setting_uses_powershell() {
Test::new()
.justfile(
r#"
set windows-powershell

foo:
Write-Output bar
Expand Down