fix(hooks): invoke Git hooks in worktrees

Author: arxanas

git-branchless-init/src/lib.rs

@@ -23,7 +23,9 @@ use path_slash::PathExt;
 use tracing::{instrument, warn};
 
 use git_branchless_opts::{write_man_pages, InitArgs, InstallManPagesArgs};
-use lib::core::config::{get_default_branch_name, get_default_hooks_dir, get_hooks_dir};
+use lib::core::config::{
+    get_default_branch_name, get_default_hooks_dir, get_main_worktree_hooks_dir,
+};
 use lib::core::dag::Dag;
 use lib::core::effects::Effects;
 use lib::core::eventlog::{EventLogDb, EventReplayer};
@@ -249,12 +251,12 @@ fn install_hooks(effects: &Effects, git_run_info: &GitRunInfo, repo: &Repo) -> e
             .map(|(hook_type, _hook_script)| hook_type)
             .join(", ")
     )?;
-    let hooks_dir = get_hooks_dir(git_run_info, repo, None)?;
+    let hooks_dir = get_main_worktree_hooks_dir(git_run_info, repo, None)?;
     for (hook_type, hook_script) in ALL_HOOKS {
         install_hook(repo, &hooks_dir, hook_type, hook_script)?;
     }
 
-    let default_hooks_dir = get_default_hooks_dir(repo);
+    let default_hooks_dir = get_default_hooks_dir(repo)?;
     if hooks_dir != default_hooks_dir {
         writeln!(
             effects.get_output_stream(),
@@ -281,7 +283,7 @@ fn uninstall_hooks(effects: &Effects, git_run_info: &GitRunInfo, repo: &Repo) ->
             .map(|(hook_type, _hook_script)| hook_type)
             .join(", ")
     )?;
-    let hooks_dir = get_hooks_dir(git_run_info, repo, None)?;
+    let hooks_dir = get_main_worktree_hooks_dir(git_run_info, repo, None)?;
     for (hook_type, _hook_script) in ALL_HOOKS {
         install_hook(
             repo,

git-branchless-lib/src/core/config.rs

@@ -18,13 +18,24 @@ use super::eventlog::EventTransactionId;
 /// Get the expected hooks dir inside `.git`, assuming that the user has not
 /// overridden it.
 #[instrument]
-pub fn get_default_hooks_dir(repo: &Repo) -> PathBuf {
-    repo.get_path().join("hooks")
+pub fn get_default_hooks_dir(repo: &Repo) -> eyre::Result<PathBuf> {
+    let parent_repo = repo.open_worktree_parent_repo()?;
+    let repo = parent_repo.as_ref().unwrap_or(repo);
+    Ok(repo.get_path().join("hooks"))
 }
 
-/// Get the path where Git hooks are stored on disk.
+/// Get the path where the main worktree's Git hooks are stored on disk.
+///
+/// Git hooks live at `$GIT_DIR/hooks` by default, which means that they will be
+/// different per wortkree. Most people, when creating a new worktree, will not
+/// also reinstall hooks or reinitialize git-branchless in that worktree, so we
+/// instead look up hooks for the main worktree, which is most likely to have them
+/// installed.
+///
+/// This could in theory cause problems for users who have different
+/// per-worktree hooks.
 #[instrument]
-pub fn get_hooks_dir(
+pub fn get_main_worktree_hooks_dir(
     git_run_info: &GitRunInfo,
     repo: &Repo,
     event_tx_id: Option<EventTransactionId>,
@@ -45,7 +56,7 @@ pub fn get_hooks_dir(
             .context("Decoding git config output for hooks path")?;
         PathBuf::from(path.strip_suffix('\n').unwrap_or(&path))
     } else {
-        get_default_hooks_dir(repo)
+        get_default_hooks_dir(repo)?
     };
     Ok(hooks_path)
 }

git-branchless-lib/src/core/eventlog.rs

@@ -471,7 +471,7 @@ pub struct EventLogDb<'conn> {
 
 impl std::fmt::Debug for EventLogDb<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "<EventLogDb>")
+        write!(f, "<EventLogDb path={:?}>", self.conn.path())
     }
 }
 

git-branchless-lib/src/git/run.rs

@@ -11,9 +11,9 @@ use std::thread::{self, JoinHandle};
 use bstr::BString;
 use eyre::{eyre, Context};
 use itertools::Itertools;
-use tracing::instrument;
+use tracing::{instrument, warn};
 
-use crate::core::config::get_hooks_dir;
+use crate::core::config::get_main_worktree_hooks_dir;
 use crate::core::effects::{Effects, OperationType};
 use crate::core::eventlog::{EventTransactionId, BRANCHLESS_TRANSACTION_ID_ENV_VAR};
 use crate::git::repo::Repo;
@@ -394,8 +394,13 @@ impl GitRunInfo {
         args: &[&str],
         stdin: Option<BString>,
     ) -> eyre::Result<()> {
-        let hook_dir = get_hooks_dir(self, repo, Some(event_tx_id))?;
+        let hook_dir = get_main_worktree_hooks_dir(self, repo, Some(event_tx_id))?;
         if !hook_dir.exists() {
+            warn!(
+                ?hook_dir,
+                ?hook_name,
+                "Git hooks dir did not exist, so could not invoke hook"
+            );
             return Ok(());
         }
 

git-branchless/src/commands/bug_report.rs

@@ -8,7 +8,7 @@ use bugreport::collector::{CollectionError, Collector};
 use bugreport::format::Markdown;
 use bugreport::report::ReportEntry;
 use itertools::Itertools;
-use lib::core::config::get_hooks_dir;
+use lib::core::config::get_main_worktree_hooks_dir;
 use lib::core::repo_ext::{RepoExt, RepoReferencesSnapshot};
 use lib::util::EyreExitOr;
 
@@ -242,7 +242,7 @@ struct HookCollector {
 
 fn collect_hooks(git_run_info: &GitRunInfo) -> eyre::Result<ReportEntry> {
     let repo = Repo::from_current_dir()?;
-    let hooks_dir = get_hooks_dir(git_run_info, &repo, None)?;
+    let hooks_dir = get_main_worktree_hooks_dir(git_run_info, &repo, None)?;
     let hook_contents = {
         let mut result = Vec::new();
         for (hook_type, _content) in ALL_HOOKS {

git-branchless/tests/test_init.rs

@@ -314,7 +314,7 @@ fn test_main_branch_not_found_error_message() -> eyre::Result<()> {
 
       ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-       0: branchless::core::eventlog::from_event_log_db with effects=<Output fancy=false> repo=<Git repository at: "<repo-path>/.git/"> event_log_db=<EventLogDb>
+       0: branchless::core::eventlog::from_event_log_db with effects=<Output fancy=false> repo=<Git repository at: "<repo-path>/.git/"> event_log_db=<EventLogDb path=Some("<repo-path>/.git/branchless/db.sqlite3")>
           at some/file/path.rs:123
        1: git_branchless_smartlog::smartlog with effects=<Output fancy=false> git_run_info=<GitRunInfo path_to_git="<git-executable>" working_directory="<repo-path>" env=not shown> options=SmartlogOptions { event_id: None, revset: None, resolve_revset_options: ResolveRevsetOptions { show_hidden_commits: false }, reverse: false }
           at some/file/path.rs:123

git-branchless/tests/test_move.rs

@@ -1,6 +1,7 @@
 use lib::testing::{
-    extract_hint_command, make_git, make_git_with_remote_repo, remove_rebase_lines, GitInitOptions,
-    GitRunOptions, GitWrapperWithRemoteRepo,
+    extract_hint_command, make_git, make_git_with_remote_repo, make_git_worktree,
+    remove_rebase_lines, GitInitOptions, GitRunOptions, GitWorktreeWrapper,
+    GitWrapperWithRemoteRepo,
 };
 
 #[test]
@@ -6237,3 +6238,85 @@ fn test_move_fixup_added_files() -> eyre::Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn test_worktree_rebase_in_memory() -> eyre::Result<()> {
+    let git = make_git()?;
+
+    if !git.supports_reference_transactions()? {
+        return Ok(());
+    }
+    git.init_repo()?;
+
+    git.commit_file("test1", 1)?;
+    git.detach_head()?;
+    git.commit_file("test2", 2)?;
+    git.commit_file("test3", 3)?;
+
+    let GitWorktreeWrapper {
+        temp_dir: _temp_dir,
+        worktree,
+    } = make_git_worktree(&git, "new-worktree")?;
+    git.run(&["checkout", "master"])?;
+    {
+        let stdout = worktree.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        :
+        O 62fc20d (master) create test1.txt
+        |
+        o 96d1c37 create test2.txt
+        |
+        @ 70deb1e create test3.txt
+        "###);
+    }
+
+    {
+        let (stdout, stderr) = worktree.branchless("move", &["-s", "@", "-d", "master"])?;
+        insta::assert_snapshot!(stderr, @r###"
+        branchless: creating working copy snapshot
+        Previous HEAD position was 70deb1e create test3.txt
+        branchless: processing 1 update: ref HEAD
+        HEAD is now at 4838e49 create test3.txt
+        branchless: processing checkout
+        "###);
+        insta::assert_snapshot!(stdout, @r###"
+        Attempting rebase in-memory...
+        [1/1] Committed as: 4838e49 create test3.txt
+        branchless: processing 1 rewritten commit
+        branchless: running command: <git-executable> checkout 4838e49b08954becdd17c0900c1179c2c654c627
+        :
+        O 62fc20d (master) create test1.txt
+        |\
+        | o 96d1c37 create test2.txt
+        |
+        @ 4838e49 create test3.txt
+        In-memory rebase succeeded.
+        "###);
+    }
+
+    {
+        let stdout = worktree.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        :
+        O 62fc20d (master) create test1.txt
+        |\
+        | o 96d1c37 create test2.txt
+        |
+        @ 4838e49 create test3.txt
+        "###);
+    }
+
+    {
+        let stdout = git.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        :
+        @ 62fc20d (> master) create test1.txt
+        |\
+        | o 96d1c37 create test2.txt
+        |
+        o 4838e49 create test3.txt
+        "###);
+    }
+
+    Ok(())
+}