Repository: casey/just
Pull request #2285 (2024-07-31 01:27:36)

Fix backtick and `shell()` working directory in submodules


Patch:
diff --git a/src/analyzer.rs b/src/analyzer.rs
index 661e7092..65bd382c 100644
--- a/src/analyzer.rs
+++ b/src/analyzer.rs
@@ -35,7 +35,8 @@ impl<'src> Analyzer<'src> {
     let mut assignments = Vec::new();
 
     let mut stack = Vec::new();
-    stack.push(asts.get(root).unwrap());
+    let ast = asts.get(root).unwrap();
+    stack.push(ast);
 
     let mut warnings = Vec::new();
 
@@ -232,6 +233,7 @@ impl<'src> Analyzer<'src> {
       unexports,
       unstable_features,
       warnings,
+      working_directory: ast.working_directory.clone(),
     })
   }
 
diff --git a/src/ast.rs b/src/ast.rs
index f2e01228..f9dd10c9 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -5,10 +5,9 @@ use super::*;
 /// are performed by the `Analyzer`, which produces a `Justfile` from an `Ast`.
 #[derive(Debug, Clone)]
 pub(crate) struct Ast<'src> {
-  /// Items in the justfile
   pub(crate) items: Vec<Item<'src>>,
-  /// Non-fatal warnings encountered during parsing
   pub(crate) warnings: Vec<Warning>,
+  pub(crate) working_directory: PathBuf,
 }
 
 impl<'src> Display for Ast<'src> {
diff --git a/src/compiler.rs b/src/compiler.rs
index 322b1d20..1a555eba 100644
--- a/src/compiler.rs
+++ b/src/compiler.rs
@@ -24,7 +24,6 @@ impl Compiler {
         &current.path,
         &current.import_offsets,
         &current.namepath,
-        current.submodule_depth,
         &tokens,
         &current.working_directory,
       )?;
@@ -220,7 +219,6 @@ impl Compiler {
       &PathBuf::new(),
       &[],
       &Namepath::default(),
-      0,
       &tokens,
       &PathBuf::new(),
     )?;
diff --git a/src/evaluator.rs b/src/evaluator.rs
index 191b7120..9efe807d 100644
--- a/src/evaluator.rs
+++ b/src/evaluator.rs
@@ -22,11 +22,9 @@ impl<'src, 'run> Evaluator<'src, 'run> {
     let context = ExecutionContext {
       config,
       dotenv,
-      module_source: &module.source,
+      module,
       scope: parent,
       search,
-      settings: &module.settings,
-      unexports: &module.unexports,
     };
 
     let mut scope = context.scope.child();
@@ -236,25 +234,19 @@ impl<'src, 'run> Evaluator<'src, 'run> {
   }
 
   pub(crate) fn run_command(&self, command: &str, args: &[&str]) -> Result<String, OutputError> {
-    let mut cmd = self.context.settings.shell_command(self.context.config);
+    let mut cmd = self
+      .context
+      .module
+      .settings
+      .shell_command(self.context.config);
     cmd.arg(command);
     cmd.args(args);
-    if let Some(working_directory) = &self.context.settings.working_directory {
-      cmd.current_dir(
-        self
-          .context
-          .search
-          .working_directory
-          .join(working_directory),
-      )
-    } else {
-      cmd.current_dir(&self.context.search.working_directory)
-    };
+    cmd.current_dir(self.context.working_directory());
     cmd.export(
-      self.context.settings,
+      &self.context.module.settings,
       self.context.dotenv,
       &self.scope,
-      self.context.unexports,
+      &self.context.module.unexports,
     );
     cmd.stdin(Stdio::inherit());
     cmd.stderr(if self.context.config.verbosity.quiet() {
diff --git a/src/execution_context.rs b/src/execution_context.rs
index 2efd9e5b..bd39ecfc 100644
--- a/src/execution_context.rs
+++ b/src/execution_context.rs
@@ -4,9 +4,23 @@ use super::*;
 pub(crate) struct ExecutionContext<'src: 'run, 'run> {
   pub(crate) config: &'run Config,
   pub(crate) dotenv: &'run BTreeMap<String, String>,
-  pub(crate) module_source: &'run Path,
+  pub(crate) module: &'run Justfile<'src>,
   pub(crate) scope: &'run Scope<'src, 'run>,
   pub(crate) search: &'run Search,
-  pub(crate) settings: &'run Settings<'src>,
-  pub(crate) unexports: &'run HashSet<String>,
+}
+
+impl<'src: 'run, 'run> ExecutionContext<'src, 'run> {
+  pub(crate) fn working_directory(&self) -> PathBuf {
+    let base = if self.module.is_submodule() {
+      &self.module.working_directory
+    } else {
+      &self.search.working_directory
+    };
+
+    if let Some(setting) = &self.module.settings.working_directory {
+      base.join(setting)
+    } else {
+      base.into()
+    }
+  }
 }
diff --git a/src/function.rs b/src/function.rs
index 50bee73e..013af683 100644
--- a/src/function.rs
+++ b/src/function.rs
@@ -458,7 +458,7 @@ fn module_directory(context: Context) -> FunctionResult {
     .justfile
     .parent()
     .unwrap()
-    .join(context.evaluator.context.module_source)
+    .join(&context.evaluator.context.module.source)
     .parent()
     .unwrap()
     .to_str()
@@ -469,7 +469,8 @@ fn module_directory(context: Context) -> FunctionResult {
         context
           .evaluator
           .context
-          .module_source
+          .module
+          .source
           .parent()
           .unwrap()
           .display(),
@@ -485,13 +486,13 @@ fn module_file(context: Context) -> FunctionResult {
     .justfile
     .parent()
     .unwrap()
-    .join(context.evaluator.context.module_source)
+    .join(&context.evaluator.context.module.source)
     .to_str()
     .map(str::to_owned)
     .ok_or_else(|| {
       format!(
         "Module file path is not valid unicode: {}",
-        context.evaluator.context.module_source.display(),
+        context.evaluator.context.module.source.display(),
       )
     })
 }
diff --git a/src/justfile.rs b/src/justfile.rs
index ab8ee547..b31084ef 100644
--- a/src/justfile.rs
+++ b/src/justfile.rs
@@ -3,10 +3,9 @@ use {super::*, serde::Serialize};
 #[derive(Debug)]
 struct Invocation<'src: 'run, 'run> {
   arguments: Vec<&'run str>,
-  module_source: &'run Path,
+  module: &'run Justfile<'src>,
   recipe: &'run Recipe<'src>,
   scope: &'run Scope<'src, 'run>,
-  settings: &'run Settings<'src>,
 }
 
 #[derive(Debug, PartialEq, Serialize)]
@@ -30,6 +29,8 @@ pub(crate) struct Justfile<'src> {
   #[serde(skip)]
   pub(crate) unstable_features: BTreeSet<UnstableFeature>,
   pub(crate) warnings: Vec<Warning>,
+  #[serde(skip)]
+  pub(crate) working_directory: PathBuf,
 }
 
 impl<'src> Justfile<'src> {
@@ -204,11 +205,9 @@ impl<'src> Justfile<'src> {
       let context = ExecutionContext {
         config,
         dotenv: &dotenv,
-        module_source: invocation.module_source,
+        module: invocation.module,
         scope: invocation.scope,
         search,
-        settings: invocation.settings,
-        unexports: &self.unexports,
       };
 
       Self::run_recipe(
@@ -267,10 +266,9 @@ impl<'src> Justfile<'src> {
     if position + 1 == path.len() {
       let recipe = self.get_recipe(&path[position]).unwrap();
       Ok(Invocation {
-        recipe,
-        module_source: &self.source,
         arguments: arguments.into(),
-        settings: &self.settings,
+        module: self,
+        recipe,
         scope: parent,
       })
     } else {
@@ -306,6 +304,10 @@ impl<'src> Justfile<'src> {
     }
   }
 
+  pub(crate) fn is_submodule(&self) -> bool {
+    self.name.is_some()
+  }
+
   pub(crate) fn name(&self) -> &'src str {
     self.name.map(|name| name.lexeme()).unwrap_or_default()
   }
diff --git a/src/parser.rs b/src/parser.rs
index a7b6243e..adc95048 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -31,7 +31,6 @@ pub(crate) struct Parser<'run, 'src> {
   module_namepath: &'run Namepath<'src>,
   next_token: usize,
   recursion_depth: usize,
-  submodule_depth: u32,
   tokens: &'run [Token<'src>],
   working_directory: &'run Path,
 }
@@ -43,7 +42,6 @@ impl<'run, 'src> Parser<'run, 'src> {
     file_path: &'run Path,
     import_offsets: &[usize],
     module_namepath: &'run Namepath<'src>,
-    submodule_depth: u32,
     tokens: &'run [Token<'src>],
     working_directory: &'run Path,
   ) -> CompileResult<'src, Ast<'src>> {
@@ -55,7 +53,6 @@ impl<'run, 'src> Parser<'run, 'src> {
       module_namepath,
       next_token: 0,
       recursion_depth: 0,
-      submodule_depth,
       tokens,
       working_directory,
     }
@@ -446,8 +443,9 @@ impl<'run, 'src> Parser<'run, 'src> {
 
     if self.next_token == self.tokens.len() {
       Ok(Ast {
-        warnings: Vec::new(),
         items,
+        warnings: Vec::new(),
+        working_directory: self.working_directory.into(),
       })
     } else {
       Err(self.internal_error(format!(
@@ -838,8 +836,6 @@ impl<'run, 'src> Parser<'run, 'src> {
       priors,
       private: name.lexeme().starts_with('_'),
       quiet,
-      submodule_depth: self.submodule_depth,
-      working_directory: self.working_directory.into(),
     })
   }
 
@@ -1089,7 +1085,6 @@ mod tests {
       &PathBuf::new(),
       &[],
       &Namepath::default(),
-      0,
       &tokens,
       &PathBuf::new(),
     )
@@ -1136,7 +1131,6 @@ mod tests {
       &PathBuf::new(),
       &[],
       &Namepath::default(),
-      0,
       &tokens,
       &PathBuf::new(),
     ) {
diff --git a/src/recipe.rs b/src/recipe.rs
index 0d40307e..78b3ef27 100644
--- a/src/recipe.rs
+++ b/src/recipe.rs
@@ -36,10 +36,6 @@ pub(crate) struct Recipe<'src, D = Dependency<'src>> {
   pub(crate) private: bool,
   pub(crate) quiet: bool,
   pub(crate) shebang: bool,
-  #[serde(skip)]
-  pub(crate) submodule_depth: u32,
-  #[serde(skip)]
-  pub(crate) working_directory: PathBuf,
 }
 
 impl<'src, D> Recipe<'src, D> {
@@ -137,20 +133,10 @@ impl<'src, D> Recipe<'src, D> {
   }
 
   fn working_directory<'a>(&'a self, context: &'a ExecutionContext) -> Option<PathBuf> {
-    if !self.change_directory() {
-      return None;
-    }
-
-    let base = if self.submodule_depth > 0 {
-      &self.working_directory
+    if self.change_directory() {
+      Some(context.working_directory())
     } else {
-      &context.search.working_directory
-    };
-
-    if let Some(setting) = &context.settings.working_directory {
-      Some(base.join(setting))
-    } else {
-      Some(base.into())
+      None
     }
   }
 
@@ -205,8 +191,8 @@ impl<'src, D> Recipe<'src, D> {
       let quiet_line = lines.peek().map_or(false, |line| line.is_quiet());
       let infallible_line = lines.peek().map_or(false, |line| line.is_infallible());
 
-      let comment_line =
-        context.settings.ignore_comments && lines.peek().map_or(false, |line| line.is_comment());
+      let comment_line = context.module.settings.ignore_comments
+        && lines.peek().map_or(false, |line| line.is_comment());
 
       loop {
         if lines.peek().is_none() {
@@ -242,7 +228,7 @@ impl<'src, D> Recipe<'src, D> {
       if config.dry_run
         || config.verbosity.loquacious()
         || !((quiet_line ^ self.quiet)
-          || (context.settings.quiet && !self.no_quiet())
+          || (context.module.settings.quiet && !self.no_quiet())
           || config.verbosity.quiet())
       {
         let color = config
@@ -269,7 +255,7 @@ impl<'src, D> Recipe<'src, D> {
         continue;
       }
 
-      let mut cmd = context.settings.shell_command(config);
+      let mut cmd = context.module.settings.shell_command(config);
 
       if let Some(working_directory) = self.working_directory(context) {
         cmd.current_dir(working_directory);
@@ -277,7 +263,7 @@ impl<'src, D> Recipe<'src, D> {
 
       cmd.arg(command);
 
-      if self.takes_positional_arguments(context.settings) {
+      if self.takes_positional_arguments(&context.module.settings) {
         cmd.arg(self.name.lexeme());
         cmd.args(positional);
       }
@@ -287,7 +273,12 @@ impl<'src, D> Recipe<'src, D> {
         cmd.stdout(Stdio::null());
       }
 
-      cmd.export(context.settings, context.dotenv, scope, context.unexports);
+      cmd.export(
+        &context.module.settings,
+        context.dotenv,
+        scope,
+        &context.module.unexports,
+      );
 
       match InterruptHandler::guard(|| cmd.status()) {
         Ok(exit_status) => {
@@ -356,7 +347,7 @@ impl<'src, D> Recipe<'src, D> {
       Executor::Command(
         interpreter
           .as_ref()
-          .or(context.settings.script_interpreter.as_ref())
+          .or(context.module.settings.script_interpreter.as_ref())
           .unwrap_or_else(|| Interpreter::default_script_interpreter()),
       )
     } else {
@@ -372,7 +363,7 @@ impl<'src, D> Recipe<'src, D> {
 
     let mut tempdir_builder = tempfile::Builder::new();
     tempdir_builder.prefix("just-");
-    let tempdir = match &context.settings.tempdir {
+    let tempdir = match &context.module.settings.tempdir {
       Some(tempdir) => tempdir_builder.tempdir_in(context.search.working_directory.join(tempdir)),
       None => {
         if let Some(runtime_dir) = dirs::runtime_dir() {
@@ -420,11 +411,16 @@ impl<'src, D> Recipe<'src, D> {
       self.working_directory(context).as_deref(),
     )?;
 
-    if self.takes_positional_arguments(context.settings) {
+    if self.takes_positional_arguments(&context.module.settings) {
       command.args(positional);
     }
 
-    command.export(context.settings, context.dotenv, scope, context.unexports);
+    command.export(
+      &context.module.settings,
+      context.dotenv,
+      scope,
+      &context.module.unexports,
+    );
 
     // run it!
     match InterruptHandler::guard(|| command.status()) {
diff --git a/src/source.rs b/src/source.rs
index f81ae0ba..5b125ecc 100644
--- a/src/source.rs
+++ b/src/source.rs
@@ -7,7 +7,6 @@ pub(crate) struct Source<'src> {
   pub(crate) import_offsets: Vec<usize>,
   pub(crate) namepath: Namepath<'src>,
   pub(crate) path: PathBuf,
-  pub(crate) submodule_depth: u32,
   pub(crate) working_directory: PathBuf,
 }
 
@@ -19,7 +18,6 @@ impl<'src> Source<'src> {
       import_offsets: Vec::new(),
       namepath: Namepath::default(),
       path: path.into(),
-      submodule_depth: 0,
       working_directory: path.parent().unwrap().into(),
     }
   }
@@ -41,7 +39,6 @@ impl<'src> Source<'src> {
         .collect(),
       namepath: self.namepath.clone(),
       path,
-      submodule_depth: self.submodule_depth,
       working_directory: self.working_directory.clone(),
     }
   }
@@ -58,7 +55,6 @@ impl<'src> Source<'src> {
       import_offsets: Vec::new(),
       namepath: self.namepath.join(name),
       path: path.clone(),
-      submodule_depth: self.submodule_depth + 1,
       working_directory: path.parent().unwrap().into(),
     }
   }
diff --git a/src/unresolved_recipe.rs b/src/unresolved_recipe.rs
index af076f20..6cbc95da 100644
--- a/src/unresolved_recipe.rs
+++ b/src/unresolved_recipe.rs
@@ -59,8 +59,6 @@ impl<'src> UnresolvedRecipe<'src> {
       private: self.private,
       quiet: self.quiet,
       shebang: self.shebang,
-      submodule_depth: self.submodule_depth,
-      working_directory: self.working_directory,
     })
   }
 }


Test patch:
diff --git a/src/testing.rs b/src/testing.rs
index a43262f1..a10c3988 100644
--- a/src/testing.rs
+++ b/src/testing.rs
@@ -64,7 +64,6 @@ pub(crate) fn analysis_error(
     &PathBuf::new(),
     &[],
     &Namepath::default(),
-    0,
     &tokens,
     &PathBuf::new(),
   )
diff --git a/tests/working_directory.rs b/tests/working_directory.rs
index eb6618c6..3396b73e 100644
--- a/tests/working_directory.rs
+++ b/tests/working_directory.rs
@@ -271,3 +271,63 @@ fn working_dir_applies_to_backticks() {
     .stdout("FILE\n")
     .run();
 }
+
+#[test]
+fn working_dir_applies_to_shell_function() {
+  Test::new()
+    .justfile(
+      "
+        set working-directory := 'foo'
+
+        file := shell('cat file.txt')
+
+        @foo:
+          echo {{ file }}
+      ",
+    )
+    .write("foo/file.txt", "FILE")
+    .stdout("FILE\n")
+    .run();
+}
+
+#[test]
+fn working_dir_applies_to_backticks_in_submodules() {
+  Test::new()
+    .justfile("mod foo")
+    .write(
+      "foo/mod.just",
+      "
+set working-directory := 'bar'
+
+file := `cat file.txt`
+
+@foo:
+  echo {{ file }}
+",
+    )
+    .arg("foo")
+    .write("foo/bar/file.txt", "FILE")
+    .stdout("FILE\n")
+    .run();
+}
+
+#[test]
+fn working_dir_applies_to_shell_function_in_submodules() {
+  Test::new()
+    .justfile("mod foo")
+    .write(
+      "foo/mod.just",
+      "
+set working-directory := 'bar'
+
+file := shell('cat file.txt')
+
+@foo:
+  echo {{ file }}
+",
+    )
+    .arg("foo")
+    .write("foo/bar/file.txt", "FILE")
+    .stdout("FILE\n")
+    .run();
+}