Skip to content

Commit

Permalink
fix(init): make relative hook path work in worktrees
Browse files Browse the repository at this point in the history
Since init previously used the root worktree for all of its actions, it
failed to resolve `core.hooksPath` relative to the worktree it was being
run in. Change `init` to work with the current worktree, and update code
to explicitly refer to the shared git directory when necessary. This
also entirely removes the need for `open_worktree_parent_repo`.
  • Loading branch information
ethanwu10 committed Jan 28, 2025
1 parent 4cc3812 commit 4d6d5fb
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 42 deletions.
5 changes: 2 additions & 3 deletions git-branchless-init/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ fn create_isolated_config(

let config = Config::open(&config_path)?;
let config_path_relative = config_path
.strip_prefix(repo.get_path())
.strip_prefix(repo.get_shared_path())
.wrap_err("Getting relative config path")?;
// Be careful when setting paths on Windows. Since the path would have a
// backslash, naively using it produces
Expand Down Expand Up @@ -606,8 +606,7 @@ fn command_init(
main_branch_name: Option<&str>,
) -> EyreExitOr<()> {
let mut in_ = BufReader::new(stdin());
let repo = Repo::from_current_dir()?;
let mut repo = repo.open_worktree_parent_repo()?.unwrap_or(repo);
let mut repo = Repo::from_current_dir()?;

let default_config = Config::open_default()?;
let readonly_config = repo.get_readonly_config()?;
Expand Down
4 changes: 1 addition & 3 deletions git-branchless-lib/src/core/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ use super::eventlog::EventTransactionId;
/// overridden it.
#[instrument]
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"))
Ok(repo.get_shared_path().join("hooks"))
}

/// Get the path where the main worktree's Git hooks are stored on disk.
Expand Down
49 changes: 13 additions & 36 deletions git-branchless-lib/src/git/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ pub enum Error {
#[error("could not open repository: {0}")]
OpenRepo(#[source] git2::Error),

#[error("could not find repository to open for worktree {path:?}")]
OpenParentWorktreeRepository { path: PathBuf },

#[error("could not open repository: {0}")]
UnsupportedExtensionWorktreeConfig(#[source] git2::Error),

Expand Down Expand Up @@ -505,11 +502,22 @@ impl Repo {
Ok(Repo { inner: repo })
}

/// Get the path to the `.git` directory for the repository.
/// Get the path to the local `.git` directory for the repository.
///
/// If this repository is a non-root worktree, this will return the path to
/// the worktree's local directory, i.e. `.git/worktrees/<name>`
pub fn get_path(&self) -> &Path {
self.inner.path()
}

/// Get the path to the shared common `.git` directory for the repository.
///
/// This always gets the shared common `.git` directory instead of the
/// per-worktree directory (`.git/worktrees/...`).
pub fn get_shared_path(&self) -> &Path {
self.inner.commondir()
}

/// Get the path to the `packed-refs` file for the repository.
pub fn get_packed_refs_path(&self) -> PathBuf {
self.inner.path().join("packed-refs")
Expand Down Expand Up @@ -569,32 +577,6 @@ impl Repo {
Ok(Index { inner: index })
}

/// If this repository is a worktree for another "parent" repository, return a [`Repo`] object
/// corresponding to that repository.
#[instrument]
pub fn open_worktree_parent_repo(&self) -> Result<Option<Self>> {
if !self.inner.is_worktree() {
return Ok(None);
}

// `git2` doesn't seem to support a way to directly look up the parent repository for the
// worktree.
let worktree_info_dir = self.get_path();
let parent_repo_path = match worktree_info_dir
.parent() // remove `.git`
.and_then(|path| path.parent()) // remove worktree name
.and_then(|path| path.parent()) // remove `worktrees`
{
Some(path) => path,
None => {
return Err(Error::OpenParentWorktreeRepository {
path: worktree_info_dir.to_owned()});
},
};
let parent_repo = Self::from_dir(parent_repo_path)?;
Ok(Some(parent_repo))
}

/// Get the configuration object for the repository.
///
/// **Warning**: This object should only be used for read operations. Write
Expand All @@ -608,12 +590,7 @@ impl Repo {

/// Get the directory where all repo-specific git-branchless state is stored.
pub fn get_branchless_dir(&self) -> Result<PathBuf> {
let maybe_worktree_parent_repo = self.open_worktree_parent_repo()?;
let repo = match maybe_worktree_parent_repo.as_ref() {
Some(repo) => repo,
None => self,
};
let dir = repo.get_path().join("branchless");
let dir = self.get_shared_path().join("branchless");
std::fs::create_dir_all(&dir).map_err(|err| Error::CreateBranchlessDir {
source: err,
path: dir.clone(),
Expand Down
107 changes: 107 additions & 0 deletions git-branchless/tests/test_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,113 @@ fn test_init_worktree() -> eyre::Result<()> {
Ok(())
}

#[test]
fn test_init_worktree_from_subdir() -> eyre::Result<()> {
let git = make_git()?;
git.init_repo_with_options(&GitInitOptions {
run_branchless_init: false,
make_initial_commit: true,
})?;
git.commit_file("test1", 1)?;
git.commit_file("test2", 2)?;

let GitWorktreeWrapper {
temp_dir: _temp_dir,
worktree,
} = make_git_worktree(&git, "new-worktree")?;

let subdir = worktree.repo_path.join("subdir");
std::fs::create_dir(&subdir)?;

worktree.branchless_with_options(
"init",
&[],
&GitRunOptions {
working_dir: Some(subdir),
..Default::default()
},
)?;
{
let (stdout, stderr) = worktree.run(&["commit", "--allow-empty", "-m", "test"])?;
insta::assert_snapshot!(stdout, @"[detached HEAD a3e6886] test
");
insta::assert_snapshot!(stderr, @r###"
branchless: processing 1 update: ref HEAD
branchless: processed commit: a3e6886 test
"###);
}

Ok(())
}

#[test]
fn test_init_worktree_from_subdir_with_hooks_path() -> eyre::Result<()> {
let git = make_git()?;
git.init_repo_with_options(&GitInitOptions {
run_branchless_init: false,
make_initial_commit: true,
})?;
git.commit_file("test1", 1)?;
git.commit_file("test2", 2)?;

let GitWorktreeWrapper {
temp_dir: _temp_dir,
worktree,
} = make_git_worktree(&git, "new-worktree")?;

let hooks_path = git.get_repo()?.get_path().join("my-hooks");
std::fs::create_dir(hooks_path)?;
git.run(&["config", "core.hooksPath", "my-hooks"])?;

let subdir = worktree.repo_path.join("subdir");
std::fs::create_dir(&subdir)?;

{
let (stdout, stderr) = worktree.branchless_with_options(
"init",
&[],
&GitRunOptions {
working_dir: Some(subdir),
..Default::default()
},
)?;

// Copied from [Git::preprocess_output] - replace the path of the root
// repo.
let stdout = stdout.replace(
std::fs::canonicalize(&git.repo_path)?
.to_str()
.ok_or_else(|| eyre::eyre!("Could not convert repo path to string"))?,
"<root-repo-path>",
);

insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stdout, @r###"
Created config file at <root-repo-path>/.git/branchless/config
Auto-detected your main branch as: master
If this is incorrect, run: git branchless init --main-branch <branch>
Installing hooks: post-applypatch, post-checkout, post-commit, post-merge, post-rewrite, pre-auto-gc, reference-transaction
Warning: the configuration value core.hooksPath was set to: <repo-path>/my-hooks,
which is not the expected default value of: <root-repo-path>/.git/hooks
The Git hooks above may have been installed to an unexpected global location.
Successfully installed git-branchless.
To uninstall, run: git branchless init --uninstall
"###);
}

{
let (stdout, stderr) = worktree.run(&["commit", "--allow-empty", "-m", "test"])?;
insta::assert_snapshot!(stdout, @"[detached HEAD a3e6886] test
");
insta::assert_snapshot!(stderr, @r###"
branchless: processing 1 update: ref HEAD
branchless: processed commit: a3e6886 test
"###);
}

Ok(())
}

#[test]
fn test_install_man_pages() -> eyre::Result<()> {
let git = make_git()?;
Expand Down

0 comments on commit 4d6d5fb

Please sign in to comment.