recursive submodule by default and no delete when fixing missing inde…

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "magoo"
-version = "0.1.4"
+version = "0.2.0"
 edition = "2021"
 description = "A wrapper for git submodule that simplifies the workflows"
 repository = "https://github.com/Pistonite/magoo"
@@ -17,12 +17,12 @@ exclude = [
 ]
 
 [dependencies]
-clap = { version = "4.4.8", features = ["derive"], optional = true }
-fs4 = "0.7.0"
+clap = { version = "4.5.4", features = ["derive"], optional = true }
+fs4 = "0.8.2"
 pathdiff = "0.2.1"
-termcolor = "1.4.0"
-thiserror = "1.0.50"
-which = "5.0.0"
+termcolor = "1.4.1"
+thiserror = "1.0.59"
+which = "6.0.1"
 
 [features]
 default = ["cli"]

README.md

@@ -72,6 +72,8 @@ The arguments for adding a submodule is very similar to [`git submodule add`](ht
 It's recommended to always specify the `BRANCH`. Git by default will use the `HEAD` branch, which
 is usually not what you want.
 
+The submodule will not be cloned recursively when adding. If you need, run `magoo install` after the `add` to clone the recursive submodules.
+
 ### Initialize/pull the submodules
 ```bash
 magoo install
@@ -80,6 +82,8 @@ magoo install
 You should run `magoo install` every time you pull the changes from others, in case they were updated.
 It also deletes submodules that are deleted by others (by running `status --fix`, see below).
 
+By default, submodules are installed recursively, you can use `--no-recursive` to only install the ones specified by the top-level repo.
+
 ### Show submodule status
 ```bash
 magoo status [--long] [--fix]

README.txtpp.md

@@ -98,6 +98,8 @@ MAGOO needs to know the following to add a submodule.:
 It's recommended to always specify the `BRANCH`. Git by default will use the `HEAD` branch, which
 is usually not what you want.
 
+The submodule will not be cloned recursively when adding. If you need, run `magoo install` after the `add` to clone the recursive submodules.
+
 ### Initialize/pull the submodules
 ```bash
 magoo install
@@ -108,6 +110,8 @@ MAGOO will ensure the submodules are cloned/updated to the commit stored in the
 You should run `magoo install` every time you pull the changes from others, in case they were updated.
 It also deletes submodules that are deleted by others (by running `status --fix`, see below).
 
+By default, submodules are installed recursively, you can use `--no-recursive` to only install the ones specified by the top-level repo.
+
 ### Show submodule status
 ```bash
 magoo status [--long] [--fix]

Taskfile.yml

@@ -14,3 +14,7 @@ tasks:
     - cargo fmt --all
     - txtpp README.md
 
+  test:
+    desc: Run tests
+    cmds:
+    - cargo test

src/git.rs

@@ -181,7 +181,7 @@ impl GitContext {
         if print::is_verbose() {
             if let Some(stderr) = child.stderr.take() {
                 let reader = BufReader::new(stderr);
-                for line in reader.lines().flatten() {
+                for line in reader.lines().map_while(Result::ok) {
                     println_verbose!("{line}");
                 }
             }
@@ -388,10 +388,14 @@ impl GitContext {
     }
 
     /// Runs `git submodule sync [-- <path>]`. Path should be from top level
-    pub fn submodule_sync(&self, path: Option<&str>) -> Result<(), GitError> {
+    pub fn submodule_sync(&self, path: Option<&str>, recursive: bool) -> Result<(), GitError> {
         let top_level_dir = self.top_level_dir()?.to_cmd_arg();
         let mut args = vec!["-C", &top_level_dir, "submodule", "sync"];
 
+        if recursive {
+            args.push("--recursive");
+        }
+
         if let Some(path) = path {
             args.push("--");
             args.push(path);
@@ -448,6 +452,7 @@ impl GitContext {
         path: Option<&str>,
         force: bool,
         remote: bool,
+        recursive: bool,
     ) -> Result<(), GitError> {
         let top_level_dir = self.top_level_dir()?.to_cmd_arg();
         let mut args = vec!["-C", &top_level_dir, "submodule", "update"];
@@ -460,6 +465,11 @@ impl GitContext {
             args.push("--remote");
         }
 
+        if recursive {
+            args.push("--init");
+            args.push("--recursive");
+        }
+
         if let Some(path) = path {
             args.push("--");
             args.push(path);
@@ -531,6 +541,7 @@ impl Guard {
             .read(true)
             .write(true)
             .create(true)
+            .truncate(true)
             .open(path)
             .map_err(|e| GitError::LockFailed(path.to_cmd_arg(), e))?;
         file.lock_exclusive()

src/lib.rs

@@ -30,6 +30,7 @@
 //!         quiet: false,
 //!         color: None,
 //!     },
+//!     delete: false,
 //! };
 //!
 //! // don't need this if you don't need output to stdout
@@ -57,6 +58,7 @@
 //!             quiet: false,
 //!             color: None,
 //!         },
+//!         delete: false,
 //!     }),
 //!     dir: "my/repo".to_string(),
 //! });
@@ -184,6 +186,10 @@ pub struct StatusCommand {
     #[cfg_attr(feature = "cli", clap(long, short))]
     pub fix: bool,
 
+    /// Prefers deleting the submodule instead of installing it when fixing
+    #[cfg_attr(feature = "cli", clap(long, requires("fix")))]
+    pub delete: bool,
+
     /// Print options
     #[cfg_attr(feature = "cli", clap(flatten))]
     pub options: PrintOptions,
@@ -212,7 +218,7 @@ impl StatusCommand {
         }
         if self.fix {
             for submodule in flat_status.iter_mut() {
-                submodule.fix(&context)?;
+                submodule.fix(&context, self.delete)?;
             }
             return Ok(status);
         }
@@ -274,6 +280,12 @@ pub struct InstallCommand {
     #[cfg_attr(feature = "cli", clap(long, short))]
     pub force: bool,
 
+    /// Don't install submodules recursively, only top-level
+    ///
+    /// By default, submodules are installed recursively with `git submodule update --recursive`.
+    #[cfg_attr(feature = "cli", clap(long))]
+    pub no_recursive: bool,
+
     /// Print options
     #[cfg_attr(feature = "cli", clap(flatten))]
     pub options: PrintOptions,
@@ -292,7 +304,7 @@ impl InstallCommand {
 
         let mut status = Status::read_from(&context)?;
         for submodule in status.flattened_mut() {
-            submodule.fix(&context)?;
+            submodule.fix(&context, false)?;
         }
 
         match &self.url {
@@ -310,8 +322,8 @@ impl InstallCommand {
             None => {
                 println_verbose!("Installing submodules");
                 context.submodule_init(None)?;
-                context.submodule_sync(None)?;
-                context.submodule_update(None, self.force, false)?;
+                context.submodule_sync(None, !self.no_recursive)?;
+                context.submodule_update(None, self.force, false, !self.no_recursive)?;
             }
         }
 
@@ -400,9 +412,8 @@ impl UpdateCommand {
                         println_hint!("  run `magoo status` to investigate. Some issues might be fixable with `magoo status --fix`.");
                         println_hint!("  alternatively, use the `--bypass` flag to ignore and continue anyway.");
                         return Err(GitError::NeedFix(false));
-                    } else {
-                        println_warn!("Bypassing warnings from unhealthy submodule `{name}`");
                     }
+                    println_warn!("Bypassing warnings from unhealthy submodule `{name}`");
                 }
 
                 let path = match submodule.path() {
@@ -425,14 +436,14 @@ impl UpdateCommand {
                     context.submodule_set_url(path, url)?;
                 }
 
-                context.submodule_sync(Some(path))?;
-                context.submodule_update(Some(path), self.force, true)?;
+                context.submodule_sync(Some(path), false)?;
+                context.submodule_update(Some(path), self.force, true, false)?;
             }
             None => {
                 println_verbose!("Updating submodules");
                 context.submodule_init(None)?;
-                context.submodule_sync(None)?;
-                context.submodule_update(None, self.force, true)?;
+                context.submodule_sync(None, false)?;
+                context.submodule_update(None, self.force, true, false)?;
             }
         }
 

src/submodule.rs

@@ -329,7 +329,7 @@ impl Submodule {
     /// 1. The submodule is healthy and initialized.
     /// 2. The submodule is healthy but not initialized.
     /// 3. The submodule is deleted.
-    pub fn fix(&mut self, context: &GitContext) -> Result<(), GitError> {
+    pub fn fix(&mut self, context: &GitContext, prefer_delete: bool) -> Result<(), GitError> {
         // the submodule can be in any shape or form
         // here are some notations:
         // - `G`: submodule data in .gitmodules is [`Some`]
@@ -371,11 +371,16 @@ impl Submodule {
             }
         }
 
-        self.fix_issue(self.find_issue(), context)?;
+        self.fix_issue(self.find_issue(), context, prefer_delete)?;
         Ok(())
     }
 
-    fn fix_issue(&mut self, issue: PartsIssue, context: &GitContext) -> Result<(), GitError> {
+    fn fix_issue(
+        &mut self,
+        issue: PartsIssue,
+        context: &GitContext,
+        prefer_delete: bool,
+    ) -> Result<(), GitError> {
         match issue {
             PartsIssue::None => {
                 // submodule is healthy
@@ -387,7 +392,31 @@ impl Submodule {
                 self.force_remove_module_dir(context)?;
             }
             PartsIssue::MissingIndex => {
-                // index is missing, delete it
+                // index is missing
+                // try installing it from the info in submodule
+                if !prefer_delete {
+                    if let Some(gitmodule) = &self.in_gitmodules {
+                        // url and path are required
+                        if let (Some(url), Some(path)) = (&gitmodule.url, &gitmodule.path) {
+                            println_verbose!("Fix: adding submodule from .gitmodules");
+                            let result = context.submodule_add(
+                                url.as_ref(),
+                                Some(path.as_ref()),
+                                gitmodule.branch.as_deref(),
+                                Some(gitmodule.name.as_ref()),
+                                None,
+                                false,
+                            );
+                            if let Err(e) = result {
+                                println_error!("Failed to add submodule as part of --fix: {e}");
+                                println_hint!("To delete it instead, add the `--delete` flag");
+                                return Err(e);
+                            }
+                            return Ok(());
+                        }
+                    }
+                }
+                // if we can't add from .gitmodules, delete it
                 println_verbose!("Fix: deleting submodule missing in index");
                 self.force_delete(context)?;
             }
@@ -397,6 +426,12 @@ impl Submodule {
                 println_verbose!("Fix: deleting submodule missing in .gitmodules");
                 self.force_delete(context)?;
             }
+            PartsIssue::MissingIndexAndGitModules => {
+                // submodule is not in .gitmodules
+                // delete it
+                println_verbose!("Fix: deleting submodule missing in index and .gitmodules");
+                self.force_delete(context)?;
+            }
         };
         Ok(())
     }
@@ -426,10 +461,14 @@ impl Submodule {
                 PartsIssue::Residue
             }
             (Some(_), None, Some(_), Some(_)) => PartsIssue::Residue,
-            (_, _, _, None) => {
-                // index is missing
+            (Some(_), _, _, None) => {
+                // index is missing, but is in .gitmodules
                 PartsIssue::MissingIndex
             }
+            (None, _, _, None) => {
+                // submodule is not in .gitmodules or index
+                PartsIssue::MissingIndexAndGitModules
+            }
             (None, _, _, _) => {
                 // submodule is not in .gitmodules
                 PartsIssue::MissingInGitModules
@@ -626,6 +665,7 @@ enum PartsIssue {
     Residue,
     MissingIndex,
     MissingInGitModules,
+    MissingIndexAndGitModules,
 }
 impl PartsIssue {
     pub fn describe(&self) -> &'static str {
@@ -634,6 +674,7 @@ impl PartsIssue {
             PartsIssue::Residue => "has residue from removal",
             PartsIssue::MissingIndex => "missing in index",
             PartsIssue::MissingInGitModules => "not in .gitmodules",
+            PartsIssue::MissingIndexAndGitModules => "missing in index and .gitmodules",
         }
     }
 }