From 5eabe37ab8cae3819257d8fddcb1454bba311601 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 15:05:51 -0700 Subject: [PATCH 01/23] Add `[script]` attribute --- src/attribute.rs | 41 ++++++++++++------ src/error.rs | 11 ++++- src/lib.rs | 1 + src/parser.rs | 35 ++++++++++----- src/platform.rs | 6 +-- src/recipe.rs | 108 ++++++++++++++++++++++++++++++++++++----------- tests/confirm.rs | 16 +++++-- tests/lib.rs | 1 + tests/script.rs | 16 +++++++ 9 files changed, 179 insertions(+), 56 deletions(-) create mode 100644 tests/script.rs diff --git a/src/attribute.rs b/src/attribute.rs index 699d1495e1..41d58e3316 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -20,6 +20,7 @@ pub(crate) enum Attribute<'src> { NoQuiet, PositionalArguments, Private, + Script(Vec>), Unix, Windows, } @@ -38,6 +39,7 @@ impl AttributeDiscriminant { | Self::Private | Self::Unix | Self::Windows => 0..=0, + Self::Script => 1..=usize::MAX, } } } @@ -45,7 +47,7 @@ impl AttributeDiscriminant { impl<'src> Attribute<'src> { pub(crate) fn new( name: Name<'src>, - argument: Option>, + arguments: Vec>, ) -> CompileResult<'src, Self> { let discriminant = name .lexeme() @@ -57,7 +59,7 @@ impl<'src> Attribute<'src> { }) })?; - let found = argument.as_ref().iter().count(); + let found = arguments.len(); let range = discriminant.argument_range(); if !range.contains(&found) { return Err( @@ -71,10 +73,10 @@ impl<'src> Attribute<'src> { } Ok(match discriminant { - AttributeDiscriminant::Confirm => Self::Confirm(argument), - AttributeDiscriminant::Doc => Self::Doc(argument), - AttributeDiscriminant::Extension => Self::Extension(argument.unwrap()), - AttributeDiscriminant::Group => Self::Group(argument.unwrap()), + AttributeDiscriminant::Confirm => Self::Confirm(arguments.into_iter().next()), + AttributeDiscriminant::Doc => Self::Doc(arguments.into_iter().next()), + AttributeDiscriminant::Extension => Self::Extension(arguments.into_iter().next().unwrap()), + AttributeDiscriminant::Group => Self::Group(arguments.into_iter().next().unwrap()), AttributeDiscriminant::Linux => Self::Linux, AttributeDiscriminant::Macos => Self::Macos, AttributeDiscriminant::NoCd => Self::NoCd, @@ -82,6 +84,7 @@ impl<'src> Attribute<'src> { AttributeDiscriminant::NoQuiet => Self::NoQuiet, AttributeDiscriminant::PositionalArguments => Self::PositionalArguments, AttributeDiscriminant::Private => Self::Private, + AttributeDiscriminant::Script => Self::Script(arguments), AttributeDiscriminant::Unix => Self::Unix, AttributeDiscriminant::Windows => Self::Windows, }) @@ -91,10 +94,10 @@ impl<'src> Attribute<'src> { self.into() } - fn argument(&self) -> Option<&StringLiteral> { + fn arguments(&self) -> &[StringLiteral] { match self { - Self::Confirm(argument) | Self::Doc(argument) => argument.as_ref(), - Self::Extension(argument) | Self::Group(argument) => Some(argument), + Self::Confirm(argument) | Self::Doc(argument) => argument.as_slice(), + Self::Extension(argument) | Self::Group(argument) => slice::from_ref(argument), Self::Linux | Self::Macos | Self::NoCd @@ -103,7 +106,8 @@ impl<'src> Attribute<'src> { | Self::PositionalArguments | Self::Private | Self::Unix - | Self::Windows => None, + | Self::Windows => &[], + Self::Script(arguments) => &arguments, } } } @@ -111,8 +115,21 @@ impl<'src> Attribute<'src> { impl<'src> Display for Attribute<'src> { fn fmt(&self, f: &mut Formatter) -> fmt::Result { write!(f, "{}", self.name())?; - if let Some(argument) = self.argument() { - write!(f, "({argument})")?; + + let arguments = self.arguments(); + + for (i, argument) in arguments.iter().enumerate() { + if i == 0 { + write!(f, "(")?; + } else { + write!(f, ", ")?; + } + + write!(f, "{argument}")?; + + if i + 1 == arguments.len() { + write!(f, ")")?; + } } Ok(()) diff --git a/src/error.rs b/src/error.rs index 31a8bd4b9a..606b9e67e1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -136,14 +136,18 @@ pub(crate) enum Error<'src> { RegexCompile { source: regex::Error, }, + Script { + io_error: io::Error, + recipe: &'src str, + }, Search { search_error: SearchError, }, Shebang { - recipe: &'src str, - command: String, argument: Option, + command: String, io_error: io::Error, + recipe: &'src str, }, Signal { recipe: &'src str, @@ -413,6 +417,9 @@ impl<'src> ColorDisplay for Error<'src> { RuntimeDirIo { io_error, path } => { write!(f, "I/O error in runtime dir `{}`: {io_error}", path.display())?; } + Script { recipe, io_error} => { + write!(f, "Recipe `{recipe}` execution error: {io_error}")?; + } Search { search_error } => Display::fmt(search_error, f)?, Shebang { recipe, command, argument, io_error} => { if let Some(argument) = argument { diff --git a/src/lib.rs b/src/lib.rs index 4245a15c89..e9542f3dd9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,6 +74,7 @@ pub(crate) use { path::{self, Path, PathBuf}, process::{self, Command, ExitStatus, Stdio}, rc::Rc, + slice, str::{self, Chars}, sync::{Mutex, MutexGuard, OnceLock}, vec, diff --git a/src/parser.rs b/src/parser.rs index f337b90f39..5ec83b6fe9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -812,8 +812,17 @@ impl<'run, 'src> Parser<'run, 'src> { let body = self.parse_body()?; + let shebang = body.first().map_or(false, Line::is_shebang); + let script = attributes + .iter() + .any(|attribute| matches!(attribute, Attribute::Script(_))); + + if shebang && script { + todo!(); + } + Ok(Recipe { - shebang: body.first().map_or(false, Line::is_shebang), + shebang: shebang || script, attributes, body, dependencies, @@ -991,7 +1000,7 @@ impl<'run, 'src> Parser<'run, 'src> { Ok(Shell { arguments, command }) } - /// Parse recipe attributes + /// Item attributes, i.e., `[macos]` or `[confirm: "warning!"]` fn parse_attributes( &mut self, ) -> CompileResult<'src, Option<(Token<'src>, BTreeSet>)>> { @@ -1005,18 +1014,22 @@ impl<'run, 'src> Parser<'run, 'src> { loop { let name = self.parse_name()?; - let maybe_argument = if self.accepted(Colon)? { - let arg = self.parse_string_literal()?; - Some(arg) + let mut arguments = Vec::new(); + + if self.accepted(Colon)? { + arguments.push(self.parse_string_literal()?); } else if self.accepted(ParenL)? { - let arg = self.parse_string_literal()?; + loop { + arguments.push(self.parse_string_literal()?); + + if !self.accepted(Comma)? { + break; + } + } self.expect(ParenR)?; - Some(arg) - } else { - None - }; + } - let attribute = Attribute::new(name, maybe_argument)?; + let attribute = Attribute::new(name, arguments)?; if let Some(line) = attributes.get(&attribute) { return Err(name.error(CompileErrorKind::DuplicateAttribute { diff --git a/src/platform.rs b/src/platform.rs index 01bf74b584..ece80fe133 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -10,13 +10,13 @@ impl PlatformInterface for Platform { _shebang: Shebang, ) -> Result { // shebang scripts can be executed directly on unix - let mut cmd = Command::new(path); + let mut command = Command::new(path); if let Some(working_directory) = working_directory { - cmd.current_dir(working_directory); + command.current_dir(working_directory); } - Ok(cmd) + Ok(command) } fn set_execute_permission(path: &Path) -> io::Result<()> { diff --git a/src/recipe.rs b/src/recipe.rs index 2ba924705e..1097eb56d8 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -338,13 +338,64 @@ impl<'src, D> Recipe<'src, D> { return Ok(()); } - let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal { - message: "evaluated_lines was empty".to_owned(), - })?; + enum Foo<'a> { + Script(&'a [StringLiteral<'a>]), + Shebang(Shebang<'a>), + } - let shebang = Shebang::new(shebang_line).ok_or_else(|| Error::Internal { - message: format!("bad shebang line: {shebang_line}"), - })?; + impl<'a> Foo<'a> { + fn include_first_line(&self) -> bool { + match self { + Self::Script(_) => true, + Self::Shebang(shebang) => shebang.include_shebang_line(), + } + } + + fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { + match self { + Self::Script(_) => { + let mut filename = recipe.to_string(); + + if let Some(extension) = extension { + filename.push_str(extension); + } + + filename + } + Self::Shebang(shebang) => shebang.script_filename(recipe, extension), + } + } + + fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { + match self { + Self::Script(_) => Error::Script { io_error, recipe }, + Self::Shebang(shebang) => Error::Shebang { + recipe, + command: shebang.interpreter.to_owned(), + argument: shebang.argument.map(String::from), + io_error, + }, + } + } + } + + let foo = if let Some(Attribute::Script(interpreter)) = self + .attributes + .iter() + .find(|attribute| matches!(attribute, Attribute::Script(_))) + { + Foo::Script(interpreter) + } else { + let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal { + message: "evaluated_lines was empty".to_owned(), + })?; + + let shebang = Shebang::new(shebang_line).ok_or_else(|| Error::Internal { + message: format!("bad shebang line: {shebang_line}"), + })?; + + Foo::Shebang(shebang) + }; let mut tempdir_builder = tempfile::Builder::new(); tempdir_builder.prefix("just-"); @@ -377,7 +428,7 @@ impl<'src, D> Recipe<'src, D> { } }); - path.push(shebang.script_filename(self.name(), extension)); + path.push(foo.script_filename(self.name(), extension)); { let mut f = fs::File::create(&path).map_err(|error| Error::TempdirIo { @@ -386,7 +437,7 @@ impl<'src, D> Recipe<'src, D> { })?; let mut text = String::new(); - if shebang.include_shebang_line() { + if foo.include_first_line() { text += &evaluated_lines[0]; } else { text += "\n"; @@ -414,20 +465,34 @@ impl<'src, D> Recipe<'src, D> { })?; } - // make script executable - Platform::set_execute_permission(&path).map_err(|error| Error::TempdirIo { - recipe: self.name(), - io_error: error, - })?; + let mut command = match foo { + Foo::Script(args) => { + let mut command = Command::new(&args[0].cooked); - // create command to run script - let mut command = - Platform::make_shebang_command(&path, self.working_directory(context.search), shebang) - .map_err(|output_error| Error::Cygpath { + for arg in &args[1..] { + command.arg(&arg.cooked); + } + + command.arg(&path); + + command + } + Foo::Shebang(shebang) => { + // make script executable + Platform::set_execute_permission(&path).map_err(|error| Error::TempdirIo { recipe: self.name(), - output_error, + io_error: error, })?; + // create command to run script + Platform::make_shebang_command(&path, self.working_directory(context.search), shebang) + .map_err(|output_error| Error::Cygpath { + recipe: self.name(), + output_error, + })? + } + }; + if self.takes_positional_arguments(context.settings) { command.args(positional); } @@ -451,12 +516,7 @@ impl<'src, D> Recipe<'src, D> { } }, ), - Err(io_error) => Err(Error::Shebang { - recipe: self.name(), - command: shebang.interpreter.to_owned(), - argument: shebang.argument.map(String::from), - io_error, - }), + Err(io_error) => Err(foo.error(io_error, self.name())), } } diff --git a/tests/confirm.rs b/tests/confirm.rs index b1a2657df3..3efa44f02b 100644 --- a/tests/confirm.rs +++ b/tests/confirm.rs @@ -124,13 +124,21 @@ fn confirm_recipe_with_prompt() { fn confirm_recipe_with_prompt_too_many_args() { Test::new() .justfile( - " - [confirm(\"This is dangerous - are you sure you want to run it?\",\"this second argument is not supported\")] + r#" + [confirm("PROMPT","EXTRA")] requires_confirmation: echo confirmed - ", + "#, + ) + .stderr( + r#" + error: Attribute `confirm` got 2 arguments but takes at most 1 argument + ——▶ justfile:1:2 + │ + 1 │ [confirm("PROMPT","EXTRA")] + │ ^^^^^^^ + "#, ) - .stderr("error: Expected ')', but found ','\n ——▶ justfile:1:64\n │\n1 │ [confirm(\"This is dangerous - are you sure you want to run it?\",\"this second argument is not supported\")]\n │ ^\n") .stdout("") .status(1) .run(); diff --git a/tests/lib.rs b/tests/lib.rs index 072574bdf8..3f8fd0d074 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -90,6 +90,7 @@ mod readme; mod recursion_limit; mod regexes; mod run; +mod script; mod search; mod search_arguments; mod shadowing_parameters; diff --git a/tests/script.rs b/tests/script.rs new file mode 100644 index 0000000000..455e2824f7 --- /dev/null +++ b/tests/script.rs @@ -0,0 +1,16 @@ +use super::*; + +#[test] +fn basic() { + Test::new() + .justfile( + " + [script('sh', '-u')] + foo: + echo FOO + + ", + ) + .stdout("FOO\n") + .run(); +} From b05d2a8940cdcd4febca0ffa5ecb26382b6dd858 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 15:12:52 -0700 Subject: [PATCH 02/23] Modify --- src/attribute.rs | 12 ++++-- src/recipe.rs | 100 +++++++++++++++++++++++------------------------ 2 files changed, 58 insertions(+), 54 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index 41d58e3316..fb01606303 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -96,9 +96,14 @@ impl<'src> Attribute<'src> { fn arguments(&self) -> &[StringLiteral] { match self { - Self::Confirm(argument) | Self::Doc(argument) => argument.as_slice(), - Self::Extension(argument) | Self::Group(argument) => slice::from_ref(argument), - Self::Linux + Self::Confirm(Some(argument)) + | Self::Doc(Some(argument)) + | Self::Extension(argument) + | Self::Group(argument) => slice::from_ref(argument), + Self::Script(arguments) => arguments, + Self::Confirm(None) + | Self::Doc(None) + | Self::Linux | Self::Macos | Self::NoCd | Self::NoExitMessage @@ -107,7 +112,6 @@ impl<'src> Attribute<'src> { | Self::Private | Self::Unix | Self::Windows => &[], - Self::Script(arguments) => &arguments, } } } diff --git a/src/recipe.rs b/src/recipe.rs index 1097eb56d8..e251a1d937 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -16,6 +16,47 @@ fn error_from_signal(recipe: &str, line_number: Option, exit_status: Exit } } +enum Executor<'a> { + Script(&'a [StringLiteral<'a>]), + Shebang(Shebang<'a>), +} + +impl<'a> Executor<'a> { + fn include_first_line(&self) -> bool { + match self { + Self::Script(_) => true, + Self::Shebang(shebang) => shebang.include_shebang_line(), + } + } + + fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { + match self { + Self::Script(_) => { + let mut filename = recipe.to_string(); + + if let Some(extension) = extension { + filename.push_str(extension); + } + + filename + } + Self::Shebang(shebang) => shebang.script_filename(recipe, extension), + } + } + + fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { + match self { + Self::Script(_) => Error::Script { io_error, recipe }, + Self::Shebang(shebang) => Error::Shebang { + recipe, + command: shebang.interpreter.to_owned(), + argument: shebang.argument.map(String::from), + io_error, + }, + } + } +} + /// A recipe, e.g. `foo: bar baz` #[derive(PartialEq, Debug, Clone, Serialize)] pub(crate) struct Recipe<'src, D = Dependency<'src>> { @@ -338,53 +379,12 @@ impl<'src, D> Recipe<'src, D> { return Ok(()); } - enum Foo<'a> { - Script(&'a [StringLiteral<'a>]), - Shebang(Shebang<'a>), - } - - impl<'a> Foo<'a> { - fn include_first_line(&self) -> bool { - match self { - Self::Script(_) => true, - Self::Shebang(shebang) => shebang.include_shebang_line(), - } - } - - fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { - match self { - Self::Script(_) => { - let mut filename = recipe.to_string(); - - if let Some(extension) = extension { - filename.push_str(extension); - } - - filename - } - Self::Shebang(shebang) => shebang.script_filename(recipe, extension), - } - } - - fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { - match self { - Self::Script(_) => Error::Script { io_error, recipe }, - Self::Shebang(shebang) => Error::Shebang { - recipe, - command: shebang.interpreter.to_owned(), - argument: shebang.argument.map(String::from), - io_error, - }, - } - } - } - - let foo = if let Some(Attribute::Script(interpreter)) = self + let executor = if let Some(Attribute::Script(interpreter)) = self .attributes .iter() .find(|attribute| matches!(attribute, Attribute::Script(_))) { - Foo::Script(interpreter) + Executor::Script(interpreter) } else { let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal { message: "evaluated_lines was empty".to_owned(), @@ -394,7 +394,7 @@ impl<'src, D> Recipe<'src, D> { message: format!("bad shebang line: {shebang_line}"), })?; - Foo::Shebang(shebang) + Executor::Shebang(shebang) }; let mut tempdir_builder = tempfile::Builder::new(); @@ -428,7 +428,7 @@ impl<'src, D> Recipe<'src, D> { } }); - path.push(foo.script_filename(self.name(), extension)); + path.push(executor.script_filename(self.name(), extension)); { let mut f = fs::File::create(&path).map_err(|error| Error::TempdirIo { @@ -437,7 +437,7 @@ impl<'src, D> Recipe<'src, D> { })?; let mut text = String::new(); - if foo.include_first_line() { + if executor.include_first_line() { text += &evaluated_lines[0]; } else { text += "\n"; @@ -465,8 +465,8 @@ impl<'src, D> Recipe<'src, D> { })?; } - let mut command = match foo { - Foo::Script(args) => { + let mut command = match executor { + Executor::Script(args) => { let mut command = Command::new(&args[0].cooked); for arg in &args[1..] { @@ -477,7 +477,7 @@ impl<'src, D> Recipe<'src, D> { command } - Foo::Shebang(shebang) => { + Executor::Shebang(shebang) => { // make script executable Platform::set_execute_permission(&path).map_err(|error| Error::TempdirIo { recipe: self.name(), @@ -516,7 +516,7 @@ impl<'src, D> Recipe<'src, D> { } }, ), - Err(io_error) => Err(foo.error(io_error, self.name())), + Err(io_error) => Err(executor.error(io_error, self.name())), } } From 03d383484f8be9271b925d3b3f14028e4bbdbc4f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 15:13:55 -0700 Subject: [PATCH 03/23] Modify --- src/recipe.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/recipe.rs b/src/recipe.rs index e251a1d937..afb7bd7bae 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -435,6 +435,7 @@ impl<'src, D> Recipe<'src, D> { recipe: self.name(), io_error: error, })?; + let mut text = String::new(); if executor.include_first_line() { @@ -444,6 +445,7 @@ impl<'src, D> Recipe<'src, D> { } text += "\n"; + // add blank lines so that lines in the generated script have the same line // number as the corresponding lines in the justfile for _ in 1..(self.line_number() + 2) { From 40d76d6e42a951575628ea7f664b301de2b5bcb6 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:01:45 -0700 Subject: [PATCH 04/23] Tweak --- src/analyzer.rs | 18 +++++-- src/error.rs | 5 +- src/executor.rs | 97 +++++++++++++++++++++++++++++++++++ src/lib.rs | 24 +++++---- src/recipe.rs | 109 +++++++++------------------------------- src/unstable_feature.rs | 2 + tests/script.rs | 42 ++++++++++++++++ 7 files changed, 197 insertions(+), 100 deletions(-) create mode 100644 src/executor.rs diff --git a/src/analyzer.rs b/src/analyzer.rs index bd4319bab5..3c866ab501 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -186,6 +186,18 @@ impl<'src> Analyzer<'src> { let root = paths.get(root).unwrap(); + let unstable_features = recipes + .values() + .flat_map(|recipe| &recipe.attributes) + .flat_map(|attribute| { + if let Attribute::Script(_) = attribute { + Some(UnstableFeature::ScriptAttribute) + } else { + None + } + }) + .collect(); + Ok(Justfile { aliases, assignments: self.assignments, @@ -208,7 +220,7 @@ impl<'src> Analyzer<'src> { settings, source: root.into(), unexports, - unstable_features: BTreeSet::new(), + unstable_features, warnings, }) } @@ -242,7 +254,7 @@ impl<'src> Analyzer<'src> { let mut continued = false; for line in &recipe.body { - if !recipe.shebang && !continued { + if !recipe.is_script() && !continued { if let Some(Fragment::Text { token }) = line.fragments.first() { let text = token.lexeme(); @@ -255,7 +267,7 @@ impl<'src> Analyzer<'src> { continued = line.is_continuation(); } - if !recipe.shebang { + if !recipe.is_script() { if let Some(attribute) = recipe .attributes .iter() diff --git a/src/error.rs b/src/error.rs index 606b9e67e1..cd22311e7b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -137,6 +137,7 @@ pub(crate) enum Error<'src> { source: regex::Error, }, Script { + command: String, io_error: io::Error, recipe: &'src str, }, @@ -417,8 +418,8 @@ impl<'src> ColorDisplay for Error<'src> { RuntimeDirIo { io_error, path } => { write!(f, "I/O error in runtime dir `{}`: {io_error}", path.display())?; } - Script { recipe, io_error} => { - write!(f, "Recipe `{recipe}` execution error: {io_error}")?; + Script { command, io_error, recipe } => { + write!(f, "Recipe `{recipe}` with command `{command}` execution error: {io_error}")?; } Search { search_error } => Display::fmt(search_error, f)?, Shebang { recipe, command, argument, io_error} => { diff --git a/src/executor.rs b/src/executor.rs new file mode 100644 index 0000000000..80aab028b3 --- /dev/null +++ b/src/executor.rs @@ -0,0 +1,97 @@ +use super::*; + +pub(crate) enum Executor<'a> { + Command(&'a [StringLiteral<'a>]), + Shebang(Shebang<'a>), +} + +impl<'a> Executor<'a> { + pub(crate) fn command<'src>( + &self, + path: &Path, + recipe: &'src str, + working_directory: Option<&Path>, + ) -> RunResult<'src, Command> { + match self { + Self::Command(args) => { + let mut command = Command::new(&args[0].cooked); + + if let Some(working_directory) = working_directory { + command.current_dir(working_directory); + } + + for arg in &args[1..] { + command.arg(&arg.cooked); + } + + command.arg(path); + + Ok(command) + } + Self::Shebang(shebang) => { + // make script executable + Platform::set_execute_permission(path).map_err(|error| Error::TempdirIo { + recipe, + io_error: error, + })?; + + // create command to run script + Platform::make_shebang_command(path, working_directory, *shebang).map_err(|output_error| { + Error::Cygpath { + recipe, + output_error, + } + }) + } + } + } + + pub(crate) fn include_first_line(&self) -> bool { + match self { + Self::Command(_) => true, + Self::Shebang(shebang) => shebang.include_shebang_line(), + } + } + + pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { + match self { + Self::Command(_) => { + let mut filename = recipe.to_string(); + + if let Some(extension) = extension { + filename.push_str(extension); + } + + filename + } + Self::Shebang(shebang) => shebang.script_filename(recipe, extension), + } + } + + pub(crate) fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { + match self { + Self::Command(args) => { + let mut command = String::new(); + + for (i, arg) in args.iter().enumerate() { + if i > 0 { + command.push(' '); + } + command.push_str(&arg.cooked); + } + + Error::Script { + command, + io_error, + recipe, + } + } + Self::Shebang(shebang) => Error::Shebang { + argument: shebang.argument.map(String::from), + command: shebang.interpreter.to_owned(), + io_error, + recipe, + }, + } + } +} diff --git a/src/lib.rs b/src/lib.rs index e9542f3dd9..d4efb53273 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,17 +29,18 @@ pub(crate) use { conditional_operator::ConditionalOperator, config::Config, config_error::ConfigError, constants::constants, count::Count, delimiter::Delimiter, dependency::Dependency, dump_format::DumpFormat, enclosure::Enclosure, error::Error, evaluator::Evaluator, - execution_context::ExecutionContext, expression::Expression, fragment::Fragment, - function::Function, interrupt_guard::InterruptGuard, interrupt_handler::InterruptHandler, - item::Item, justfile::Justfile, keyed::Keyed, keyword::Keyword, lexer::Lexer, line::Line, - list::List, load_dotenv::load_dotenv, loader::Loader, module_path::ModulePath, name::Name, - namepath::Namepath, ordinal::Ordinal, output::output, output_error::OutputError, - parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, platform::Platform, - platform_interface::PlatformInterface, position::Position, positional::Positional, ran::Ran, - range_ext::RangeExt, recipe::Recipe, recipe_resolver::RecipeResolver, - recipe_signature::RecipeSignature, scope::Scope, search::Search, search_config::SearchConfig, - search_error::SearchError, set::Set, setting::Setting, settings::Settings, shebang::Shebang, - shell::Shell, show_whitespace::ShowWhitespace, source::Source, string_kind::StringKind, + execution_context::ExecutionContext, executor::Executor, expression::Expression, + fragment::Fragment, function::Function, interrupt_guard::InterruptGuard, + interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, keyed::Keyed, + keyword::Keyword, lexer::Lexer, line::Line, list::List, load_dotenv::load_dotenv, + loader::Loader, module_path::ModulePath, name::Name, namepath::Namepath, ordinal::Ordinal, + output::output, output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind, + parser::Parser, platform::Platform, platform_interface::PlatformInterface, position::Position, + positional::Positional, ran::Ran, range_ext::RangeExt, recipe::Recipe, + recipe_resolver::RecipeResolver, recipe_signature::RecipeSignature, scope::Scope, + search::Search, search_config::SearchConfig, search_error::SearchError, set::Set, + setting::Setting, settings::Settings, shebang::Shebang, shell::Shell, + show_whitespace::ShowWhitespace, source::Source, string_kind::StringKind, string_literal::StringLiteral, subcommand::Subcommand, suggestion::Suggestion, table::Table, thunk::Thunk, token::Token, token_kind::TokenKind, unresolved_dependency::UnresolvedDependency, unresolved_recipe::UnresolvedRecipe, unstable_feature::UnstableFeature, use_color::UseColor, @@ -150,6 +151,7 @@ mod enclosure; mod error; mod evaluator; mod execution_context; +mod executor; mod expression; mod fragment; mod function; diff --git a/src/recipe.rs b/src/recipe.rs index afb7bd7bae..4d55fade4d 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -16,47 +16,6 @@ fn error_from_signal(recipe: &str, line_number: Option, exit_status: Exit } } -enum Executor<'a> { - Script(&'a [StringLiteral<'a>]), - Shebang(Shebang<'a>), -} - -impl<'a> Executor<'a> { - fn include_first_line(&self) -> bool { - match self { - Self::Script(_) => true, - Self::Shebang(shebang) => shebang.include_shebang_line(), - } - } - - fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { - match self { - Self::Script(_) => { - let mut filename = recipe.to_string(); - - if let Some(extension) = extension { - filename.push_str(extension); - } - - filename - } - Self::Shebang(shebang) => shebang.script_filename(recipe, extension), - } - } - - fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { - match self { - Self::Script(_) => Error::Script { io_error, recipe }, - Self::Shebang(shebang) => Error::Shebang { - recipe, - command: shebang.interpreter.to_owned(), - argument: shebang.argument.map(String::from), - io_error, - }, - } - } -} - /// A recipe, e.g. `foo: bar baz` #[derive(PartialEq, Debug, Clone, Serialize)] pub(crate) struct Recipe<'src, D = Dependency<'src>> { @@ -147,6 +106,14 @@ impl<'src, D> Recipe<'src, D> { !self.private && !self.attributes.contains(&Attribute::Private) } + pub(crate) fn is_script(&self) -> bool { + self.shebang + || self + .attributes + .iter() + .any(|attribute| matches!(attribute, Attribute::Script(_))) + } + pub(crate) fn takes_positional_arguments(&self, settings: &Settings) -> bool { settings.positional_arguments || self.attributes.contains(&Attribute::PositionalArguments) } @@ -210,8 +177,8 @@ impl<'src, D> Recipe<'src, D> { let evaluator = Evaluator::new(context, is_dependency, scope); - if self.shebang { - self.run_shebang(context, scope, positional, config, evaluator) + if self.is_script() { + self.run_script(context, scope, positional, config, evaluator) } else { self.run_linewise(context, scope, positional, config, evaluator) } @@ -349,7 +316,7 @@ impl<'src, D> Recipe<'src, D> { } } - pub(crate) fn run_shebang<'run>( + pub(crate) fn run_script<'run>( &self, context: &ExecutionContext<'src, 'run>, scope: &Scope<'src, 'run>, @@ -379,20 +346,19 @@ impl<'src, D> Recipe<'src, D> { return Ok(()); } - let executor = if let Some(Attribute::Script(interpreter)) = self + let executor = if let Some(Attribute::Script(args)) = self .attributes .iter() .find(|attribute| matches!(attribute, Attribute::Script(_))) { - Executor::Script(interpreter) + Executor::Command(args) } else { - let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal { - message: "evaluated_lines was empty".to_owned(), - })?; + let line = evaluated_lines + .first() + .ok_or_else(|| Error::internal("evaluated_lines was empty"))?; - let shebang = Shebang::new(shebang_line).ok_or_else(|| Error::Internal { - message: format!("bad shebang line: {shebang_line}"), - })?; + let shebang = + Shebang::new(line).ok_or_else(|| Error::internal(format!("bad shebang line: {line}")))?; Executor::Shebang(shebang) }; @@ -431,11 +397,6 @@ impl<'src, D> Recipe<'src, D> { path.push(executor.script_filename(self.name(), extension)); { - let mut f = fs::File::create(&path).map_err(|error| Error::TempdirIo { - recipe: self.name(), - io_error: error, - })?; - let mut text = String::new(); if executor.include_first_line() { @@ -460,40 +421,20 @@ impl<'src, D> Recipe<'src, D> { eprintln!("{}", config.color.doc().stderr().paint(&text)); } - f.write_all(text.as_bytes()) + fs::File::create(&path) .map_err(|error| Error::TempdirIo { recipe: self.name(), io_error: error, - })?; - } - - let mut command = match executor { - Executor::Script(args) => { - let mut command = Command::new(&args[0].cooked); - - for arg in &args[1..] { - command.arg(&arg.cooked); - } - - command.arg(&path); - - command - } - Executor::Shebang(shebang) => { - // make script executable - Platform::set_execute_permission(&path).map_err(|error| Error::TempdirIo { + })? + .write_all(text.as_bytes()) + .map_err(|error| Error::TempdirIo { recipe: self.name(), io_error: error, })?; + } - // create command to run script - Platform::make_shebang_command(&path, self.working_directory(context.search), shebang) - .map_err(|output_error| Error::Cygpath { - recipe: self.name(), - output_error, - })? - } - }; + let mut command = + executor.command(&path, self.name(), self.working_directory(context.search))?; if self.takes_positional_arguments(context.settings) { command.args(positional); diff --git a/src/unstable_feature.rs b/src/unstable_feature.rs index 84fb3a2366..98eb4937c1 100644 --- a/src/unstable_feature.rs +++ b/src/unstable_feature.rs @@ -3,12 +3,14 @@ use super::*; #[derive(Copy, Clone, Debug, PartialEq, Ord, Eq, PartialOrd)] pub(crate) enum UnstableFeature { FormatSubcommand, + ScriptAttribute, } impl Display for UnstableFeature { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match self { Self::FormatSubcommand => write!(f, "The `--fmt` command is currently unstable."), + Self::ScriptAttribute => write!(f, "The `[script]` attribute is currently unstable."), } } } diff --git a/tests/script.rs b/tests/script.rs index 455e2824f7..12cd664a35 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -1,10 +1,28 @@ use super::*; +#[test] +fn unstable() { + Test::new() + .justfile( + " + [script('sh', '-u')] + foo: + echo FOO + + ", + ) + .stderr_regex(r"error: The `\[script\]` attribute is currently unstable\..*") + .status(EXIT_FAILURE) + .run(); +} + #[test] fn basic() { Test::new() .justfile( " + set unstable + [script('sh', '-u')] foo: echo FOO @@ -14,3 +32,27 @@ fn basic() { .stdout("FOO\n") .run(); } + +#[test] +fn requires_argument() { + Test::new() + .justfile( + " + set unstable + + [script] + foo: + ", + ) + .stderr( + " + error: Attribute `script` got 0 arguments but takes at least 1 argument + ——▶ justfile:3:2 + │ + 3 │ [script] + │ ^^^^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} From 4d2fa421d8bf5fe3be2e8b66617c8eeb6b8f574a Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:03:04 -0700 Subject: [PATCH 05/23] Adjust --- src/analyzer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/analyzer.rs b/src/analyzer.rs index 3c866ab501..35b192f343 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -196,6 +196,8 @@ impl<'src> Analyzer<'src> { None } }) + .next() + .into_iter() .collect(); Ok(Justfile { From dfbad087a7e04dafebcb86d4ff42613a6946d2ca Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:03:14 -0700 Subject: [PATCH 06/23] Revise --- src/analyzer.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 35b192f343..3c866ab501 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -196,8 +196,6 @@ impl<'src> Analyzer<'src> { None } }) - .next() - .into_iter() .collect(); Ok(Justfile { From f4eee7fa6fa0d3439c4f5ca9b23d4c5e3f86a416 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:12:43 -0700 Subject: [PATCH 07/23] Reform --- src/compile_error.rs | 4 ++++ src/compile_error_kind.rs | 3 +++ src/parser.rs | 4 +++- tests/script.rs | 26 ++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/compile_error.rs b/src/compile_error.rs index 1056f37e77..b2396219b8 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -238,6 +238,10 @@ impl Display for CompileError<'_> { ) } } + ShebangAndScriptAttribute { recipe } => write!( + f, + "Recipe `{recipe}` has both shebang line and `[script]` attribute" + ), ShellExpansion { err } => write!(f, "Shell expansion failed: {err}"), RequiredParameterFollowsDefaultParameter { parameter } => write!( f, diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index 7400b2925f..73c3960e26 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -98,6 +98,9 @@ pub(crate) enum CompileErrorKind<'src> { RequiredParameterFollowsDefaultParameter { parameter: &'src str, }, + ShebangAndScriptAttribute { + recipe: &'src str, + }, ShellExpansion { err: shellexpand::LookupError, }, diff --git a/src/parser.rs b/src/parser.rs index 5ec83b6fe9..10ce2cde6c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -818,7 +818,9 @@ impl<'run, 'src> Parser<'run, 'src> { .any(|attribute| matches!(attribute, Attribute::Script(_))); if shebang && script { - todo!(); + return Err(name.error(CompileErrorKind::ShebangAndScriptAttribute { + recipe: name.lexeme(), + })); } Ok(Recipe { diff --git a/tests/script.rs b/tests/script.rs index 12cd664a35..33df7a4d75 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -56,3 +56,29 @@ fn requires_argument() { .status(EXIT_FAILURE) .run(); } + +#[test] +fn not_allowed_with_shebang() { + Test::new() + .justfile( + " + set unstable + + [script('sh', '-u')] + foo: + #!/bin/sh + + ", + ) + .stderr( + " + error: Recipe `foo` has both shebang line and `[script]` attribute + ——▶ justfile:4:1 + │ + 4 │ foo: + │ ^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} From 68e63d44760403142d095f00fed417183b62bad5 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:14:31 -0700 Subject: [PATCH 08/23] Reform --- src/recipe.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/recipe.rs b/src/recipe.rs index 4d55fade4d..53e7c45f47 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -108,10 +108,6 @@ impl<'src, D> Recipe<'src, D> { pub(crate) fn is_script(&self) -> bool { self.shebang - || self - .attributes - .iter() - .any(|attribute| matches!(attribute, Attribute::Script(_))) } pub(crate) fn takes_positional_arguments(&self, settings: &Settings) -> bool { From 49dd6ec1bf0df0fe5488bba0bdf8a5e22523ac8e Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:20:57 -0700 Subject: [PATCH 09/23] Revise --- src/analyzer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 3c866ab501..c1b1a7d63d 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -189,7 +189,7 @@ impl<'src> Analyzer<'src> { let unstable_features = recipes .values() .flat_map(|recipe| &recipe.attributes) - .flat_map(|attribute| { + .filter_map(|attribute| { if let Attribute::Script(_) = attribute { Some(UnstableFeature::ScriptAttribute) } else { From 38a739da1fc540cc28fd556b2fe04c8d4f3d9594 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:56:15 -0700 Subject: [PATCH 10/23] Tweak --- src/executor.rs | 62 ++++++++++++++++++++++++------- src/recipe.rs | 2 +- src/shebang.rs | 98 +------------------------------------------------ 3 files changed, 50 insertions(+), 112 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index 80aab028b3..7cd0c4085e 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,7 +1,7 @@ use super::*; pub(crate) enum Executor<'a> { - Command(&'a [StringLiteral<'a>]), + Command(Vec<&'a str>), Shebang(Shebang<'a>), } @@ -14,14 +14,14 @@ impl<'a> Executor<'a> { ) -> RunResult<'src, Command> { match self { Self::Command(args) => { - let mut command = Command::new(&args[0].cooked); + let mut command = Command::new(&args[0]); if let Some(working_directory) = working_directory { command.current_dir(working_directory); } for arg in &args[1..] { - command.arg(&arg.cooked); + command.arg(&arg); } command.arg(path); @@ -54,18 +54,20 @@ impl<'a> Executor<'a> { } pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { - match self { - Self::Command(_) => { - let mut filename = recipe.to_string(); - - if let Some(extension) = extension { - filename.push_str(extension); - } + let extension = extension.unwrap_or_else(|| { + let interpreter = match self { + Self::Command(args) => &args[0], + Self::Shebang(shebang) => shebang.interpreter_filename(), + }; - filename + match interpreter { + "cmd" | "cmd.exe" => ".bat", + "powershell" | "powershell.exe" | "pwsh" | "pwsh.exe" => ".ps1", + _ => "", } - Self::Shebang(shebang) => shebang.script_filename(recipe, extension), - } + }); + + format!("{recipe}{extension}") } pub(crate) fn error<'src>(&self, io_error: io::Error, recipe: &'src str) -> Error<'src> { @@ -77,7 +79,7 @@ impl<'a> Executor<'a> { if i > 0 { command.push(' '); } - command.push_str(&arg.cooked); + command.push_str(&arg); } Error::Script { @@ -95,3 +97,35 @@ impl<'a> Executor<'a> { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shebang_script_filename() { + #[track_caller] + fn case(interpreter: &str, recipe: &str, extension: Option<&str>, expected: &str) { + assert_eq!( + Executor::Shebang(Shebang::new(&format!("#!{interpreter}")).unwrap()) + .script_filename(recipe, extension), + expected + ); + assert_eq!( + Executor::Command(vec![interpreter]).script_filename(recipe, extension), + expected + ); + } + + case("bar", "foo", Some(".sh"), "foo.sh"); + case("pwsh.exe", "foo", Some(".sh"), "foo.sh"); + case("cmd.exe", "foo", Some(".sh"), "foo.sh"); + case("powershell", "foo", None, "foo.ps1"); + case("pwsh", "foo", None, "foo.ps1"); + case("powershell.exe", "foo", None, "foo.ps1"); + case("pwsh.exe", "foo", None, "foo.ps1"); + case("cmd", "foo", None, "foo.bat"); + case("cmd.exe", "foo", None, "foo.bat"); + case("bar", "foo", None, "foo"); + } +} diff --git a/src/recipe.rs b/src/recipe.rs index 53e7c45f47..cc6a060a41 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -347,7 +347,7 @@ impl<'src, D> Recipe<'src, D> { .iter() .find(|attribute| matches!(attribute, Attribute::Script(_))) { - Executor::Command(args) + Executor::Command(args.iter().map(|arg| arg.cooked.as_str()).collect()) } else { let line = evaluated_lines .first() diff --git a/src/shebang.rs b/src/shebang.rs index 36b79dce45..2d2a60164e 100644 --- a/src/shebang.rs +++ b/src/shebang.rs @@ -30,7 +30,7 @@ impl<'line> Shebang<'line> { }) } - fn interpreter_filename(&self) -> &str { + pub fn interpreter_filename(&self) -> &str { self .interpreter .split(|c| matches!(c, '/' | '\\')) @@ -38,16 +38,6 @@ impl<'line> Shebang<'line> { .unwrap_or(self.interpreter) } - pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { - let extension = extension.unwrap_or_else(|| match self.interpreter_filename() { - "cmd" | "cmd.exe" => ".bat", - "powershell" | "powershell.exe" | "pwsh" | "pwsh.exe" => ".ps1", - _ => "", - }); - - format!("{recipe}{extension}") - } - pub(crate) fn include_shebang_line(&self) -> bool { !(cfg!(windows) || matches!(self.interpreter_filename(), "cmd" | "cmd.exe")) } @@ -137,70 +127,6 @@ mod tests { ); } - #[test] - fn powershell_script_filename() { - assert_eq!( - Shebang::new("#!powershell") - .unwrap() - .script_filename("foo", None), - "foo.ps1" - ); - } - - #[test] - fn pwsh_script_filename() { - assert_eq!( - Shebang::new("#!pwsh").unwrap().script_filename("foo", None), - "foo.ps1" - ); - } - - #[test] - fn powershell_exe_script_filename() { - assert_eq!( - Shebang::new("#!powershell.exe") - .unwrap() - .script_filename("foo", None), - "foo.ps1" - ); - } - - #[test] - fn pwsh_exe_script_filename() { - assert_eq!( - Shebang::new("#!pwsh.exe") - .unwrap() - .script_filename("foo", None), - "foo.ps1" - ); - } - - #[test] - fn cmd_script_filename() { - assert_eq!( - Shebang::new("#!cmd").unwrap().script_filename("foo", None), - "foo.bat" - ); - } - - #[test] - fn cmd_exe_script_filename() { - assert_eq!( - Shebang::new("#!cmd.exe") - .unwrap() - .script_filename("foo", None), - "foo.bat" - ); - } - - #[test] - fn plain_script_filename() { - assert_eq!( - Shebang::new("#!bar").unwrap().script_filename("foo", None), - "foo" - ); - } - #[test] fn dont_include_shebang_line_cmd() { assert!(!Shebang::new("#!cmd").unwrap().include_shebang_line()); @@ -222,26 +148,4 @@ mod tests { fn include_shebang_line_other_windows() { assert!(!Shebang::new("#!foo -c").unwrap().include_shebang_line()); } - - #[test] - fn filename_with_extension() { - assert_eq!( - Shebang::new("#!bar") - .unwrap() - .script_filename("foo", Some(".sh")), - "foo.sh" - ); - assert_eq!( - Shebang::new("#!pwsh.exe") - .unwrap() - .script_filename("foo", Some(".sh")), - "foo.sh" - ); - assert_eq!( - Shebang::new("#!cmd.exe") - .unwrap() - .script_filename("foo", Some(".sh")), - "foo.sh" - ); - } } From 47bec9eff169cfa5b1d37c1629f5feb1ad8e86ea Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 16:57:02 -0700 Subject: [PATCH 11/23] Adjust --- src/executor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index 7cd0c4085e..81e035ef7b 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -14,14 +14,14 @@ impl<'a> Executor<'a> { ) -> RunResult<'src, Command> { match self { Self::Command(args) => { - let mut command = Command::new(&args[0]); + let mut command = Command::new(args[0]); if let Some(working_directory) = working_directory { command.current_dir(working_directory); } for arg in &args[1..] { - command.arg(&arg); + command.arg(arg); } command.arg(path); @@ -56,7 +56,7 @@ impl<'a> Executor<'a> { pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { let extension = extension.unwrap_or_else(|| { let interpreter = match self { - Self::Command(args) => &args[0], + Self::Command(args) => args[0], Self::Shebang(shebang) => shebang.interpreter_filename(), }; @@ -79,7 +79,7 @@ impl<'a> Executor<'a> { if i > 0 { command.push(' '); } - command.push_str(&arg); + command.push_str(arg); } Error::Script { From c1f698194184d183d4e9a5b60feb7b6c642ddd47 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 17:02:43 -0700 Subject: [PATCH 12/23] Reform --- tests/fmt.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/fmt.rs b/tests/fmt.rs index 29a12ba467..013c0a27d2 100644 --- a/tests/fmt.rs +++ b/tests/fmt.rs @@ -1073,3 +1073,26 @@ fn exported_parameter() { .stdout("foo +$f:\n") .run(); } + +#[test] +fn multi_argument_attribute() { + Test::new() + .justfile( + " + set unstable + + [script('a', 'b', 'c')] + foo: + ", + ) + .arg("--dump") + .stdout( + " + set unstable := true + + [script('a', 'b', 'c')] + foo: + ", + ) + .run(); +} From 1ae66df18802e2eca2c59fdd9345eada0153818f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:16:00 -0700 Subject: [PATCH 13/23] Fix line numbers --- src/executor.rs | 54 ++++++++++-- src/line.rs | 2 + src/parser.rs | 19 ++--- src/recipe.rs | 42 ++------- tests/misc.rs | 59 ------------- tests/script.rs | 223 +++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 285 insertions(+), 114 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index 81e035ef7b..ee978c626d 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -46,13 +46,6 @@ impl<'a> Executor<'a> { } } - pub(crate) fn include_first_line(&self) -> bool { - match self { - Self::Command(_) => true, - Self::Shebang(shebang) => shebang.include_shebang_line(), - } - } - pub(crate) fn script_filename(&self, recipe: &str, extension: Option<&str>) -> String { let extension = extension.unwrap_or_else(|| { let interpreter = match self { @@ -96,6 +89,53 @@ impl<'a> Executor<'a> { }, } } + + // Script text for `recipe` given evaluated `lines` including blanks so line + // numbers in errors from generated script match justfile source lines. + pub(crate) fn script<'src, D>(&self, recipe: &Recipe<'src, D>, lines: &[String]) -> String { + let mut script = String::new(); + + match self { + Self::Shebang(shebang) => { + let mut n = 0; + + for (i, (line, evaluated)) in recipe.body.iter().zip(lines).enumerate() { + if i == 0 { + if shebang.include_shebang_line() { + script.push_str(evaluated); + script.push('\n'); + n += 1; + } + } else { + while n < line.number { + script.push('\n'); + n += 1; + } + + script.push_str(evaluated); + script.push('\n'); + n += 1; + } + } + } + Self::Command(_) => { + let mut n = 0; + + for (line, evaluated) in recipe.body.iter().zip(lines) { + while n < line.number { + script.push('\n'); + n += 1; + } + + script.push_str(&evaluated); + script.push('\n'); + n += 1; + } + } + } + + script + } } #[cfg(test)] diff --git a/src/line.rs b/src/line.rs index 6b0de6c7da..6218f744d4 100644 --- a/src/line.rs +++ b/src/line.rs @@ -5,6 +5,8 @@ use super::*; #[serde(transparent)] pub(crate) struct Line<'src> { pub(crate) fragments: Vec>, + #[serde(skip)] + pub(crate) number: usize, } impl<'src> Line<'src> { diff --git a/src/parser.rs b/src/parser.rs index 10ce2cde6c..27b342dadb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -869,13 +869,14 @@ impl<'run, 'src> Parser<'run, 'src> { if self.accepted(Indent)? { while !self.accepted(Dedent)? { - let line = if self.accepted(Eol)? { - Line { - fragments: Vec::new(), - } - } else { - let mut fragments = Vec::new(); - + let mut fragments = Vec::new(); + let number = self + .tokens + .get(self.next_token) + .map(|token| token.line) + .unwrap_or_default(); + + if !self.accepted(Eol)? { while !(self.accepted(Eol)? || self.next_is(Dedent)) { if let Some(token) = self.accept(Text)? { fragments.push(Fragment::Text { token }); @@ -888,11 +889,9 @@ impl<'run, 'src> Parser<'run, 'src> { return Err(self.unexpected_token()?); } } - - Line { fragments } }; - lines.push(line); + lines.push(Line { fragments, number }); } } diff --git a/src/recipe.rs b/src/recipe.rs index cc6a060a41..ac95199a3d 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -392,43 +392,17 @@ impl<'src, D> Recipe<'src, D> { path.push(executor.script_filename(self.name(), extension)); - { - let mut text = String::new(); - - if executor.include_first_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 - for _ in 1..(self.line_number() + 2) { - text += "\n"; - } - for line in &evaluated_lines[1..] { - text += line; - text += "\n"; - } - - if config.verbosity.grandiloquent() { - eprintln!("{}", config.color.doc().stderr().paint(&text)); - } + let script = executor.script(self, &evaluated_lines); - fs::File::create(&path) - .map_err(|error| Error::TempdirIo { - recipe: self.name(), - io_error: error, - })? - .write_all(text.as_bytes()) - .map_err(|error| Error::TempdirIo { - recipe: self.name(), - io_error: error, - })?; + if config.verbosity.grandiloquent() { + eprintln!("{}", config.color.doc().stderr().paint(&script)); } + fs::write(&path, script).map_err(|error| Error::TempdirIo { + recipe: self.name(), + io_error: error, + })?; + let mut command = executor.command(&path, self.name(), self.working_directory(context.search))?; diff --git a/tests/misc.rs b/tests/misc.rs index ad1055a9d2..bc13fe19bf 100644 --- a/tests/misc.rs +++ b/tests/misc.rs @@ -1086,65 +1086,6 @@ test! { stderr: "#!/bin/sh\necho hello\n", } -#[cfg(not(windows))] -test! { - name: shebang_line_numbers, - justfile: r#" - quiet: - #!/usr/bin/env cat - - a - - b - - - c - - - "#, - stdout: " - #!/usr/bin/env cat - - - a - - b - - - c - ", -} - -#[cfg(windows)] -test! { - name: shebang_line_numbers, - justfile: r#" - quiet: - #!/usr/bin/env cat - - a - - b - - - c - - - "#, - stdout: " - - - - - a - - b - - - c - ", -} - test! { name: complex_dependencies, justfile: r#" diff --git a/tests/script.rs b/tests/script.rs index 33df7a4d75..b525b6fe81 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -17,19 +17,59 @@ fn unstable() { } #[test] -fn basic() { +fn runs_with_command() { Test::new() .justfile( " set unstable - [script('sh', '-u')] + [script('cat')] foo: - echo FOO + FOO + ", + ) + .stdout( + " + + + + + FOO + ", + ) + .run(); +} + +#[test] +fn no_arguments() { + Test::new() + .justfile( + " + set unstable + + [script('sh')] + foo: + echo $UNSET_JUST_TEST_VARIABLE_ASDF + ", + ) + .stdout_regex(r"\n") + .run(); +} + +#[test] +fn with_arguments() { + Test::new() + .justfile( + " + set unstable + [script('sh', '-u')] + foo: + echo $UNSET_JUST_TEST_VARIABLE_ASDF ", ) - .stdout("FOO\n") + .stderr_regex(".*unbound variable.*") + .status(EXIT_FAILURE) .run(); } @@ -82,3 +122,178 @@ fn not_allowed_with_shebang() { .status(EXIT_FAILURE) .run(); } + +#[test] +fn script_line_numbers() { + Test::new() + .justfile( + " + set unstable + + [script('cat')] + foo: + FOO + + BAR + ", + ) + .stdout( + " + + + + + FOO + + BAR + ", + ) + .run(); +} + +#[test] +fn script_line_numbers_with_multi_line_recipe_signature() { + Test::new() + .justfile( + r" + set unstable + + [script('cat')] + foo bar='baz' \ + : + FOO + + BAR + + {{ \ + bar \ + }} + + BAZ + ", + ) + .stdout( + " + + + + + + FOO + + BAR + + baz + + + + BAZ + ", + ) + .run(); +} + +#[cfg(not(windows))] +#[test] +fn shebang_line_numbers() { + Test::new() + .justfile( + "foo: + #!/usr/bin/env cat + + a + + b + + + c + + +", + ) + .stdout( + "#!/usr/bin/env cat + + +a + +b + + +c +", + ) + .run(); +} + +#[cfg(not(windows))] +#[test] +fn shebang_line_numbers_with_multiline_constructs() { + Test::new() + .justfile( + r"foo b='b'\ + : + #!/usr/bin/env cat + + a + + {{ \ + b \ + }} + + + c + + +", + ) + .stdout( + "#!/usr/bin/env cat + + + +a + +b + + + + +c +", + ) + .run(); +} + +#[cfg(windows)] +#[test] +fn shebang_line_numbers() { + Test::new() + .justfile( + "foo: + #!/usr/bin/env cat + + a + + b + + + c + + +", + ) + .stdout( + " + + +a + +b + + +c +", + ) + .run(); +} From 5697b9db697b0df4d11d4b67fc45a451e440de48 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:17:06 -0700 Subject: [PATCH 14/23] Enhance --- src/executor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index ee978c626d..1345d902aa 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -92,7 +92,7 @@ impl<'a> Executor<'a> { // Script text for `recipe` given evaluated `lines` including blanks so line // numbers in errors from generated script match justfile source lines. - pub(crate) fn script<'src, D>(&self, recipe: &Recipe<'src, D>, lines: &[String]) -> String { + pub(crate) fn script(&self, recipe: &Recipe, lines: &[String]) -> String { let mut script = String::new(); match self { @@ -127,7 +127,7 @@ impl<'a> Executor<'a> { n += 1; } - script.push_str(&evaluated); + script.push_str(evaluated); script.push('\n'); n += 1; } From 635a4bbb9128bc6d54236d21ee346906c8e1af94 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:20:18 -0700 Subject: [PATCH 15/23] Adjust --- tests/script.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/script.rs b/tests/script.rs index b525b6fe81..23526e8c45 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -68,7 +68,7 @@ fn with_arguments() { echo $UNSET_JUST_TEST_VARIABLE_ASDF ", ) - .stderr_regex(".*unbound variable.*") + .stderr_regex(".*Recipe `foo` failed.*") .status(EXIT_FAILURE) .run(); } From f468e5822285706fb64dd19e973e99405262665a Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:28:21 -0700 Subject: [PATCH 16/23] Revise --- README.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9d4f9f6cd5..ebe50d7e0e 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Yay, all your tests passed! [available for most popular shells](#shell-completion-scripts). - Recipes can be written in - [arbitrary languages](#writing-recipes-in-other-languages), like Python or NodeJS. + [arbitrary languages](#shebang-recipes), like Python or NodeJS. - `just` can be invoked from any subdirectory, not just the directory that contains the `justfile`. @@ -1721,6 +1721,7 @@ Recipes may be annotated with attributes that change their behavior. | `[no-quiet]`1.23.0 | Override globally quiet recipes and always echo out the recipe. | | `[positional-arguments]`1.29.0 | Turn on [positional arguments](#positional-arguments) for this recipe. | | `[private]`1.10.0 | See [Private Recipes](#private-recipes). | +| `[script(COMMAND)]`master | Execute recipe as a script interpretered by `COMMAND`. See [script recipes](#script-recipes) for more details. | | `[unix]`1.8.0 | Enable recipe on Unixes. (Includes MacOS). | | `[windows]`1.8.0 | Enable recipe on Windows. | @@ -2443,7 +2444,7 @@ This has limitations, since recipe `c` is run with an entirely new invocation of `just`: Assignments will be recalculated, dependencies might run twice, and command line arguments will not be propagated to the child `just` process. -### Writing Recipes in Other Languages +### Shebang Recipes Recipes that start with `#!` are called shebang recipes, and are executed by saving the recipe body to a file and running it. This lets you write recipes in @@ -2514,6 +2515,22 @@ the final argument. For example, on Windows, if a recipe starts with `#! py`, the final command the OS runs will be something like `py C:\Temp\PATH_TO_SAVED_RECIPE_BODY`. + +### Script Recipes + +Recipes with a `[script(COMMAND)]` attributemaster are run as +scripts interpreted by `COMMAND`. This avoids some of the issues with shebang +recipes, such as the use of `cygpath` to get them working on windows, the need +to use `/usr/bin/env`, and inconsistences in shebang line splitting across Unix +OSs. + +The body of the recipe is evaluated, written to disk in the temporary +directory, and run by passing its path as an argument to `COMMAND`. + +The `[script(…)]` attribute is unstable, so you'll need to use `set unstable`, +set the `JUST_UNSTABLE` environment variable, or pass `--unstable` on the +command line. + ### Safer Bash Shebang Recipes If you're writing a `bash` shebang recipe, consider adding `set -euxo From 961d0b53086b56c20b3377fcb2ba470168f813e7 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:33:31 -0700 Subject: [PATCH 17/23] Tweak --- tests/script.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/script.rs b/tests/script.rs index 23526e8c45..704552c917 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -49,7 +49,7 @@ fn no_arguments() { [script('sh')] foo: - echo $UNSET_JUST_TEST_VARIABLE_ASDF + echo $UNSET_JUST_TEST_VARIABLE_ASDF || exit 1 ", ) .stdout_regex(r"\n") @@ -65,10 +65,10 @@ fn with_arguments() { [script('sh', '-u')] foo: - echo $UNSET_JUST_TEST_VARIABLE_ASDF + echo $UNSET_JUST_TEST_VARIABLE_ASDF || exit 1 ", ) - .stderr_regex(".*Recipe `foo` failed.*") + .stderr_regex(".*Recipe `foo` failed .*") .status(EXIT_FAILURE) .run(); } From 8afa71438bc6b6ef37cb1828dc82d1af7a7403ba Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:33:36 -0700 Subject: [PATCH 18/23] Revise --- tests/script.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/script.rs b/tests/script.rs index 704552c917..83fd9b4373 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -68,7 +68,7 @@ fn with_arguments() { echo $UNSET_JUST_TEST_VARIABLE_ASDF || exit 1 ", ) - .stderr_regex(".*Recipe `foo` failed .*") + .stderr_regex(".*Recipe `foo` failed.*") .status(EXIT_FAILURE) .run(); } From 89d5dd4d917e5a7dd1c1d79179b6340e346692c9 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:42:49 -0700 Subject: [PATCH 19/23] Amend --- tests/script.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/script.rs b/tests/script.rs index 83fd9b4373..0a71c4582d 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -49,10 +49,10 @@ fn no_arguments() { [script('sh')] foo: - echo $UNSET_JUST_TEST_VARIABLE_ASDF || exit 1 + echo foo ", ) - .stdout_regex(r"\n") + .stdout("foo\n") .run(); } @@ -63,13 +63,13 @@ fn with_arguments() { " set unstable - [script('sh', '-u')] + [script('sh', '-x')] foo: - echo $UNSET_JUST_TEST_VARIABLE_ASDF || exit 1 + echo foo ", ) - .stderr_regex(".*Recipe `foo` failed.*") - .status(EXIT_FAILURE) + .stdout("foo\n") + .stderr("+ echo foo\n") .run(); } From 6bcd751c56b0796199fe76ae6117d2d4162637ef Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:54:39 -0700 Subject: [PATCH 20/23] Tweak --- tests/script.rs | 1 + tests/tempdir.rs | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/script.rs b/tests/script.rs index 0a71c4582d..95757bdc06 100644 --- a/tests/script.rs +++ b/tests/script.rs @@ -287,6 +287,7 @@ fn shebang_line_numbers() { " + a b diff --git a/tests/tempdir.rs b/tests/tempdir.rs index 4f89792bfb..63b07a7de6 100644 --- a/tests/tempdir.rs +++ b/tests/tempdir.rs @@ -37,7 +37,6 @@ fn test_tempdir_is_set() { - cat just*/foo " } else { From 0c843a38b24898d4acd0f33b8fa8a8d5aec44c14 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:55:44 -0700 Subject: [PATCH 21/23] Modify --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ebe50d7e0e..0adb2b5fc3 100644 --- a/README.md +++ b/README.md @@ -1721,7 +1721,7 @@ Recipes may be annotated with attributes that change their behavior. | `[no-quiet]`1.23.0 | Override globally quiet recipes and always echo out the recipe. | | `[positional-arguments]`1.29.0 | Turn on [positional arguments](#positional-arguments) for this recipe. | | `[private]`1.10.0 | See [Private Recipes](#private-recipes). | -| `[script(COMMAND)]`master | Execute recipe as a script interpretered by `COMMAND`. See [script recipes](#script-recipes) for more details. | +| `[script(COMMAND)]`master | Execute recipe as a script interpreted by `COMMAND`. See [script recipes](#script-recipes) for more details. | | `[unix]`1.8.0 | Enable recipe on Unixes. (Includes MacOS). | | `[windows]`1.8.0 | Enable recipe on Windows. | From 6d3c5f641c2d3d96617c54942c4a7153a85987c6 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:56:05 -0700 Subject: [PATCH 22/23] Tweak --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 0adb2b5fc3..2d45067cfb 100644 --- a/README.md +++ b/README.md @@ -2515,7 +2515,6 @@ the final argument. For example, on Windows, if a recipe starts with `#! py`, the final command the OS runs will be something like `py C:\Temp\PATH_TO_SAVED_RECIPE_BODY`. - ### Script Recipes Recipes with a `[script(COMMAND)]` attributemaster are run as From 29912984171e5ddb7956a41f16d5d6457aa5cef3 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 17 Jul 2024 22:56:36 -0700 Subject: [PATCH 23/23] Reform --- README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2d45067cfb..a54a7467d2 100644 --- a/README.md +++ b/README.md @@ -2519,9 +2519,8 @@ C:\Temp\PATH_TO_SAVED_RECIPE_BODY`. Recipes with a `[script(COMMAND)]` attributemaster are run as scripts interpreted by `COMMAND`. This avoids some of the issues with shebang -recipes, such as the use of `cygpath` to get them working on windows, the need -to use `/usr/bin/env`, and inconsistences in shebang line splitting across Unix -OSs. +recipes, such as the use of `cygpath` on Windows, the need to use +`/usr/bin/env`, and inconsistences in shebang line splitting across Unix OSs. The body of the recipe is evaluated, written to disk in the temporary directory, and run by passing its path as an argument to `COMMAND`.