From a6b13a31b64a6c85463331dd439e82ce81b3d5c2 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 17 May 2021 22:44:12 -0500 Subject: [PATCH] Refactor shebang handling (#833) --- src/platform.rs | 14 +++--- src/platform_interface.rs | 3 +- src/recipe.rs | 47 ++++++++----------- src/shebang.rs | 95 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 38 deletions(-) diff --git a/src/platform.rs b/src/platform.rs index b08d90a..ad5b702 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -7,8 +7,7 @@ impl PlatformInterface for Platform { fn make_shebang_command( path: &Path, working_directory: &Path, - _command: &str, - _argument: Option<&str>, + _shebang: Shebang, ) -> Result { // shebang scripts can be executed directly on unix let mut cmd = Command::new(path); @@ -50,30 +49,29 @@ impl PlatformInterface for Platform { fn make_shebang_command( path: &Path, working_directory: &Path, - command: &str, - argument: Option<&str>, + shebang: Shebang, ) -> Result { use std::borrow::Cow; // If the path contains forward slashes… - let command = if command.contains('/') { + let command = if shebang.interpreter.contains('/') { // …translate path to the interpreter from unix style to windows style. let mut cygpath = Command::new("cygpath"); cygpath.current_dir(working_directory); cygpath.arg("--windows"); - cygpath.arg(command); + cygpath.arg(shebang.interpreter); Cow::Owned(output(cygpath)?) } else { // …otherwise use it as-is. - Cow::Borrowed(command) + Cow::Borrowed(shebang.interpreter) }; let mut cmd = Command::new(command.as_ref()); cmd.current_dir(working_directory); - if let Some(argument) = argument { + if let Some(argument) = shebang.argument { cmd.arg(argument); } diff --git a/src/platform_interface.rs b/src/platform_interface.rs index eca029d..0d78edc 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -6,8 +6,7 @@ pub(crate) trait PlatformInterface { fn make_shebang_command( path: &Path, working_directory: &Path, - command: &str, - argument: Option<&str>, + shebang: Shebang, ) -> Result; /// Set the execute permission on the file pointed to by `path` diff --git a/src/recipe.rs b/src/recipe.rs index 288b38c..993d2f8 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -113,12 +113,10 @@ impl<'src, D> Recipe<'src, D> { message: "evaluated_lines was empty".to_owned(), })?; - let Shebang { - interpreter, - argument, - } = Shebang::new(shebang_line).ok_or_else(|| RuntimeError::Internal { + let shebang = Shebang::new(shebang_line).ok_or_else(|| RuntimeError::Internal { message: format!("bad shebang line: {}", shebang_line), })?; + let tmp = tempfile::Builder::new() .prefix("just") .tempdir() @@ -127,26 +125,22 @@ impl<'src, D> Recipe<'src, D> { io_error: error, })?; let mut path = tmp.path().to_path_buf(); - let suffix = if interpreter.ends_with("cmd") || interpreter.ends_with("cmd.exe") { - ".bat" - } else if interpreter.ends_with("powershell") || interpreter.ends_with("powershell.exe") { - ".ps1" - } else { - "" - }; - path.push(format!("{}{}", self.name(), suffix)); + + path.push(shebang.script_filename(self.name())); + { let mut f = fs::File::create(&path).map_err(|error| RuntimeError::TmpdirIoError { recipe: self.name(), io_error: error, })?; let mut text = String::new(); - // add the shebang - if interpreter.ends_with("cmd") || interpreter.ends_with("cmd.exe") { - text += "\n"; - } else { + + if shebang.include_shebang_line() { text += &evaluated_lines[0]; + } else { + text += "\n"; } + text += "\n"; // add blank lines so that lines in the generated script have the same line // number as the corresponding lines in the justfile @@ -176,16 +170,13 @@ impl<'src, D> Recipe<'src, D> { })?; // create a command to run the script - let mut command = Platform::make_shebang_command( - &path, - &context.search.working_directory, - interpreter, - argument, - ) - .map_err(|output_error| RuntimeError::Cygpath { - recipe: self.name(), - output_error, - })?; + let mut command = + Platform::make_shebang_command(&path, &context.search.working_directory, shebang).map_err( + |output_error| RuntimeError::Cygpath { + recipe: self.name(), + output_error, + }, + )?; if context.settings.positional_arguments { command.args(positional); @@ -210,8 +201,8 @@ impl<'src, D> Recipe<'src, D> { Err(io_error) => { return Err(RuntimeError::Shebang { recipe: self.name(), - command: interpreter.to_owned(), - argument: argument.map(String::from), + command: shebang.interpreter.to_owned(), + argument: shebang.argument.map(String::from), io_error, }); }, diff --git a/src/shebang.rs b/src/shebang.rs index 1fe7a95..29f5330 100644 --- a/src/shebang.rs +++ b/src/shebang.rs @@ -1,3 +1,4 @@ +#[derive(Copy, Clone)] pub(crate) struct Shebang<'line> { pub(crate) interpreter: &'line str, pub(crate) argument: Option<&'line str>, @@ -28,6 +29,26 @@ impl<'line> Shebang<'line> { argument, }) } + + fn interpreter_filename(&self) -> &str { + self + .interpreter + .rsplit_once(|c| matches!(c, '/' | '\\')) + .map(|(_path, filename)| filename) + .unwrap_or(self.interpreter) + } + + pub(crate) fn script_filename(&self, recipe: &str) -> String { + match self.interpreter_filename() { + "cmd" | "cmd.exe" => format!("{}.bat", recipe), + "powershell" | "powershell.exe" => format!("{}.ps1", recipe), + _ => recipe.to_owned(), + } + } + + pub(crate) fn include_shebang_line(&self) -> bool { + !matches!(self.interpreter_filename(), "cmd" | "cmd.exe") + } } #[cfg(test)] @@ -93,4 +114,78 @@ mod tests { ); check("# /usr/bin/env python \t-x\t", None); } + + #[test] + fn interpreter_filename_with_forward_slash() { + assert_eq!( + Shebang::new("#!/foo/bar/baz") + .unwrap() + .interpreter_filename(), + "baz" + ); + } + + #[test] + fn interpreter_filename_with_backslash() { + assert_eq!( + Shebang::new("#!\\foo\\bar\\baz") + .unwrap() + .interpreter_filename(), + "baz" + ); + } + + #[test] + fn powershell_script_filename() { + assert_eq!( + Shebang::new("#!powershell").unwrap().script_filename("foo"), + "foo.ps1" + ); + } + + #[test] + fn powershell_exe_script_filename() { + assert_eq!( + Shebang::new("#!powershell.exe") + .unwrap() + .script_filename("foo"), + "foo.ps1" + ); + } + + #[test] + fn cmd_script_filename() { + assert_eq!( + Shebang::new("#!cmd").unwrap().script_filename("foo"), + "foo.bat" + ); + } + + #[test] + fn cmd_exe_script_filename() { + assert_eq!( + Shebang::new("#!cmd.exe").unwrap().script_filename("foo"), + "foo.bat" + ); + } + + #[test] + fn plain_script_filename() { + assert_eq!(Shebang::new("#!bar").unwrap().script_filename("foo"), "foo"); + } + + #[test] + fn dont_include_shebang_line_cmd() { + assert!(!Shebang::new("#!cmd").unwrap().include_shebang_line()); + } + + #[test] + fn dont_include_shebang_line_cmd_exe() { + assert!(!Shebang::new("#!cmd.exe /C").unwrap().include_shebang_line()); + } + + #[test] + fn include_shebang_line_other() { + assert!(Shebang::new("#!foo -c").unwrap().include_shebang_line()); + } }