From 80bbb4f8c5e8a3f0222b4b4e02263e88ab5f3964 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Mar 2022 03:11:10 -0700 Subject: [PATCH 1/5] Draft implementation of run recursive --- src/config.rs | 2 +- src/search.rs | 25 +++++++------- src/subcommand.rs | 86 +++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/src/config.rs b/src/config.rs index d116355565..5946eb626f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -606,7 +606,7 @@ impl Config { } pub(crate) fn run<'src>(self, loader: &'src Loader) -> Result<(), Error<'src>> { - self.subcommand.run(&self, loader) + self.subcommand.execute(&self, loader) } } diff --git a/src/search.rs b/src/search.rs index fc4aa946e7..eaf42f0bbc 100644 --- a/src/search.rs +++ b/src/search.rs @@ -17,17 +17,7 @@ impl Search { invocation_directory: &Path, ) -> SearchResult { match search_config { - SearchConfig::FromInvocationDirectory => { - let justfile = Self::justfile(invocation_directory)?; - - let working_directory = Self::working_directory_from_justfile(&justfile)?; - - Ok(Self { - justfile, - working_directory, - }) - } - + SearchConfig::FromInvocationDirectory => Self::find_next(invocation_directory), SearchConfig::FromSearchDirectory { search_directory } => { let search_directory = Self::clean(invocation_directory, search_directory); @@ -40,7 +30,6 @@ impl Search { working_directory, }) } - SearchConfig::WithJustfile { justfile } => { let justfile = Self::clean(invocation_directory, justfile); @@ -51,7 +40,6 @@ impl Search { working_directory, }) } - SearchConfig::WithJustfileAndWorkingDirectory { justfile, working_directory, @@ -62,6 +50,17 @@ impl Search { } } + pub(crate) fn find_next(starting_dir: &Path) -> SearchResult { + let justfile = Self::justfile(starting_dir)?; + + let working_directory = Self::working_directory_from_justfile(&justfile)?; + + Ok(Self { + justfile, + working_directory, + }) + } + pub(crate) fn init( search_config: &SearchConfig, invocation_directory: &Path, diff --git a/src/subcommand.rs b/src/subcommand.rs index 64ba5736e4..da91a42625 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -27,8 +27,8 @@ pub(crate) enum Subcommand { Init, List, Run { - overrides: BTreeMap, arguments: Vec, + overrides: BTreeMap, }, Show { name: String, @@ -38,7 +38,11 @@ pub(crate) enum Subcommand { } impl Subcommand { - pub(crate) fn run<'src>(&self, config: &Config, loader: &'src Loader) -> Result<(), Error<'src>> { + pub(crate) fn execute<'src>( + &self, + config: &Config, + loader: &'src Loader, + ) -> Result<(), Error<'src>> { use Subcommand::*; match self { @@ -48,6 +52,10 @@ impl Subcommand { } Completions { shell } => return Self::completions(shell), Init => return Self::init(config), + Run { + arguments, + overrides, + } => return Self::run(config, loader, &arguments, &overrides), _ => {} } @@ -79,19 +87,83 @@ impl Subcommand { Dump => Self::dump(config, ast, justfile)?, Format => Self::format(config, &search, src, ast)?, List => Self::list(config, justfile), - Run { - arguments, - overrides, - } => justfile.run(config, &search, overrides, arguments)?, Show { ref name } => Self::show(config, name, justfile)?, Summary => Self::summary(config, justfile), Variables => Self::variables(justfile), - Changelog | Completions { .. } | Edit | Init => unreachable!(), + Changelog | Completions { .. } | Edit | Init | Run { .. } => unreachable!(), } Ok(()) } + pub(crate) fn run<'src>( + config: &Config, + loader: &'src Loader, + arguments: &[String], + overrides: &BTreeMap, + ) -> Result<(), Error<'src>> { + if config.search_config == SearchConfig::FromInvocationDirectory { + let mut path = config.invocation_directory.clone(); + + let mut errors = Vec::new(); + + loop { + let search = match Search::find_next(&path) { + Err(err) => { + errors.push(err.into()); + break; + } + Ok(search) => search, + }; + + match Self::run_inner(config, loader, arguments, overrides, &search) { + Err(err @ Error::UnknownRecipes { .. }) => { + match search.justfile.parent().unwrap().parent() { + Some(parent) => { + errors.push(err); + path = parent.into(); + } + None => return Err(err), + } + } + result => return result, + } + } + + Err(errors.remove(0).into()) + } else { + Self::run_inner( + config, + loader, + arguments, + overrides, + &Search::find(&config.search_config, &config.invocation_directory)?, + ) + } + } + + pub(crate) fn run_inner<'src>( + config: &Config, + loader: &'src Loader, + arguments: &[String], + overrides: &BTreeMap, + search: &Search, + ) -> Result<(), Error<'src>> { + let src = loader.load(&search.justfile)?; + + let tokens = Lexer::lex(src)?; + let ast = Parser::parse(&tokens)?; + let justfile = Analyzer::analyze(ast.clone())?; + + if config.verbosity.loud() { + for warning in &justfile.warnings { + eprintln!("{}", warning.color_display(config.color.stderr())); + } + } + + justfile.run(config, &search, overrides, arguments) + } + fn changelog() { print!("{}", include_str!("../CHANGELOG.md")); } From ac886619dbc66c13579d2844d14bfad2fcf9dfd9 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Mar 2022 03:33:02 -0700 Subject: [PATCH 2/5] Factor out duplicate code --- src/subcommand.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index da91a42625..b012d251c9 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -65,17 +65,7 @@ impl Subcommand { return Self::edit(&search); } - let src = loader.load(&search.justfile)?; - - let tokens = Lexer::lex(src)?; - let ast = Parser::parse(&tokens)?; - let justfile = Analyzer::analyze(ast.clone())?; - - if config.verbosity.loud() { - for warning in &justfile.warnings { - eprintln!("{}", warning.color_display(config.color.stderr())); - } - } + let (src, ast, justfile) = Self::compile(&config, loader, &search)?; match self { Choose { overrides, chooser } => { @@ -142,13 +132,22 @@ impl Subcommand { } } - pub(crate) fn run_inner<'src>( + fn run_inner<'src>( config: &Config, loader: &'src Loader, arguments: &[String], overrides: &BTreeMap, search: &Search, ) -> Result<(), Error<'src>> { + let (_src, _ast, justfile) = Self::compile(config, loader, search)?; + justfile.run(config, &search, overrides, arguments) + } + + fn compile<'src>( + config: &Config, + loader: &'src Loader, + search: &Search, + ) -> Result<(&'src str, Ast<'src>, Justfile<'src>), Error<'src>> { let src = loader.load(&search.justfile)?; let tokens = Lexer::lex(src)?; @@ -161,7 +160,7 @@ impl Subcommand { } } - justfile.run(config, &search, overrides, arguments) + Ok((src, ast, justfile)) } fn changelog() { From e6d9b8a510362d9861740bf6d6d629b5a8587820 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Mar 2022 21:58:02 -0700 Subject: [PATCH 3/5] Use option --- README.md | 29 ++++++ src/common.rs | 2 +- src/config.rs | 4 + src/justfile.rs | 4 - src/subcommand.rs | 35 +++++-- tests/dotenv.rs | 1 - tests/fall_back_to_parent.rs | 189 +++++++++++++++++++++++++++++++++++ tests/lib.rs | 1 + tests/test.rs | 15 --- 9 files changed, 250 insertions(+), 30 deletions(-) create mode 100644 tests/fall_back_to_parent.rs diff --git a/README.md b/README.md index 1ae9d98f29..d867a1755c 100644 --- a/README.md +++ b/README.md @@ -1831,6 +1831,35 @@ default: The `--dump` command can be used with `--dump-format json` to print a JSON representation of a `justfile`. The JSON format is currently unstable, so the `--unstable` flag is required. +### Falling back to parent `justfile`s + +If a recipe is not found, `just` will look for `justfile`s in the parent +directory and up, until it reaches the root directory. + +This feature is currently unstable, and so must be enabled with the +`--unstable` flag. + +As an example, suppose the current directory contains this `justfile`: + +```make +foo: + echo foo +``` + +And the parent directory contains this `justfile`: + +```make +bar: + echo bar +``` + +```sh +$ just --unstable bar +Trying ../justfile +echo bar +bar +``` + Changelog --------- diff --git a/src/common.rs b/src/common.rs index a88154a81d..339e7e2f70 100644 --- a/src/common.rs +++ b/src/common.rs @@ -10,7 +10,7 @@ pub(crate) use std::{ iter::{self, FromIterator}, mem, ops::{Index, Range, RangeInclusive}, - path::{Path, PathBuf}, + path::{self, Path, PathBuf}, process::{self, Command, ExitStatus, Stdio}, rc::Rc, str::{self, Chars}, diff --git a/src/config.rs b/src/config.rs index 5946eb626f..a10826b2f1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -606,6 +606,10 @@ impl Config { } pub(crate) fn run<'src>(self, loader: &'src Loader) -> Result<(), Error<'src>> { + if let Err(error) = InterruptHandler::install(self.verbosity) { + warn!("Failed to set CTRL-C handler: {}", error); + } + self.subcommand.execute(&self, loader) } } diff --git a/src/justfile.rs b/src/justfile.rs index eb0923ca9b..864ef90d03 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -74,10 +74,6 @@ impl<'src> Justfile<'src> { overrides: &BTreeMap, arguments: &[String], ) -> RunResult<'src, ()> { - if let Err(error) = InterruptHandler::install(config.verbosity) { - warn!("Failed to set CTRL-C handler: {}", error); - } - let unknown_overrides = overrides .keys() .filter(|name| !self.assignments.contains_key(name.as_str())) diff --git a/src/subcommand.rs b/src/subcommand.rs index b012d251c9..e5c20b0d87 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -92,25 +92,44 @@ impl Subcommand { arguments: &[String], overrides: &BTreeMap, ) -> Result<(), Error<'src>> { - if config.search_config == SearchConfig::FromInvocationDirectory { + if config.unstable && config.search_config == SearchConfig::FromInvocationDirectory { let mut path = config.invocation_directory.clone(); - let mut errors = Vec::new(); + let mut unknown_recipes_errors = None; loop { let search = match Search::find_next(&path) { - Err(err) => { - errors.push(err.into()); - break; + Err(SearchError::NotFound) => match unknown_recipes_errors { + Some(err) => return Err(err), + None => return Err(SearchError::NotFound.into()), + }, + Err(err) => return Err(err.into()), + Ok(search) => { + if config.verbosity.loud() { + if path != config.invocation_directory { + eprintln!( + "Trying {}", + config + .invocation_directory + .strip_prefix(path) + .unwrap() + .components() + .map(|_| path::Component::ParentDir) + .collect::() + .join(search.justfile.file_name().unwrap()) + .display() + ); + } + } + search } - Ok(search) => search, }; match Self::run_inner(config, loader, arguments, overrides, &search) { Err(err @ Error::UnknownRecipes { .. }) => { match search.justfile.parent().unwrap().parent() { Some(parent) => { - errors.push(err); + unknown_recipes_errors.get_or_insert(err); path = parent.into(); } None => return Err(err), @@ -119,8 +138,6 @@ impl Subcommand { result => return result, } } - - Err(errors.remove(0).into()) } else { Self::run_inner( config, diff --git a/tests/dotenv.rs b/tests/dotenv.rs index 35084ce20a..e06853237d 100644 --- a/tests/dotenv.rs +++ b/tests/dotenv.rs @@ -71,7 +71,6 @@ fn no_warning() { ) .stdout("unset\n") .stderr("echo ${DOTENV_KEY:-unset}\n") - .suppress_dotenv_load_warning(false) .run(); } diff --git a/tests/fall_back_to_parent.rs b/tests/fall_back_to_parent.rs new file mode 100644 index 0000000000..05ce4493d3 --- /dev/null +++ b/tests/fall_back_to_parent.rs @@ -0,0 +1,189 @@ +use crate::common::*; + +#[test] +fn runs_recipe_in_parent_if_not_found_in_current() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile( + " + foo: + echo root + ", + ) + .args(&["--unstable", "foo"]) + .current_dir("bar") + .stderr( + " + Trying ../justfile + echo root + ", + ) + .stdout("root\n") + .run(); +} + +#[test] +fn print_error_from_parent_if_recipe_not_found_in_current() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile("foo:\n echo {{bar}}") + .args(&["--unstable", "foo"]) + .current_dir("bar") + .stderr( + " + Trying ../justfile + error: Variable `bar` not defined + | + 2 | echo {{bar}} + | ^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} + +#[test] +fn requires_unstable() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile( + " + foo: + echo root + ", + ) + .args(&["foo"]) + .current_dir("bar") + .status(EXIT_FAILURE) + .stderr("error: Justfile does not contain recipe `foo`.\n") + .run(); +} + +#[test] +fn doesnt_work_with_search_directory() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile( + " + foo: + echo root + ", + ) + .args(&["--unstable", "./foo"]) + .current_dir("bar") + .status(EXIT_FAILURE) + .stderr("error: Justfile does not contain recipe `foo`.\n") + .run(); +} + +#[test] +fn doesnt_work_with_justfile() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile( + " + foo: + echo root + ", + ) + .args(&["--unstable", "--justfile", "justfile", "foo"]) + .current_dir("bar") + .status(EXIT_FAILURE) + .stderr("error: Justfile does not contain recipe `foo`.\n") + .run(); +} + +#[test] +fn doesnt_work_with_justfile_and_working_directory() { + Test::new() + .tree(tree! { + bar: { + justfile: " + baz: + echo subdir + " + } + }) + .justfile( + " + foo: + echo root + ", + ) + .args(&[ + "--unstable", + "--justfile", + "justfile", + "--working-directory", + ".", + "foo", + ]) + .current_dir("bar") + .status(EXIT_FAILURE) + .stderr("error: Justfile does not contain recipe `foo`.\n") + .run(); +} + +#[test] +fn prints_correct_error_message_when_recipe_not_found() { + Test::new() + .tree(tree! { + bar: { + justfile: " + bar: + echo subdir + " + } + }) + .justfile( + " + bar: + echo root + ", + ) + .args(&["--unstable", "foo"]) + .current_dir("bar") + .status(EXIT_FAILURE) + .stderr( + " + Trying ../justfile + error: Justfile does not contain recipe `foo`. + ", + ) + .run(); +} diff --git a/tests/lib.rs b/tests/lib.rs index 873941576a..798b045517 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -19,6 +19,7 @@ mod error_messages; mod evaluate; mod examples; mod export; +mod fall_back_to_parent; mod fmt; mod functions; mod init; diff --git a/tests/test.rs b/tests/test.rs index 77fb77b910..4673ad2fdb 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -45,7 +45,6 @@ pub(crate) struct Test { pub(crate) stderr_regex: Option, pub(crate) stdin: String, pub(crate) stdout: String, - pub(crate) suppress_dotenv_load_warning: bool, pub(crate) tempdir: TempDir, pub(crate) unindent_stdout: bool, } @@ -67,7 +66,6 @@ impl Test { stderr_regex: None, stdin: String::new(), stdout: String::new(), - suppress_dotenv_load_warning: true, tempdir, unindent_stdout: true, } @@ -139,11 +137,6 @@ impl Test { self } - pub(crate) fn suppress_dotenv_load_warning(mut self, suppress_dotenv_load_warning: bool) -> Self { - self.suppress_dotenv_load_warning = suppress_dotenv_load_warning; - self - } - pub(crate) fn tree(self, mut tree: Tree) -> Self { tree.map(|_name, content| unindent(content)); tree.instantiate(self.tempdir.path()).unwrap(); @@ -183,14 +176,6 @@ impl Test { let mut child = command .args(self.args) .envs(&self.env) - .env( - "JUST_SUPPRESS_DOTENV_LOAD_WARNING", - if self.suppress_dotenv_load_warning { - "1" - } else { - "0" - }, - ) .current_dir(self.tempdir.path().join(self.current_dir)) .stdin(Stdio::piped()) .stdout(Stdio::piped()) From b01fbbc11a463d0996ddffb81e15cb24b6ab0537 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Mar 2022 22:02:49 -0700 Subject: [PATCH 4/5] Placate clippy --- src/justfile.rs | 4 ++-- src/subcommand.rs | 34 ++++++++++++++++------------------ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/justfile.rs b/src/justfile.rs index 864ef90d03..ce0ce9fd27 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -340,8 +340,8 @@ impl<'src> Justfile<'src> { } let mut invocation = vec![recipe.name().to_owned()]; - for argument in arguments.iter().copied() { - invocation.push(argument.to_owned()); + for argument in arguments { + invocation.push((*argument).to_string()); } ran.insert(invocation); diff --git a/src/subcommand.rs b/src/subcommand.rs index e5c20b0d87..eabb2330ea 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -55,7 +55,7 @@ impl Subcommand { Run { arguments, overrides, - } => return Self::run(config, loader, &arguments, &overrides), + } => return Self::run(config, loader, arguments, overrides), _ => {} } @@ -65,7 +65,7 @@ impl Subcommand { return Self::edit(&search); } - let (src, ast, justfile) = Self::compile(&config, loader, &search)?; + let (src, ast, justfile) = Self::compile(config, loader, &search)?; match self { Choose { overrides, chooser } => { @@ -105,21 +105,19 @@ impl Subcommand { }, Err(err) => return Err(err.into()), Ok(search) => { - if config.verbosity.loud() { - if path != config.invocation_directory { - eprintln!( - "Trying {}", - config - .invocation_directory - .strip_prefix(path) - .unwrap() - .components() - .map(|_| path::Component::ParentDir) - .collect::() - .join(search.justfile.file_name().unwrap()) - .display() - ); - } + if config.verbosity.loud() && path != config.invocation_directory { + eprintln!( + "Trying {}", + config + .invocation_directory + .strip_prefix(path) + .unwrap() + .components() + .map(|_| path::Component::ParentDir) + .collect::() + .join(search.justfile.file_name().unwrap()) + .display() + ); } search } @@ -157,7 +155,7 @@ impl Subcommand { search: &Search, ) -> Result<(), Error<'src>> { let (_src, _ast, justfile) = Self::compile(config, loader, search)?; - justfile.run(config, &search, overrides, arguments) + justfile.run(config, search, overrides, arguments) } fn compile<'src>( From f668d935706c1015a75dea65e4af6bb7835fa78c Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Mar 2022 22:11:02 -0700 Subject: [PATCH 5/5] Fix on windows --- tests/common.rs | 2 +- tests/fall_back_to_parent.rs | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/tests/common.rs b/tests/common.rs index 05f53c19eb..1915197c90 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -6,7 +6,7 @@ pub(crate) use std::{ fs, io::Write, iter, - path::{Path, PathBuf}, + path::{Path, PathBuf, MAIN_SEPARATOR}, process::{Command, Output, Stdio}, str, }; diff --git a/tests/fall_back_to_parent.rs b/tests/fall_back_to_parent.rs index 05ce4493d3..74351a1c69 100644 --- a/tests/fall_back_to_parent.rs +++ b/tests/fall_back_to_parent.rs @@ -19,12 +19,13 @@ fn runs_recipe_in_parent_if_not_found_in_current() { ) .args(&["--unstable", "foo"]) .current_dir("bar") - .stderr( + .stderr(format!( " - Trying ../justfile + Trying ..{}justfile echo root ", - ) + MAIN_SEPARATOR + )) .stdout("root\n") .run(); } @@ -43,15 +44,16 @@ fn print_error_from_parent_if_recipe_not_found_in_current() { .justfile("foo:\n echo {{bar}}") .args(&["--unstable", "foo"]) .current_dir("bar") - .stderr( + .stderr(format!( " - Trying ../justfile + Trying ..{}justfile error: Variable `bar` not defined | - 2 | echo {{bar}} + 2 | echo {{{{bar}}}} | ^^^ ", - ) + MAIN_SEPARATOR + )) .status(EXIT_FAILURE) .run(); } @@ -179,11 +181,12 @@ fn prints_correct_error_message_when_recipe_not_found() { .args(&["--unstable", "foo"]) .current_dir("bar") .status(EXIT_FAILURE) - .stderr( + .stderr(format!( " - Trying ../justfile + Trying ..{}justfile error: Justfile does not contain recipe `foo`. ", - ) + MAIN_SEPARATOR, + )) .run(); }