Skip to content

Commit

Permalink
fix(hooks): invoke Git hooks in worktrees
Browse files Browse the repository at this point in the history
  • Loading branch information
arxanas committed Oct 22, 2023
1 parent 12ad204 commit 40cdcab
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 18 deletions.
10 changes: 6 additions & 4 deletions git-branchless-init/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
Expand Down
21 changes: 16 additions & 5 deletions git-branchless-lib/src/core/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion git-branchless-lib/src/core/eventlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down
11 changes: 8 additions & 3 deletions git-branchless-lib/src/git/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(());
}

Expand Down
4 changes: 2 additions & 2 deletions git-branchless/src/commands/bug_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion git-branchless/tests/test_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 85 additions & 2 deletions git-branchless/tests/test_move.rs
Original file line number Diff line number Diff line change
@@ -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]
Expand Down Expand Up @@ -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(())
}

0 comments on commit 40cdcab

Please sign in to comment.