fix: Fix hooks dir for Git submodules/worktrees. (#1765)

* Clean up submodule.

* Add worktree.

* Update tests.

* Debug test.

* Fix python.

CHANGELOG.md

@@ -4,7 +4,9 @@
 
 #### 🐞 Fixes
 
+- Fixed an issue where `python venv` would fail to find an applicable Python version.
 - Fixed an issue with PowerShell Git hooks not bubbling up exit codes of failed commands.
+- Fixed an issue where Git submodules/worktrees would point to the wrong hooks folder.
 
 #### ⚙️ Internal
 

crates/process/Cargo.toml

@@ -20,7 +20,7 @@ starbase_shell = { workspace = true }
 system_env = { workspace = true }
 thiserror = { workspace = true }
 tracing = { workspace = true }
-tokio = { workspace = true }
+tokio = { workspace = true, features = ["io-util"] }
 
 [lints]
 workspace = true

crates/vcs/src/git.rs

@@ -1,4 +1,5 @@
 use crate::git_submodule::*;
+use crate::git_worktree::*;
 use crate::process_cache::ProcessCache;
 use crate::touched_files::TouchedFiles;
 use crate::vcs::Vcs;
@@ -94,6 +95,9 @@ pub enum GitError {
 
 #[derive(Debug)]
 pub struct Git {
+    /// Ignore rules derived from a root `.gitignore` file.
+    ignore: Option<Gitignore>,
+
     /// Default git branch name.
     pub default_branch: Arc<String>,
 
@@ -106,14 +110,14 @@ pub struct Git {
     /// List of remotes to use as merge candidates.
     pub remote_candidates: Vec<String>,
 
+    /// Root of the repository that contains `.git`.
+    pub repository_root: PathBuf,
+
     /// Path between the git and workspace root.
     pub root_prefix: Option<RelativePathBuf>,
 
-    /// If in a git worktree, the root of the worktree (the `.git` file).
-    pub worktree_root: Option<PathBuf>,
-
-    /// Ignore rules derived from a root `.gitignore` file.
-    ignore: Option<Gitignore>,
+    /// If in a git worktree, information about it's location (the `.git` file).
+    pub worktree: Option<GitWorktree>,
 
     /// Map of submodules within the repository.
     /// The root is also considered a module to keep things easy.
@@ -137,7 +141,7 @@ impl Git {
 
         // Find the .git dir
         let mut current_dir = workspace_root;
-        let mut worktree_root = None;
+        let mut worktree = None;
         let repository_root;
         let git_root;
 
@@ -148,13 +152,15 @@ impl Git {
                 if git_check.is_file() {
                     debug!(
                         git = ?git_check,
-                        "Found a .git file (worktree root)"
+                        "Found a .git file (submodule or worktree root)"
                     );
 
-                    git_root = extract_gitdir_from_worktree(&git_check)?;
-                    repository_root = current_dir.to_path_buf();
-                    worktree_root = Some(current_dir.to_path_buf());
-                    break;
+                    worktree = Some(GitWorktree {
+                        checkout_dir: current_dir.to_path_buf(),
+                        git_dir: extract_gitdir_from_worktree(&git_check)?,
+                    });
+
+                    // Don't break and continue searching for the root
                 } else {
                     debug!(
                         git = ?git_check,
@@ -209,32 +215,51 @@ impl Git {
 
         // Load .gitmodules
         let modules_path = repository_root.join(".gitmodules");
-        let mut modules = BTreeMap::from_iter([("(root)".into(), GitModule::default())]);
+        let mut modules = BTreeMap::from_iter([(
+            "(root)".into(),
+            GitModule {
+                checkout_dir: repository_root.clone(),
+                git_dir: git_root.clone(),
+                ..Default::default()
+            },
+        )]);
 
         if modules_path.exists() {
             debug!(
                 modules_file = ?modules_path,
                 "Loading submodules from .gitmodules",
             );
 
-            modules.extend(parse_gitmodules_file(&modules_path)?);
+            modules.extend(parse_gitmodules_file(&modules_path, &repository_root)?);
         }
 
-        Ok(Git {
+        let git = Git {
             default_branch: Arc::new(default_branch.as_ref().to_owned()),
             ignore,
             remote_candidates: remote_candidates.to_owned(),
             root_prefix: if repository_root == workspace_root {
                 None
+            } else if let Some(tree) = &worktree {
+                if tree.checkout_dir == workspace_root {
+                    None
+                } else {
+                    RelativePathBuf::from_path(
+                        workspace_root.strip_prefix(&tree.checkout_dir).unwrap(),
+                    )
+                    .ok()
+                }
             } else {
-                RelativePathBuf::from_path(workspace_root.strip_prefix(repository_root).unwrap())
+                RelativePathBuf::from_path(workspace_root.strip_prefix(&repository_root).unwrap())
                     .ok()
             },
+            repository_root,
             process: ProcessCache::new("git", workspace_root),
             git_root,
-            worktree_root,
+            worktree,
             modules,
-        })
+        };
+
+        Ok(git)
     }
 
     async fn get_merge_base(&self, base: &str, head: &str) -> miette::Result<Option<Arc<String>>> {
@@ -681,6 +706,10 @@ impl Vcs for Git {
         let is_in_repo =
             |dir: &Path| dir.is_absolute() && dir.starts_with(self.git_root.parent().unwrap());
 
+        if let Some(tree) = &self.worktree {
+            return Ok(tree.git_dir.join("hooks"));
+        }
+
         if let Ok(output) = self
             .process
             .run(["config", "--get", "core.hooksPath"], true)
@@ -706,8 +735,9 @@ impl Vcs for Git {
 
     async fn get_repository_root(&self) -> miette::Result<PathBuf> {
         Ok(self
-            .worktree_root
-            .as_deref()
+            .worktree
+            .as_ref()
+            .map(|tree| tree.checkout_dir.as_ref())
             .unwrap_or_else(|| self.git_root.parent().unwrap())
             .to_path_buf())
     }
@@ -856,26 +886,3 @@ impl Vcs for Git {
         Ok(result)
     }
 }
-
-fn extract_gitdir_from_worktree(git_file: &Path) -> miette::Result<PathBuf> {
-    let contents =
-        std::fs::read_to_string(git_file).map_err(|error| GitError::LoadWorktreeFailed {
-            path: git_file.to_owned(),
-            error: Box::new(error),
-        })?;
-
-    for line in contents.lines() {
-        if let Some(suffix) = line.strip_prefix("gitdir:") {
-            let git_dir = PathBuf::from(suffix.trim());
-
-            return Ok(git_dir
-                .canonicalize()
-                .map_err(|error| GitError::LoadWorktreeFailed {
-                    path: git_dir,
-                    error: Box::new(error),
-                })?);
-        }
-    }
-
-    Err(GitError::ParseWorktreeFailed.into())
-}

crates/vcs/src/git_submodule.rs

@@ -1,12 +1,23 @@
 use moon_common::path::RelativePathBuf;
 use rustc_hash::FxHashMap;
 use starbase_utils::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 #[derive(Debug, Default)]
 pub struct GitModule {
     pub branch: Option<String>,
+
+    /// Absolute path to where the submodule is checked out to within the repository.
+    pub checkout_dir: PathBuf,
+
+    /// Absolute path to the submodule's `.git` directory, which is housed in the
+    /// parent's `.git/modules`.
+    pub git_dir: PathBuf,
+
+    /// Relative path to the submodule checkout, defined in `.gitmodules`.
     pub path: RelativePathBuf,
+
+    /// URL of the repository.
     pub url: String,
 }
 
@@ -16,11 +27,15 @@ impl GitModule {
     }
 }
 
-pub fn parse_gitmodules_file(path: &Path) -> miette::Result<FxHashMap<String, GitModule>> {
+// https://git-scm.com/docs/gitmodules
+pub fn parse_gitmodules_file(
+    gitmodules_path: &Path,
+    repository_root: &Path,
+) -> miette::Result<FxHashMap<String, GitModule>> {
     let mut modules = FxHashMap::default();
     let mut current_module_name = None;
     let mut current_module = GitModule::default();
-    let contents = fs::read_file(path)?;
+    let contents = fs::read_file(gitmodules_path)?;
 
     fn clean_line(line: &str) -> String {
         line.replace("=", "").replace("\"", "").trim().to_owned()
@@ -58,6 +73,17 @@ pub fn parse_gitmodules_file(path: &Path) -> miette::Result<FxHashMap<String, Gi
 
     Ok(modules
         .into_iter()
-        .filter(|(_, module)| !module.path.as_str().is_empty())
+        .filter_map(|(name, mut module)| {
+            let rel_path = module.path.as_str();
+
+            if rel_path.is_empty() {
+                None
+            } else {
+                module.checkout_dir = repository_root.join(rel_path);
+                module.git_dir = repository_root.join(".git/modules").join(rel_path);
+
+                Some((name, module))
+            }
+        })
         .collect())
 }

crates/vcs/src/git_worktree.rs

@@ -0,0 +1,35 @@
+use crate::git::GitError;
+use std::fs;
+use std::path::{Path, PathBuf};
+
+#[derive(Debug, Default, PartialEq)]
+pub struct GitWorktree {
+    /// Absolute path to where the worktree is checked out to within the repository.
+    pub checkout_dir: PathBuf,
+
+    /// Absolute path to the worktree's `.git` directory, which is housed in the
+    /// parent's `.git/worktrees`.
+    pub git_dir: PathBuf,
+}
+
+pub fn extract_gitdir_from_worktree(path: &Path) -> miette::Result<PathBuf> {
+    let contents = fs::read_to_string(path).map_err(|error| GitError::LoadWorktreeFailed {
+        path: path.to_owned(),
+        error: Box::new(error),
+    })?;
+
+    for line in contents.lines() {
+        if let Some(suffix) = line.strip_prefix("gitdir:") {
+            let git_dir = PathBuf::from(suffix.trim());
+
+            return Ok(git_dir
+                .canonicalize()
+                .map_err(|error| GitError::LoadWorktreeFailed {
+                    path: git_dir,
+                    error: Box::new(error),
+                })?);
+        }
+    }
+
+    Err(GitError::ParseWorktreeFailed.into())
+}

crates/vcs/src/lib.rs

@@ -1,10 +1,12 @@
 mod git;
 mod git_submodule;
+mod git_worktree;
 mod process_cache;
 mod touched_files;
 mod vcs;
 
 pub use git::*;
+pub use git_worktree::*;
 pub use touched_files::*;
 pub use vcs::*;
 

crates/vcs/tests/git_test.rs

@@ -1,5 +1,5 @@
 use moon_common::path::{RelativePathBuf, WorkspaceRelativePathBuf};
-use moon_vcs::{clean_git_version, Git, TouchedFiles, Vcs};
+use moon_vcs::{clean_git_version, Git, GitWorktree, TouchedFiles, Vcs};
 use rustc_hash::FxHashSet;
 use starbase_sandbox::{create_sandbox, Sandbox};
 use std::collections::BTreeMap;
@@ -56,7 +56,7 @@ mod root_detection {
         let (sandbox, git) = create_git_sandbox("vcs");
 
         assert_eq!(git.git_root, sandbox.path().join(".git"));
-        assert_eq!(git.worktree_root, None);
+        assert_eq!(git.worktree, None);
         assert_eq!(git.process.root, sandbox.path());
         assert_eq!(git.root_prefix, None);
     }
@@ -68,7 +68,7 @@ mod root_detection {
         let git = Git::load(sandbox.path(), "master", &["origin".into()]).unwrap();
 
         assert_eq!(git.git_root, sandbox.path().join(".git"));
-        assert_eq!(git.worktree_root, None);
+        assert_eq!(git.worktree, None);
         assert_eq!(git.process.root, sandbox.path());
         assert_eq!(git.root_prefix, None);
     }
@@ -86,7 +86,7 @@ mod root_detection {
         .unwrap();
 
         assert_eq!(git.git_root, sandbox.path().join(".git"));
-        assert_eq!(git.worktree_root, None);
+        assert_eq!(git.worktree, None);
         assert_eq!(git.process.root, sandbox.path().join("nested/moon"));
         assert_eq!(git.root_prefix, Some(RelativePathBuf::from("nested/moon")));
     }
@@ -102,8 +102,18 @@ mod root_detection {
 
         let git = Git::load(sandbox.path().join("tree"), "master", &["origin".into()]).unwrap();
 
-        assert!(git.git_root.ends_with(".git/worktrees/tree"));
-        assert_eq!(git.worktree_root, Some(sandbox.path().join("tree")));
+        assert_eq!(git.git_root, sandbox.path().join(".git"));
+        assert_eq!(
+            git.worktree,
+            Some(GitWorktree {
+                checkout_dir: sandbox.path().join("tree"),
+                git_dir: sandbox
+                    .path()
+                    .join(".git/worktrees/tree")
+                    .canonicalize()
+                    .unwrap(),
+            })
+        );
         assert_eq!(git.process.root, sandbox.path().join("tree"));
         assert_eq!(git.root_prefix, None);
     }
@@ -124,8 +134,18 @@ mod root_detection {
         )
         .unwrap();
 
-        assert!(git.git_root.ends_with(".git/worktrees/tree"));
-        assert_eq!(git.worktree_root, Some(sandbox.path().join("tree")));
+        assert_eq!(git.git_root, sandbox.path().join(".git"));
+        assert_eq!(
+            git.worktree,
+            Some(GitWorktree {
+                checkout_dir: sandbox.path().join("tree"),
+                git_dir: sandbox
+                    .path()
+                    .join(".git/worktrees/tree")
+                    .canonicalize()
+                    .unwrap(),
+            })
+        );
         assert_eq!(git.process.root, sandbox.path().join("tree/nested/moon"));
         assert_eq!(git.root_prefix, Some(RelativePathBuf::from("nested/moon")));
     }