Precise description of directory relations when searching upward for spin.toml. Such precise, very great-grand.

Signed-off-by: itowlson <[email protected]>

Author: itowlson

crates/common/src/paths.rs

@@ -10,12 +10,16 @@ pub const DEFAULT_MANIFEST_FILE: &str = "spin.toml";
 
 /// Attempts to find a manifest. If a path is provided, that path is resolved
 /// using `resolve_manifest_file_path`; otherwise, a directory search is carried out
-/// using `search_upwards_for_manifest`. If a manifest is found, a boolean is
-/// also returned indicating if it was the default: this can be used to
-/// notify the user that a non-default manifest is being used.
-pub fn find_manifest_file_path(provided_path: Option<impl AsRef<Path>>) -> Result<(PathBuf, bool)> {
+/// using `search_upwards_for_manifest`. If we had to search, and a manifest is found,
+/// a (non-zero) usize is returned indicating how far above the current directory it
+/// was found. (A usize of 0 indicates that the manifest was provided or found
+/// in the current directory.) This can be used to notify the user that a
+/// non-default manifest is being used.
+pub fn find_manifest_file_path(
+    provided_path: Option<impl AsRef<Path>>,
+) -> Result<(PathBuf, usize)> {
     match provided_path {
-        Some(provided_path) => resolve_manifest_file_path(provided_path).map(|p| (p, true)),
+        Some(provided_path) => resolve_manifest_file_path(provided_path).map(|p| (p, 0)),
         None => search_upwards_for_manifest()
             .ok_or_else(|| anyhow!("\"{}\" not found", DEFAULT_MANIFEST_FILE)),
     }
@@ -52,25 +56,38 @@ pub fn resolve_manifest_file_path(provided_path: impl AsRef<Path>) -> Result<Pat
 /// Starting from the current directory, searches upward through
 /// the directory tree for a manifest (that is, a file with the default
 /// manifest name `spin.toml`). If found, the path to the manifest
-/// is returned, with a boolean flag indicating if the found path was
-/// the default (i.e. `spin.toml` in the current directory).
+/// is returned, with a usize indicating how far above the current directory it
+/// was found. (A usize of 0 indicates that the manifest was provided or found
+/// in the current directory.) This can be used to notify the user that a
+/// non-default manifest is being used.
 /// If no matching file is found, the function returns None.
-pub fn search_upwards_for_manifest() -> Option<(PathBuf, bool)> {
-    let mut inferred_dir = std::env::current_dir().unwrap();
-    let mut is_default = true;
+///
+/// The search is abandoned if it reaches the root directory, or the
+/// root of a Git repository, without finding a 'spin.toml'.
+pub fn search_upwards_for_manifest() -> Option<(PathBuf, usize)> {
+    let candidate = PathBuf::from(DEFAULT_MANIFEST_FILE);
+
+    if candidate.is_file() {
+        return Some((candidate, 0));
+    }
 
-    loop {
-        let candidate = inferred_dir.join(DEFAULT_MANIFEST_FILE);
+    for distance in 1..20 {
+        let inferred_dir = PathBuf::from("../".repeat(distance));
+        if !inferred_dir.is_dir() {
+            return None;
+        }
 
+        let candidate = inferred_dir.join(DEFAULT_MANIFEST_FILE);
         if candidate.is_file() {
-            return Some((candidate, is_default));
+            return Some((candidate, distance));
         }
 
-        is_default = false;
-        let parent = inferred_dir.parent()?;
-
-        inferred_dir = parent.to_owned();
+        if is_git_root(&inferred_dir) {
+            return None;
+        }
     }
+
+    None
 }
 
 /// Resolves the parent directory of a path, returning an error if the path
@@ -86,6 +103,10 @@ pub fn parent_dir(path: impl AsRef<Path>) -> Result<PathBuf> {
     Ok(parent.into())
 }
 
+fn is_git_root(dir: &Path) -> bool {
+    dir.join(".git").is_dir()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/terminal/src/lib.rs

@@ -128,6 +128,16 @@ macro_rules! ceprint {
     };
 }
 
+#[macro_export]
+macro_rules! ceprintln {
+    ($color:expr, $($arg:tt)*) => {
+        use std::io::Write;
+        let mut out = $crate::ColorText::stderr($color);
+        let _ = writeln!(out, $($arg)*);
+        drop(out); // Reset colors
+    };
+}
+
 pub mod colors {
     use termcolor::{Color, ColorSpec};
 

src/commands/build.rs

@@ -2,9 +2,11 @@ use std::{ffi::OsString, path::PathBuf};
 
 use anyhow::Result;
 use clap::Parser;
-use spin_common::ui::quoted_path;
 
-use crate::opts::{APP_MANIFEST_FILE_OPT, BUILD_UP_OPT};
+use crate::{
+    directory_rels::notify_if_nondefault_rel,
+    opts::{APP_MANIFEST_FILE_OPT, BUILD_UP_OPT},
+};
 
 use super::up::UpCommand;
 
@@ -37,15 +39,9 @@ pub struct BuildCommand {
 
 impl BuildCommand {
     pub async fn run(self) -> Result<()> {
-        let (manifest_file, is_default) =
+        let (manifest_file, distance) =
             spin_common::paths::find_manifest_file_path(self.app_source.as_ref())?;
-        if !is_default {
-            terminal::einfo!(
-                "Using 'spin.toml' from parent directory:",
-                "{}",
-                quoted_path(&manifest_file)
-            );
-        }
+        notify_if_nondefault_rel(&manifest_file, distance);
 
         spin_build::build(&manifest_file, &self.component_id).await?;
 

src/commands/doctor.rs

@@ -17,31 +17,29 @@ pub struct DoctorCommand {
         name = APP_MANIFEST_FILE_OPT,
         short = 'f',
         long = "from",
-        alias = "file",
+        alias = "file"
     )]
     pub app_source: Option<PathBuf>,
 }
 
 impl DoctorCommand {
     pub async fn run(self) -> Result<()> {
-        let (manifest_file, is_default) =
+        let (manifest_file, distance) =
             spin_common::paths::find_manifest_file_path(self.app_source.as_ref())?;
-
-        println!("{icon}The Spin Doctor is in.", icon = Emoji("📟 ", ""));
-        if is_default {
-            println!(
-                "{icon}Checking {}...",
-                manifest_file.display(),
-                icon = Emoji("🩺 ", "")
-            );
-        } else {
-            println!(
-                "{icon}Checking file from parent directory {}...",
-                manifest_file.display(),
-                icon = Emoji("🩺 ", "")
+        if distance > 0 {
+            anyhow::bail!(
+                "No spin.toml in current directory - did you mean '--from {}'?",
+                manifest_file.display()
             );
         }
 
+        println!("{icon}The Spin Doctor is in.", icon = Emoji("📟 ", ""));
+        println!(
+            "{icon}Checking {}...",
+            manifest_file.display(),
+            icon = Emoji("🩺 ", "")
+        );
+
         let mut checkup = spin_doctor::Checkup::new(manifest_file)?;
         let mut has_problems = false;
         while let Some(PatientDiagnosis { diagnosis, patient }) = checkup.next_diagnosis().await? {

src/commands/registry.rs

@@ -1,8 +1,8 @@
-use crate::opts::*;
+use crate::{directory_rels::notify_if_nondefault_rel, opts::*};
 use anyhow::{Context, Result};
 use clap::{Parser, Subcommand};
 use indicatif::{ProgressBar, ProgressStyle};
-use spin_common::{arg_parser::parse_kv, ui::quoted_path};
+use spin_common::arg_parser::parse_kv;
 use spin_oci::{client::InferPredefinedAnnotations, Client};
 use std::{io::Read, path::PathBuf, time::Duration};
 
@@ -70,15 +70,9 @@ pub struct Push {
 
 impl Push {
     pub async fn run(self) -> Result<()> {
-        let (app_file, is_default) =
+        let (app_file, distance) =
             spin_common::paths::find_manifest_file_path(self.app_source.as_ref())?;
-        if !is_default {
-            terminal::einfo!(
-                "Using 'spin.toml' from parent directory:",
-                "{}",
-                quoted_path(&app_file)
-            );
-        }
+        notify_if_nondefault_rel(&app_file, distance);
 
         if self.build {
             spin_build::build(&app_file, &[]).await?;

src/commands/up.rs

@@ -18,7 +18,7 @@ use spin_oci::OciLoader;
 use spin_trigger::cli::{LaunchMetadata, SPIN_LOCAL_APP_DIR, SPIN_LOCKED_URL, SPIN_WORKING_DIR};
 use tempfile::TempDir;
 
-use crate::opts::*;
+use crate::{directory_rels::notify_if_nondefault_rel, opts::*};
 
 use self::app_source::{AppSource, ResolvedAppSource};
 
@@ -383,14 +383,8 @@ impl UpCommand {
             AppSource::Unresolvable(msg)
         } else {
             match spin_common::paths::search_upwards_for_manifest() {
-                Some((manifest_path, is_default)) => {
-                    if !is_default {
-                        terminal::einfo!(
-                            "Using 'spin.toml' from parent directory:",
-                            "{}",
-                            quoted_path(&manifest_path)
-                        );
-                    }
+                Some((manifest_path, distance)) => {
+                    notify_if_nondefault_rel(&manifest_path, distance);
                     AppSource::File(manifest_path)
                 }
                 None => AppSource::None,

src/commands/watch.rs

@@ -8,12 +8,13 @@ use anyhow::{Context, Result};
 use clap::Parser;
 use itertools::Itertools;
 use path_absolutize::Absolutize;
-use spin_common::{paths::parent_dir, ui::quoted_path};
+use spin_common::paths::parent_dir;
 use uuid::Uuid;
 use watchexec::Watchexec;
 
-use crate::opts::{
-    APP_MANIFEST_FILE_OPT, WATCH_CLEAR_OPT, WATCH_DEBOUNCE_OPT, WATCH_SKIP_BUILD_OPT,
+use crate::{
+    directory_rels::notify_if_nondefault_rel,
+    opts::{APP_MANIFEST_FILE_OPT, WATCH_CLEAR_OPT, WATCH_DEBOUNCE_OPT, WATCH_SKIP_BUILD_OPT},
 };
 
 mod buildifier;
@@ -91,15 +92,10 @@ impl WatchCommand {
         //     has just done so.  Subsequent asset changes _do_ clear the screen.
 
         let spin_bin = std::env::current_exe()?;
-        let (manifest_file, is_default) =
+        let (manifest_file, distance) =
             spin_common::paths::find_manifest_file_path(self.app_source.as_ref())?;
-        if !is_default {
-            terminal::einfo!(
-                "Using 'spin.toml' from parent directory:",
-                "{}",
-                quoted_path(&manifest_file)
-            );
-        }
+        notify_if_nondefault_rel(&manifest_file, distance);
+
         let manifest_file = manifest_file.absolutize()?.to_path_buf(); // or watchexec misses files in subdirectories
         let manifest_dir = parent_dir(&manifest_file)?;
 

src/directory_rels.rs

@@ -0,0 +1,37 @@
+//! Human-readable descriptions for directory relationships,
+//! and helpers for standard display.
+
+use std::path::Path;
+
+fn parent_rel(distance: usize) -> String {
+    match distance {
+        0 => "".to_owned(),
+        1 => "parent".to_owned(),
+        2 => "grandparent".to_owned(),
+        _ => format!("{}grandparent", "great-".repeat(distance - 2)),
+    }
+}
+
+pub fn notify_if_nondefault_rel(manifest_file: &Path, distance: usize) {
+    if distance > 0 {
+        terminal::einfo!(
+            "No 'spin.toml' in current directory.",
+            "Using 'spin.toml' from {} directory ({})",
+            parent_rel(distance),
+            manifest_file.display(),
+        );
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn ancestry_text_is_correct() {
+        assert_eq!("parent", parent_rel(1));
+        assert_eq!("grandparent", parent_rel(2));
+        assert_eq!("great-grandparent", parent_rel(3));
+        assert_eq!("great-great-great-grandparent", parent_rel(5)); // I hope you're happy Lann
+    }
+}

src/lib.rs

@@ -1,5 +1,6 @@
 pub mod build_info;
 pub mod commands;
+mod directory_rels;
 pub(crate) mod opts;
 pub mod subprocess;