From 522501702d4e1b5066a0f40cfac491e9f8155156 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Tue, 19 Sep 2023 16:59:47 -0700 Subject: [PATCH] Improve the select boot failure help. (#145) This fixes the error messages displayed when a boot command could not be found by both differentiating the cases where this can occur with their own specialized messages and improving the guidance in general. As part of these messaging fixes, support for hiding commands is added by omitting descriptions for those named commands you wish to hide from the boot failure help screen. Fixes #143 Fixes #144 --- CHANGES.md | 7 + Cargo.lock | 4 +- Cargo.toml | 2 +- docs/packaging.md | 4 +- .../default-only-lift.linux-x86_64.json | 21 +++ examples/busybox/lift.linux-x86_64.json | 18 +++ ...xed-no-default-desc-lift.linux-x86_64.json | 36 +++++ ...d-with-default-desc-lift.linux-x86_64.json | 37 +++++ ...mmands-only-no-desc-lift.linux-x86_64.json | 25 +++ ...ands-only-with-desc-lift.linux-x86_64.json | 31 ++++ examples/busybox/test.sh | 143 ++++++++++++++++++ jump/Cargo.toml | 2 +- jump/src/context.rs | 46 +++--- jump/src/lib.rs | 55 ++++--- src/boot.rs | 108 +++++++++---- 15 files changed, 467 insertions(+), 72 deletions(-) create mode 100644 examples/busybox/default-only-lift.linux-x86_64.json create mode 100644 examples/busybox/lift.linux-x86_64.json create mode 100644 examples/busybox/mixed-no-default-desc-lift.linux-x86_64.json create mode 100644 examples/busybox/mixed-with-default-desc-lift.linux-x86_64.json create mode 100644 examples/busybox/named-commands-only-no-desc-lift.linux-x86_64.json create mode 100644 examples/busybox/named-commands-only-with-desc-lift.linux-x86_64.json create mode 100644 examples/busybox/test.sh diff --git a/CHANGES.md b/CHANGES.md index 8c6af7d..b6475f3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,12 @@ # Release Notes +## 0.13.0 + +This release improves the help screen for BusyBox scies with more clear messages for the various +causes of boot command selection failure. It also adds the ability to hide internal-only named boot +commands by omitting a description for those commands (This only kicks in if at least one named +command has a description). + ## 0.12.0 This release adds support for using placeholders in the `scie.lift.base` lift manifest value as well diff --git a/Cargo.lock b/Cargo.lock index 9729cad..2f44c5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -427,7 +427,7 @@ dependencies = [ [[package]] name = "jump" -version = "0.12.0" +version = "0.13.0" dependencies = [ "bstr", "byteorder", @@ -748,7 +748,7 @@ dependencies = [ [[package]] name = "scie-jump" -version = "0.12.0" +version = "0.13.0" dependencies = [ "bstr", "env_logger", diff --git a/Cargo.toml b/Cargo.toml index 79c7e0d..fbea3f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ members = [ [package] name = "scie-jump" -version = "0.12.0" +version = "0.13.0" description = "The self contained interpreted executable launcher." authors = [ "John Sirois ", diff --git a/docs/packaging.md b/docs/packaging.md index abf29a5..45c6018 100644 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -288,7 +288,9 @@ variable, e.g.: `SCIE_BOOT=some_other_command ./coursier`. If there is no defaul and the `SCIE_BOOT` environment variable is not set, a help screen will be printed listing all the commands you defined in the lift manifest. You can add a "lift.description" to provide overall help in this help page as well as a "description" for each command to provide help displayed after the -command name. +command name. If any named command has a description (or there is a default command), then only +named commands with descriptions will appear in the help page. You can use this behavior to hide +internal-only commands by giving them no description. This style of multi-command scie with no default command is called a [BusyBox]( https://busybox.net/), and it functions like one. Instead of using `SCIE_BOOT` to address a command, diff --git a/examples/busybox/default-only-lift.linux-x86_64.json b/examples/busybox/default-only-lift.linux-x86_64.json new file mode 100644 index 0000000..33a9d96 --- /dev/null +++ b/examples/busybox/default-only-lift.linux-x86_64.json @@ -0,0 +1,21 @@ +{ + "scie": { + "lift": { + "name": "default-only", + "boot": { + "commands": { + "": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "import sys; print('.'.join(map(str, sys.version_info[:3])))"] + } + } + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz", + "key": "cpython" + } + ] + } + } +} diff --git a/examples/busybox/lift.linux-x86_64.json b/examples/busybox/lift.linux-x86_64.json new file mode 100644 index 0000000..b5b8f1d --- /dev/null +++ b/examples/busybox/lift.linux-x86_64.json @@ -0,0 +1,18 @@ +{ + "scie": { + "lift": { + "name": "no-commands", + "boot": { + "commands": {} + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz" + } + ] + } + }, + "fetch": [ + "https://github.com/indygreg/python-build-standalone/releases/download/20230826/cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz" + ] +} diff --git a/examples/busybox/mixed-no-default-desc-lift.linux-x86_64.json b/examples/busybox/mixed-no-default-desc-lift.linux-x86_64.json new file mode 100644 index 0000000..479e0e3 --- /dev/null +++ b/examples/busybox/mixed-no-default-desc-lift.linux-x86_64.json @@ -0,0 +1,36 @@ +{ + "scie": { + "lift": { + "name": "mixed-no-default-desc", + "description": "The scie's overall description.", + "boot": { + "commands": { + "": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "import sys; print('.'.join(map(str, sys.version_info[:3])))"] + }, + "foo": { + "description": "Prints foo.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('foo')"] + }, + "bar": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('bar')"] + }, + "runs-baz": { + "description": "Runs baz.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('ran baz')"] + } + } + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz", + "key": "cpython" + } + ] + } + } +} diff --git a/examples/busybox/mixed-with-default-desc-lift.linux-x86_64.json b/examples/busybox/mixed-with-default-desc-lift.linux-x86_64.json new file mode 100644 index 0000000..4b0e1c6 --- /dev/null +++ b/examples/busybox/mixed-with-default-desc-lift.linux-x86_64.json @@ -0,0 +1,37 @@ +{ + "scie": { + "lift": { + "name": "mixed-with-default-desc", + "description": "The scie's overall description.", + "boot": { + "commands": { + "": { + "description": "Prints the Python version.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "import sys; print('.'.join(map(str, sys.version_info[:3])))"] + }, + "foo": { + "description": "Prints foo.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('foo')"] + }, + "bar": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('bar')"] + }, + "runs-baz": { + "description": "Runs baz.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('ran baz')"] + } + } + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz", + "key": "cpython" + } + ] + } + } +} diff --git a/examples/busybox/named-commands-only-no-desc-lift.linux-x86_64.json b/examples/busybox/named-commands-only-no-desc-lift.linux-x86_64.json new file mode 100644 index 0000000..afd8dcc --- /dev/null +++ b/examples/busybox/named-commands-only-no-desc-lift.linux-x86_64.json @@ -0,0 +1,25 @@ +{ + "scie": { + "lift": { + "name": "named-commands-only-no-desc", + "boot": { + "commands": { + "foo": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('foo')"] + }, + "bar": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('bar')"] + } + } + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz", + "key": "cpython" + } + ] + } + } +} diff --git a/examples/busybox/named-commands-only-with-desc-lift.linux-x86_64.json b/examples/busybox/named-commands-only-with-desc-lift.linux-x86_64.json new file mode 100644 index 0000000..19499f5 --- /dev/null +++ b/examples/busybox/named-commands-only-with-desc-lift.linux-x86_64.json @@ -0,0 +1,31 @@ +{ + "scie": { + "lift": { + "name": "named-commands-only-with-desc", + "boot": { + "commands": { + "foo": { + "description": "Prints foo.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('foo')"] + }, + "bar": { + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('bar')"] + }, + "runs-baz": { + "description": "Runs baz.", + "exe": "{cpython}/python/bin/python3.11", + "args": ["-c", "print('ran baz')"] + } + } + }, + "files": [ + { + "name": "cpython-3.11.5+20230826-x86_64-unknown-linux-gnu-install_only.tar.gz", + "key": "cpython" + } + ] + } + } +} diff --git a/examples/busybox/test.sh b/examples/busybox/test.sh new file mode 100644 index 0000000..46a49da --- /dev/null +++ b/examples/busybox/test.sh @@ -0,0 +1,143 @@ +# Copyright 2022 Science project contributors. +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +# shellcheck source=../common.sh +source "${COMMON}" +trap gc EXIT + +check_cmd diff mktemp + +OUTPUT="$(mktemp)" +gc "${OUTPUT}" + + +# Test no boot commands error. +"${SCIE_JUMP}" "${LIFT}" +gc "${PWD}/no-commands${EXE_EXT}" + +./no-commands 2>"${OUTPUT}" && die "Expected ./no-commands to fail to execute." +diff -u \ +<(cat <"${OUTPUT}" && die "Expected SCIE_BOOT=dne ./default-only to fail to execute." +diff -u \ +<(cat <"${OUTPUT}" && die "Expected SCIE_BOOT=dne ./named-commands-only-no-desc to fail to execute." +diff -u \ +<(cat <"${OUTPUT}" && die "Expected SCIE_BOOT=dne ./named-commands-only-with-desc to fail to execute." +diff -u \ +<(cat <"${OUTPUT}" && die "Expected SCIE_BOOT=dne ./mixed-no-default-desc to fail to execute." +diff -u \ +<(cat < (when SCIE_BOOT is not set in the environment) +foo Prints foo. +runs-baz Runs baz. + +You can select a boot command by setting the SCIE_BOOT environment variable. +EOF +) "${OUTPUT}" + + +# Test default command with its own description. +"${SCIE_JUMP}" "mixed-with-default-desc-lift.${OS_ARCH}.json" +gc "${PWD}/mixed-with-default-desc${EXE_EXT}" + +SCIE_BOOT=dne ./mixed-with-default-desc 2>"${OUTPUT}" && die "Expected SCIE_BOOT=dne ./mixed-with-default-desc to fail to execute." +diff -u \ +<(cat < (when SCIE_BOOT is not set in the environment) Prints the Python version. +foo Prints foo. +runs-baz Runs baz. + +You can select a boot command by setting the SCIE_BOOT environment variable. +EOF +) "${OUTPUT}" diff --git a/jump/Cargo.toml b/jump/Cargo.toml index e1ecb25..c90d4ee 100644 --- a/jump/Cargo.toml +++ b/jump/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "jump" -version = "0.12.0" +version = "0.13.0" description = "The bulk of the scie-jump binary logic." authors = [ "John Sirois ", diff --git a/jump/src/context.rs b/jump/src/context.rs index 168646d..09b76a3 100644 --- a/jump/src/context.rs +++ b/jump/src/context.rs @@ -212,10 +212,6 @@ pub(crate) struct Context<'a> { installed: HashSet, } -fn try_as_str(os_str: &OsStr) -> Option<&str> { - <[u8]>::from_os_str(os_str).and_then(|bytes| std::str::from_utf8(bytes).ok()) -} - impl<'a> Context<'a> { #[time("debug", "Context::{}")] fn new( @@ -432,7 +428,8 @@ impl<'a> Context<'a> { Ok(None) } - fn select_command(&mut self, invoked_as: &Path) -> Result, String> { + fn select_command(&mut self, scie_name: &str, exe: &CurrentExe) -> Result { + // Forced command. if let Some(cmd) = env::var_os("SCIE_BOOT") { // Avoid subprocesses that re-execute this SCIE unintentionally getting in an infinite // loop. @@ -440,27 +437,36 @@ impl<'a> Context<'a> { let name = cmd.into_string().map_err(|value| { format!("Failed to decode environment variable SCIE_BOOT: {value:?}") })?; - return self.select_cmd(&name, false); + if let Some(selected_cmd) = self.select_cmd(&name, false)? { + return Ok(selected_cmd); + } else { + return Err(format!( + "`SCIE_BOOT={name}` was found in the environment but \"{name}\" does \ + not correspond to any {scie_name} commands." + )); + } } + + // Default command. if let Some(selected_cmd) = self.select_cmd("", false)? { - return Ok(Some(selected_cmd)); + return Ok(selected_cmd); } - #[cfg(windows)] - let basename = invoked_as.file_stem().and_then(try_as_str); - - #[cfg(unix)] - let basename = invoked_as.file_name().and_then(try_as_str); - - if let Some(basename) = basename { - if let Some(selected_command) = self.select_cmd(basename, false)? { - return Ok(Some(selected_command)); + // BusyBox style where basename indicates command name. + if let Some(name) = exe.name() { + if let Some(selected_command) = self.select_cmd(name, false)? { + return Ok(selected_command); } } + + // BusyBox style where 1st arg indicates command name. if let Some(argv1) = env::args().nth(1) { - return self.select_cmd(&argv1, true); + if let Some(selected_cmd) = self.select_cmd(&argv1, true)? { + return Ok(selected_cmd); + } } - Ok(None) + + Err("Could not determine which command to run.".to_string()) } fn get_path(&self, file: &File) -> PathBuf { @@ -652,9 +658,9 @@ pub(crate) fn select_command( jump: &Jump, lift: &Lift, installer: &Installer, -) -> Result, String> { +) -> Result { let mut context = Context::new(¤t_exe.exe, jump, lift, installer)?; - context.select_command(¤t_exe.invoked_as) + context.select_command(lift.name.as_str(), current_exe) } #[cfg(test)] diff --git a/jump/src/lib.rs b/jump/src/lib.rs index 05a2112..92a0923 100644 --- a/jump/src/lib.rs +++ b/jump/src/lib.rs @@ -39,9 +39,10 @@ pub use crate::process::{execute, EnvVar, EnvVars, Process}; pub use crate::zip::check_is_zip; pub struct SelectBoot { + pub scie: CurrentExe, pub boots: Vec, pub description: Option, - pub error_message: Option, + pub error_message: String, } const HELP: &str = "\ @@ -93,11 +94,30 @@ pub fn config(jump: Jump, mut lift: Lift) -> Config { Config::new(jump, lift, other) } -struct CurrentExe { +pub struct CurrentExe { exe: PathBuf, invoked_as: PathBuf, } +impl CurrentExe { + pub fn name(&self) -> Option<&str> { + #[cfg(windows)] + let invoked_as = self.invoked_as.file_stem(); + + #[cfg(unix)] + let invoked_as = self.invoked_as.file_name(); + + invoked_as.and_then(|basename| basename.to_str()) + } + + pub fn invoked_as(&self) -> String { + self.invoked_as + .to_str() + .map(|path| path.to_string()) + .unwrap_or_else(|| format!("{}", self.invoked_as.display())) + } +} + fn find_current_exe() -> Result { let exe = current_exe().map_err(|e| format!("Failed to find path of the current executable: {e}"))?; @@ -166,22 +186,23 @@ pub fn prepare_boot() -> Result { } let payload = &data[jump.size..data.len() - lift.size]; let installer = Installer::new(payload); - let result = context::select_command(¤t_exe, &jump, &lift, &installer); - if let Ok(Some(selected_command)) = result { - installer.install(&selected_command.files)?; - let process = selected_command.process; - trace!("Prepared {process:#?}"); - env::set_var("SCIE", current_exe.exe.as_os_str()); - env::set_var("SCIE_ARGV0", current_exe.invoked_as.as_os_str()); - Ok(BootAction::Execute(( - process, - selected_command.argv1_consumed, - ))) - } else { - Ok(BootAction::Select(SelectBoot { + match context::select_command(¤t_exe, &jump, &lift, &installer) { + Ok(selected_command) => { + installer.install(&selected_command.files)?; + let process = selected_command.process; + trace!("Prepared {process:#?}"); + env::set_var("SCIE", current_exe.exe.as_os_str()); + env::set_var("SCIE_ARGV0", current_exe.invoked_as.as_os_str()); + Ok(BootAction::Execute(( + process, + selected_command.argv1_consumed, + ))) + } + Err(error_message) => Ok(BootAction::Select(SelectBoot { + scie: current_exe, boots: lift.boots(), description: lift.description, - error_message: result.err(), - })) + error_message, + })), } } diff --git a/src/boot.rs b/src/boot.rs index 02314ec..20dd0f9 100644 --- a/src/boot.rs +++ b/src/boot.rs @@ -33,45 +33,93 @@ pub(crate) fn inspect(jump: Jump, lift: Lift) -> ExitResult { } pub(crate) fn select(select_boot: SelectBoot) -> ExitResult { - let header = if select_boot.boots.iter().any(|boot| boot.default) { - "" - } else { - "This Scie binary has no default boot command.\n" - }; + let default_cmd = select_boot + .boots + .iter() + .find(|boot| boot.default) + .map(|boot| { + ( + " (when SCIE_BOOT is not set in the environment)".to_string(), + boot.description.as_ref().cloned().unwrap_or_default(), + ) + }); + let mut selectable_cmds = select_boot + .boots + .iter() + .filter(|boot| !boot.default) + .filter_map(|boot| { + boot.description + .as_ref() + .map(|desc| (boot.name.clone(), desc.clone())) + }) + .collect::>(); + + // Only include hidden named commands when that's all there is. + if selectable_cmds.is_empty() && default_cmd.is_none() { + selectable_cmds.extend( + select_boot + .boots + .iter() + .filter(|boot| !boot.default) + .map(|boot| (boot.name.clone(), "".to_string())), + ); + } + + if selectable_cmds.is_empty() && default_cmd.is_none() { + return Err(Code::FAILURE.with_message(format!( + "The {scie} scie is malformed - it has no boot commands.\n\ + \n\ + You might begin debugging by inspecting the output of `SCIE=inspect {scie}`.", + scie = select_boot.scie.invoked_as() + ))); + } + + if default_cmd.is_some() && selectable_cmds.is_empty() { + return Err(Code::FAILURE.with_message(format!( + "{error_message}\n\ + \n\ + The {scie} scie contains no alternate boot commands.", + scie = select_boot.scie.invoked_as(), + error_message = select_boot.error_message + ))); + } + + let maybe_scie_description = select_boot + .description + .map(|description| format!("{description}\n\n")) + .unwrap_or_default(); + let max_name_width = default_cmd + .iter() + .chain(selectable_cmds.iter()) + .map(|(name, _)| name.len()) + .max() + .expect("We verified we have at least one boot command earlier"); Err(Code::FAILURE.with_message(format!( - "{description}\n\ + "{error_message}\n\ + \n\ + {maybe_scie_description}\ Please select from the following boot commands:\n\ \n\ {boot_commands}\n\ \n\ - You can select a boot command by passing it as the 1st argument or else by \ - setting the SCIE_BOOT environment variable.\n\ - {error_message}", - description = select_boot - .description - .map(|message| format!("{header}{message}\n")) - .unwrap_or_default(), - boot_commands = select_boot - .boots - .into_iter() - .map(|boot| if let Some(description) = boot.description { - format!( - "{name}: {description}", - name = if boot.default { - "" - } else { - boot.name.as_str() - } - ) + You can select a boot command by setting the SCIE_BOOT environment variable\ + {or_else_by}.", + boot_commands = default_cmd + .iter() + .chain(selectable_cmds.iter()) + .map(|(name, description)| if description.is_empty() { + name.to_string() } else { - boot.name + format!("{name:>() .join("\n"), - error_message = select_boot - .error_message - .map(|err| format!("\nERROR: {err}")) - .unwrap_or_default() + or_else_by = if default_cmd.is_none() { + " or else by passing it as the 1st argument" + } else { + "" + }, + error_message = select_boot.error_message ))) }