Use correct backtick and shell() expression working directory in submodules (#2285)

Author: casey

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(),
     })
   }
 

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> {

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(),
     )?;

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() {

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()
+    }
+  }
 }

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(),
       )
     })
 }

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()
   }

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(),
     ) {

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,15 +255,15 @@ 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);
       }
 
       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()) {