Skip to content

Commit

Permalink
cmds: make "zsh" split words just like other shells
Browse files Browse the repository at this point in the history
Use the "zsh -o shwordsplit" option by default to improve portability
of garden commands. zsh does not split words by default, which makes
the behavior of $variable expressions different from bash.

Add a garden.shell-wordsplit option to opt-out of this behavior
and use the default zsh behavior where unquoted $variable expressions
are not subject to word splitting.
  • Loading branch information
davvid committed Feb 12, 2024
1 parent 11b37ed commit 33bf67f
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 32 deletions.
8 changes: 8 additions & 0 deletions doc/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
when a variable with a cyclical expression is evaluated.
([#24](https://github.com/davvid/garden/pull/24))

- When `zsh` is used as the `garden.shell`, which happens automatically when `zsh`
is installed, `garden` will now use `zsh -o shwordsplit` in order to enable
word-splitting of `$variable` expressions by default. This makes `zsh` behave
just like other shells by default, which improves the portability of commands.
Configure `garden.shell-wordsplit` to `false` or use the
`garden <cmd> -z | --no-wordsplit` option to opt-out of this behavior.
([#25](https://github.com/davvid/garden/pull/25))


## v1.2.1

Expand Down
10 changes: 9 additions & 1 deletion doc/src/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,17 @@ The `--no-errexit` option causes commands with multiple statements to run to com
even when a non-zero exit code is encountered. This is akin to a regular shell script.

Configure `garden.shell-errexit` to `false` in `garden.yaml` to opt-out of this behavior.
You can also opt-out of the `errexit` behavior on a per-command basis by adding
You can also opt-out of the `errexit` behavior on a per-command basis by using
`set +e` as the first line of a multi-line command.

When `zsh` is used it is executed with the `-o shwordsplit` option so that zsh behaves
similarly to traditional shells and splits words in unquoted `$variable` expressions
rather than treating `$variable` like a single argument.

Configure `garden.shell-wordsplit` to `false` to opt-out of this behavior.
You can also opt-out of the `shwordsplit` behavior on a per-command basis by using
`set +o shwordsplit` as the first line of a multi-line command.

Additional command-line `<arguments>` specified after a double-dash (`--`)
end-of-options marker are forwarded to each command.

Expand Down
63 changes: 44 additions & 19 deletions src/cmds/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,18 @@ pub struct CmdOptions {
/// Prevent the "errexit" shell option from being set. By default, the "-e" option
/// is passed to the configured shell so that multi-line and multi-statement
/// commands halt execution when the first statement with a non-zero exit code is
/// encountered. "--no-errexit" has the effect of making multi-line and
/// encountered. This option has the effect of making multi-line and
/// multi-statement commands run all statements even when an earlier statement
/// returns a non-zero exit code.
#[arg(long, short)]
no_errexit: bool,
#[arg(long = "no-errexit", short = 'n', default_value_t = true, action = clap::ArgAction::SetFalse)]
exit_on_error: bool,
/// Do not pass "-o shwordsplit" to zsh.
/// Prevent the "shwordsplit" shell option from being set when using zsh.
/// The "-o shwordsplit" option is passed to zsh by default so that unquoted
/// $variable expressions are subject to word splitting, just like other shells.
/// This option disables this behavior.
#[arg(long = "no-wordsplit", short = 'z', default_value_t = true, action = clap::ArgAction::SetFalse)]
word_split: bool,
/// Tree query for the gardens, groups or trees to execute commands within
query: String,
/// Custom commands to run over the resolved trees
Expand All @@ -52,11 +59,18 @@ pub struct CustomOptions {
/// Prevent the "errexit" shell option from being set. By default, the "-e" option
/// is passed to the configured shell so that multi-line and multi-statement
/// commands halt execution when the first statement with a non-zero exit code is
/// encountered. "--no-errexit" has the effect of making multi-line and
/// encountered. This option has the effect of making multi-line and
/// multi-statement commands run all statements even when an earlier statement
/// returns a non-zero exit code.
#[arg(long, short)]
no_errexit: bool,
#[arg(long = "no-errexit", short = 'n', default_value_t = true, action = clap::ArgAction::SetFalse)]
exit_on_error: bool,
/// Do not pass "-o shwordsplit" to zsh.
/// Prevent the "shwordsplit" shell option from being set when using zsh.
/// The "-o shwordsplit" option is passed to zsh by default so that unquoted
/// $variable expressions are subject to word splitting, just like other shells.
/// This option disables this behavior.
#[arg(long = "no-wordsplit", short = 'z', default_value_t = true, action = clap::ArgAction::SetFalse)]
word_split: bool,
/// Tree queries for the Gardens/Groups/Trees to execute commands within
// NOTE: value_terminator may not be needed in future versions of clap_complete.
// https://github.com/clap-rs/clap/pull/4612
Expand All @@ -76,7 +90,10 @@ pub fn main_cmd(app_context: &model::ApplicationContext, options: &mut CmdOption
debug!("trees: {:?}", options.trees);
}
if !app_context.get_root_config().shell_exit_on_error {
options.no_errexit = true;
options.exit_on_error = false;
}
if !app_context.get_root_config().shell_word_split {
options.word_split = false;
}
let params: CmdParams = options.clone().into();
let exit_status = cmd(app_context, &options.query, &params)?;
Expand All @@ -98,6 +115,8 @@ pub struct CmdParams {
keep_going: bool,
#[derivative(Default(value = "true"))]
exit_on_error: bool,
#[derivative(Default(value = "true"))]
word_split: bool,
}

/// Build CmdParams from a CmdOptions struct.
Expand All @@ -107,9 +126,10 @@ impl From<CmdOptions> for CmdParams {
commands: options.commands.clone(),
arguments: options.arguments.clone(),
breadth_first: options.breadth_first,
exit_on_error: !options.no_errexit,
exit_on_error: options.exit_on_error,
keep_going: options.keep_going,
tree_pattern: glob::Pattern::new(&options.trees).unwrap_or_default(),
word_split: options.word_split,
..Default::default()
}
}
Expand All @@ -128,8 +148,9 @@ impl From<CustomOptions> for CmdParams {
// --breadth-first was added to "garden cmd" and made opt-in.
breadth_first: true,
keep_going: options.keep_going,
exit_on_error: !options.no_errexit,
exit_on_error: options.exit_on_error,
tree_pattern: glob::Pattern::new(&options.trees).unwrap_or_default(),
word_split: options.word_split,
..Default::default()
};

Expand Down Expand Up @@ -159,7 +180,10 @@ pub fn main_custom(app_context: &model::ApplicationContext, arguments: &Vec<Stri
let mut options = <CustomOptions as FromArgMatches>::from_arg_matches(&matches)
.map_err(format_error::<CustomOptions>)?;
if !app_context.get_root_config().shell_exit_on_error {
options.no_errexit = true;
options.exit_on_error = false;
}
if !app_context.get_root_config().shell_word_split {
options.word_split = false;
}

if app_context.options.debug_level(constants::DEBUG_LEVEL_CMD) > 0 {
Expand Down Expand Up @@ -253,8 +277,7 @@ fn run_cmd_breadth_first(
&shell,
&env,
&cmd_seq_vec,
&params.arguments,
params.exit_on_error,
params,
) {
exit_status = cmd_status;
if !params.keep_going {
Expand Down Expand Up @@ -324,8 +347,7 @@ fn run_cmd_depth_first(
&shell,
&env,
&cmd_seq_vec,
&params.arguments,
params.exit_on_error,
params,
) {
exit_status = cmd_status;
if !params.keep_going {
Expand Down Expand Up @@ -353,8 +375,7 @@ fn run_cmd_vec(
shell: &str,
env: &Vec<(String, String)>,
cmd_seq_vec: &[Vec<String>],
arguments: &[String],
exit_on_error: bool,
params: &CmdParams,
) -> Result<(), i32> {
// Get the current executable name
let current_exe = cmd::current_exe();
Expand All @@ -375,6 +396,7 @@ fn run_cmd_vec(
| constants::SHELL_SH
| constants::SHELL_ZSH
);
let is_zsh = matches!(basename, constants::SHELL_ZSH);
// Does the shell use "-e <string>" or "-c <string>" to evaluate commands?
let use_dash_e = matches!(
basename,
Expand All @@ -394,7 +416,10 @@ fn run_cmd_vec(
);
}
let mut exec = subprocess::Exec::cmd(shell).cwd(path);
if exit_on_error && is_shell {
if params.word_split && is_zsh {
exec = exec.arg("-o").arg("shwordsplit");
}
if params.exit_on_error && is_shell {
exec = exec.arg("-e");
}
if use_dash_e {
Expand All @@ -406,7 +431,7 @@ fn run_cmd_vec(
if is_shell {
exec = exec.arg(current_exe.as_str());
}
exec = exec.args(arguments);
exec = exec.args(&params.arguments);
// Update the command environment
for (k, v) in env {
exec = exec.env(k, v);
Expand All @@ -416,7 +441,7 @@ fn run_cmd_vec(
let status = cmd::status(exec);
if status != errors::EX_OK {
exit_status = status;
if exit_on_error {
if params.exit_on_error {
return Err(status);
}
} else {
Expand Down
8 changes: 7 additions & 1 deletion src/cmds/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,17 @@ pub fn main(options: &cli::MainOptions, completion_options: &CompletionOptions)
.long("keep-going"),
)
.arg(
Arg::new("no_errexit")
Arg::new("no-errexit")
.help("Do not pass -e to the shell")
.short('n')
.long("no-errexit"),
)
.arg(
Arg::new("no-wordsplit")
.help("Do not pass -o shwordsplit to zsh")
.short('z')
.long("no-wordsplit"),
)
.arg(
Arg::new("queries")
// NOTE: value_terminator may not be needed in future versions of clap_complete.
Expand Down
21 changes: 19 additions & 2 deletions src/config/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,34 @@ fn parse_recursive(
) && config_verbose > 0
{
debug!(
"yaml: garden.shell-errexit = {}",
"config: {} = {}",
constants::GARDEN_SHELL_ERREXIT,
config.shell_exit_on_error
);
}
// garden.shell-wordsplit
if get_bool(
&doc[constants::GARDEN][constants::SHELL_WORDSPLIT],
&mut config.shell_word_split,
) && config_verbose > 0
{
debug!(
"config: {} = {}",
constants::GARDEN_SHELL_WORDSPLIT,
config.shell_word_split
);
}
// garden.tree-branches
if get_bool(
&doc[constants::GARDEN][constants::TREE_BRANCHES],
&mut config.tree_branches,
) && config_verbose > 0
{
debug!("yaml: garden.tree-branches = {}", config.tree_branches);
debug!(
"config: {} = {}",
constants::GARDEN_TREE_BRANCHES,
config.tree_branches
);
}

// GARDEN_ROOT and GARDEN_CONFIG_DIR are relative to the root configuration.
Expand Down
9 changes: 9 additions & 0 deletions src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ pub const GARDEN_CONFIG_DIR_EXPR: &str = "${GARDEN_CONFIG_DIR}";
/// Builtin variable for the "garden.root" location where trees are grown.
pub const GARDEN_ROOT: &str = "GARDEN_ROOT";

/// Command-line defines for overriding configurable behavior.
pub(crate) const GARDEN_SHELL: &str = "garden.shell";
pub(crate) const GARDEN_SHELL_ERREXIT: &str = "garden.shell-errexit";
pub(crate) const GARDEN_SHELL_WORDSPLIT: &str = "garden.shell-wordsplit";
pub(crate) const GARDEN_TREE_BRANCHES: &str = "garden.tree-branches";

/// The "gitconfig" section in a tree block defines local ".git/config"
/// settings that are applied when a tree is grown.
pub const GITCONFIG: &str = "gitconfig";
Expand Down Expand Up @@ -129,6 +135,9 @@ pub(crate) const SHELL_DASH: &str = "dash";
/// shell option.
pub const SHELL_ERREXIT: &str = "shell-errexit";

/// The "shell-wordsplit" key in the garden block disables the `zsh -o shwordsplit` option.
pub const SHELL_WORDSPLIT: &str = "shell-wordsplit";

/// KornShell is a standard/restricted command and programming language.
pub(crate) const SHELL_KSH: &str = "ksh";

Expand Down
18 changes: 12 additions & 6 deletions src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ pub struct Configuration {
pub override_variables: VariableHashMap,
pub verbose: u8,
pub(crate) shell_exit_on_error: bool,
pub(crate) shell_word_split: bool,
pub(crate) tree_branches: bool,
pub(crate) parent_id: Option<ConfigId>,
id: Option<ConfigId>,
Expand All @@ -518,6 +519,7 @@ impl Configuration {
parent_id: None,
shell: get_default_shell(),
shell_exit_on_error: true,
shell_word_split: true,
tree_branches: true,
..std::default::Default::default()
}
Expand Down Expand Up @@ -649,12 +651,10 @@ impl Configuration {
if config_verbose > 2 {
debug!("{}", self);
}

for key in &options.debug {
let current = *self.debug.get(key).unwrap_or(&0);
self.debug.insert(key.into(), current + 1);
}

for k_eq_v in &options.define {
let name: String;
let expr: String;
Expand All @@ -670,10 +670,16 @@ impl Configuration {
}
// Allow overridding garden.<value> using "garden -D garden.<value>=false".
match name.as_str() {
"garden.shell-errexit" => {
constants::GARDEN_SHELL => {
self.shell = expr;
}
constants::GARDEN_SHELL_ERREXIT => {
set_bool(name.as_str(), &expr, &mut self.shell_exit_on_error);
}
"garden.tree-branches" => {
constants::GARDEN_SHELL_WORDSPLIT => {
set_bool(name.as_str(), &expr, &mut self.shell_word_split);
}
constants::GARDEN_TREE_BRANCHES => {
set_bool(name.as_str(), &expr, &mut self.tree_branches);
}
_ => {
Expand Down Expand Up @@ -1359,10 +1365,10 @@ impl ApplicationContext {
let path = path.to_path_buf();
let config_verbose = self.options.debug_level(constants::DEBUG_LEVEL_CONFIG);
let mut graft_config = Configuration::new();
// Propagate the current config's "garden.tree-branches" and "garden.shell_exit_on_error"
// settings onto child grafts.
// Propagate the current config's settings onto child grafts.
graft_config.tree_branches = self.get_config(config_id).tree_branches;
graft_config.shell_exit_on_error = self.get_config(config_id).shell_exit_on_error;
graft_config.shell_word_split = self.get_config(config_id).shell_word_split;
// Parse the config file for the graft.
graft_config.update(self, Some(&path), root, config_verbose, Some(config_id))?;

Expand Down
7 changes: 6 additions & 1 deletion tests/data/garden.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ trees:
echo-pre-and-post-nested>: echo-pre-and-post-nested-after
echo-pre-and-post-nested-after>: echo-pre-and-post-nested-fini
echo-pre-and-post-nested-fini: echo fini

echo-wordsplit-variable: |
abc='a b c'
for arg in $abc
do
echo $arg
done
example/shallow:
path: example/tree/shallow
url: file://${repos}/example.git
Expand Down
Loading

0 comments on commit 33bf67f

Please sign in to comment.