Repository: Canop/bacon (language: rust)

Issue:
bacon.toml in a non rust subfolder of a rust project is ignored
As it was initially designed for rust projects, bacon, when deep inside a rust project, looks for the enclosing package (i.e. directory with a `Cargo.toml` file) and considers it the root of the project. In such case, the provided path is considered as a directory to *watch* only.

This logic doesn't work when bacon is used with non rust projects, and breaks when a rust projects contains another non rust project.
This is revealed by PR #272.

We need to be able to specify at command line

- the path to the project (the dir containing the deepest bacon.toml file to use), by default the current dir. This path is also the execution directory (work dir for the tool executed)
- the path to watch when we doesn't want to watch all the sources

What I propose is to change bacon arguments:

1) `-p`/`--project` becomes exclusively the path to the project. Bacon looks for bacon.toml file in this dir and up. If this dir doesn't contain a `Cargo.toml` file, bacon doesn't look for one (and rust specific features won't be available).
2) `--watch` can be used to override normal project sources (as computed from the `Cargo.toml` and `bacon.toml` files)

When bacon is called with a path eg `bacon some/path`, the path is always considered as if it was given with `--project` and never used to override watch paths. This is a breaking change but it will be clearer.



Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2890c6a..9f11b9e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -125,7 +125,7 @@ checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26"
 
 [[package]]
 name = "bacon"
-version = "3.5.0-dev"
+version = "3.5.0-new-args"
 dependencies = [
  "anyhow",
  "cargo_metadata",
diff --git a/Cargo.toml b/Cargo.toml
index 03d63f4..0a0c094 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bacon"
-version = "3.5.0-dev"
+version = "3.5.0-new-args"
 authors = ["dystroy <denys.seguret@gmail.com>"]
 repository = "https://github.com/Canop/bacon"
 description = "background rust compiler"
diff --git a/src/conf/args.rs b/src/conf/args.rs
index 2c443da..2681a6d 100644
--- a/src/conf/args.rs
+++ b/src/conf/args.rs
@@ -113,9 +113,14 @@ pub struct Args {
     #[clap(short = 'E', long)]
     pub no_export_locations: bool,
 
-    /// Path to watch (must be a rust directory or inside)
-    #[clap(short = 'p', long, value_name = "path")]
-    pub path: Option<String>,
+    /// Path to watch (overriding what's normally computed from the project's
+    /// type, bacon.toml file, etc.)
+    #[clap(long, value_name = "watch")]
+    pub watch: Option<String>,
+
+    /// Projet to run jobs on, and use as working directory
+    #[clap(long, value_name = "project")]
+    pub project: Option<String>,
 
     #[clap()]
     /// What to do: either a job, or a path, or both
@@ -137,16 +142,16 @@ impl Args {
             args.next(),
             args.next(),
             self.job.is_none(),
-            self.path.is_none(),
+            self.project.is_none(),
         ) {
             (Some(a), b, true, true) => {
                 if a.contains('.') || a.contains('/') {
                     // a is a path, it can't be job
-                    self.path = Some(a);
+                    self.project = Some(a);
                     self.job = b.map(|b| b.as_str().into());
                 } else {
                     self.job = Some(a.as_str().into());
-                    self.path = b;
+                    self.project = b;
                 }
             }
             (Some(_), Some(_), _, _) => {
@@ -156,7 +161,7 @@ impl Args {
                 self.job = Some(a.as_str().into());
             }
             (Some(a), None, false, true) => {
-                self.path = Some(a);
+                self.project = Some(a);
             }
             (Some(a), None, false, false) => {
                 bail!("Unexpected argument {:?}", a);
diff --git a/src/context.rs b/src/context.rs
index 3b3701e..c44df02 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -8,7 +8,6 @@ use {
     std::{
         env,
         fmt,
-        fs,
         path::{
             Path,
             PathBuf,
@@ -22,8 +21,8 @@ static DEFAULT_WATCHES: &[&str] = &["src", "tests", "benches", "examples", "buil
 #[derive(Debug)]
 pub struct Context {
     pub name: String,
-    pub intended_dir: PathBuf,
-    /// The current package
+    pub nature: ContextNature,
+    /// The current package/project
     pub package_directory: PathBuf,
     /// The root of the workspace, only defined when it makes sense
     /// and it's different from the package directory.
@@ -32,9 +31,9 @@ pub struct Context {
     /// it could be obtained from other kind of sources.
     pub workspace_root: Option<PathBuf>,
     cargo_mission_location: Option<CargoContext>,
-    /// When intended, the path given at launch, isn't a package, it means
-    /// we don't want to watch the whole workspace but only the given path
-    pub intended_is_package: bool,
+    /// An optional path to watch, given at launch and overriding the settings
+    /// of the Cargo.toml file, bacon.toml file, etc.
+    pub path_to_watch: Option<PathBuf>,
 }
 
 /// Specific data for a cargo related mission
@@ -56,95 +55,72 @@ impl fmt::Debug for CargoContext {
 
 impl Context {
     pub fn new(args: &Args) -> Result<Self> {
-        let intended_dir = args
-            .path
+        let package_directory = args
+            .project
             .as_ref()
             .map_or_else(|| env::current_dir().unwrap(), PathBuf::from);
-        let metadata = if args.offline {
-            MetadataCommand::new()
-                .current_dir(&intended_dir)
-                .no_deps()
-                .other_options(["--frozen".to_string(), "--offline".to_string()])
-                .exec()
+
+        if !package_directory.exists() || !package_directory.is_dir() {
+            bail!("The project path must be a directory");
+        }
+
+        let name = package_directory
+            .file_name()
+            .unwrap_or(package_directory.as_os_str())
+            .to_string_lossy()
+            .to_string();
+
+        let path_to_watch = args.watch.as_ref().map(PathBuf::from);
+
+        // A cargo project is one directly containing a Cargo.toml file.
+        // When the project is a Cargo project, some additional rules apply and
+        // the Cargo.toml file(s) is/are used to determine the paths to watch.
+        let mut cargo_toml_file = package_directory.join("Cargo.toml");
+        let nature = if cargo_toml_file.exists() && cargo_toml_file.is_file() {
+            ContextNature::Cargo
         } else {
-            MetadataCommand::new().current_dir(&intended_dir).exec()
+            ContextNature::Other
         };
-        let need_cargo = false;
-        let metadata = match metadata {
-            Ok(m) => Some(m),
-            Err(cargo_metadata::Error::CargoMetadata { stderr })
-                if cargo_manifest_not_found(&stderr) =>
-            {
-                if need_cargo {
-                    bail!(
-                        "Cargo.toml file not found.\n\
-                        bacon must be launched \n\
-                        * in a rust project directory\n\
-                        * or with a rust project directory given in argument\n\
-                        (a rust project directory contains a Cargo.toml file or has such parent)\n\
-                        "
-                    );
-                } else {
-                    None
-                }
-            }
-            Err(other) => bail!(other),
-        };
-        let package_directory;
-        let workspace_root;
-        let cargo_mission_location;
-        let intended_is_package;
-        if let Some(metadata) = metadata {
-            // Cargo/Rust project
-            let cargo_toml_file;
+
+        let mut workspace_root = None;
+        let mut cargo_mission_location = None;
+        if nature == ContextNature::Cargo {
+            let metadata = if args.offline {
+                MetadataCommand::new()
+                    .current_dir(&package_directory)
+                    .no_deps()
+                    .other_options(["--frozen".to_string(), "--offline".to_string()])
+                    .exec()
+            } else {
+                MetadataCommand::new()
+                    .current_dir(&package_directory)
+                    .exec()
+            };
+            let metadata = metadata?;
             if let Some(resolved_root) = metadata.resolve.and_then(|resolve| resolve.root) {
-                // resolved to a single package
                 cargo_toml_file = metadata
                     .packages
                     .iter()
                     .find(|p| p.id == resolved_root)
                     .map(|p| p.manifest_path.as_std_path().to_path_buf())
                     .expect("resolved manifest was not in package list");
-                package_directory = cargo_toml_file
-                    .parent()
-                    .expect("file has no parent")
-                    .to_path_buf();
-                workspace_root = if metadata.workspace_root.as_std_path() == package_directory {
-                    None
-                } else {
-                    Some(metadata.workspace_root.as_std_path().to_path_buf())
-                };
-            } else {
-                // resolved to a virtual manifest (of a workspace)
-                package_directory = metadata.workspace_root.as_std_path().to_path_buf();
-                cargo_toml_file = package_directory.join("Cargo.toml");
-                workspace_root = None;
+                if metadata.workspace_root.as_std_path() != package_directory {
+                    workspace_root = Some(metadata.workspace_root.as_std_path().to_path_buf());
+                }
             }
-            intended_is_package =
-                fs::canonicalize(&intended_dir)? == fs::canonicalize(&package_directory)?;
             cargo_mission_location = Some(CargoContext {
                 cargo_toml_file,
                 packages: metadata.packages,
             });
-        } else {
-            // Non cargo project
-            workspace_root = None;
-            cargo_mission_location = None;
-            package_directory = intended_dir.clone();
-            intended_is_package = true;
-        };
-        let name = package_directory
-            .file_name()
-            .unwrap_or(package_directory.as_os_str())
-            .to_string_lossy()
-            .to_string();
+        }
+
         Ok(Self {
             name,
-            intended_dir,
+            nature,
             package_directory,
             workspace_root,
-            intended_is_package,
             cargo_mission_location,
+            path_to_watch,
         })
     }
     pub fn mission<'s>(
@@ -154,36 +130,30 @@ impl Context {
         settings: &'s Settings,
     ) -> Result<Mission<'s>> {
         let location_name = self.name.clone();
-        // We don't need to make the difference between a file and a dir, this can
-        // be determined by the watcher
-
-        // "watches", at this point, aren't full path, they still must be joined
-        // with the right path which may depend on the
-        let mut watches: Vec<&str> = job
-            .watch
-            .as_ref()
-            .unwrap_or(&settings.watch)
-            .iter()
-            .map(|s| s.as_str())
-            .collect();
-        let add_default = job.default_watch.unwrap_or(settings.default_watch);
-        if add_default {
-            for watch in DEFAULT_WATCHES {
-                if !watches.contains(watch) {
-                    watches.push(watch);
+        let mut paths_to_watch: Vec<PathBuf> = Vec::new();
+        if let Some(path_to_watch) = &self.path_to_watch {
+            paths_to_watch.push(path_to_watch.clone());
+        } else {
+            // Automatically watch all kinds of source files.
+            // "watches", at this point, aren't full path, they still must be joined
+            // with the right path which may depend on the
+            let mut watches: Vec<&str> = job
+                .watch
+                .as_ref()
+                .unwrap_or(&settings.watch)
+                .iter()
+                .map(|s| s.as_str())
+                .collect();
+            let add_default = job.default_watch.unwrap_or(settings.default_watch);
+            if add_default {
+                for watch in DEFAULT_WATCHES {
+                    if !watches.contains(watch) {
+                        watches.push(watch);
+                    }
                 }
             }
-        }
-        debug!("watches: {watches:?}");
-
-        let mut paths_to_watch: Vec<PathBuf> = Vec::new();
-
-        // when bacon is given a path at launch, and when this path isn't a
-        // path to a package, the goal is to specify that this path should
-        // be watched, wich allows not watching everything in the workspace
-        if self.intended_is_package {
-            // automatically watch all kinds of source files
-            add_to_paths_to_watch(&watches, &self.intended_dir, &mut paths_to_watch);
+            debug!("watches: {watches:?}");
+            add_to_paths_to_watch(&watches, &self.package_directory, &mut paths_to_watch);
             if let Some(workspace_root) = &self.workspace_root {
                 add_to_paths_to_watch(&watches, workspace_root, &mut paths_to_watch);
             }
@@ -208,9 +178,6 @@ impl Context {
                     }
                 }
             }
-        } else {
-            // we only watch the given "intended" path
-            paths_to_watch.push(self.intended_dir.clone());
         }
 
         let execution_directory = self.package_directory.clone();
@@ -249,10 +216,6 @@ impl Context {
     }
 }
 
-fn cargo_manifest_not_found(err: &str) -> bool {
-    err.starts_with("error: could not find `Cargo.toml`")
-}
-
 fn add_to_paths_to_watch(
     watches: &[&str],
     base_path: &Path,
diff --git a/src/context_nature.rs b/src/context_nature.rs
new file mode 100644
index 0000000..f89c8a5
--- /dev/null
+++ b/src/context_nature.rs
@@ -0,0 +1,8 @@
+
+/// The kind of projec/context, as it impacts computing features,
+/// files to watch, etc.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ContextNature {
+    Cargo,
+    Other,
+}
diff --git a/src/lib.rs b/src/lib.rs
index 94b6bc7..f53f285 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -5,6 +5,7 @@ pub mod burp;
 mod cli;
 mod conf;
 mod context;
+mod context_nature;
 mod drawing;
 mod exec;
 mod export;
@@ -27,6 +28,7 @@ pub use {
     cli::*,
     conf::*,
     context::*,
+    context_nature::*,
     drawing::*,
     exec::*,
     export::*,


Test patch: