Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,11 @@ pub async fn run() -> Result<()> {
// - Migrate: avoid errors during migration with potentially invalid configs
// - Completion/Usage: shell completion generation shouldn't require valid config
// - Version: just prints version info
// - Builtins: just lists compiled-in builtin names, no project config needed
let settings = if matches!(
args.command,
Commands::Init(_)
Commands::Builtins(_)
| Commands::Init(_)
| Commands::Migrate(_)
| Commands::Completion(_)
| Commands::Usage(_)
Expand Down
43 changes: 32 additions & 11 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,41 @@ pub enum StashMethod {

impl Git {
pub fn new() -> Result<Self> {
let cwd = std::env::current_dir()?;
let root = xx::file::find_up(&cwd, &[".git"])
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
.ok_or(eyre!("failed to find git repository"))?;
std::env::set_current_dir(&root)?;
// Respect GIT_DIR / GIT_WORK_TREE so hk works with bare-repo dotfile
// managers like YADM where there is no `.git` in the work tree.
let has_git_env =
std::env::var_os("GIT_DIR").is_some() || std::env::var_os("GIT_WORK_TREE").is_some();

if !has_git_env {
let cwd = std::env::current_dir()?;
let root = xx::file::find_up(&cwd, &[".git"])
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
.ok_or(eyre!("failed to find git repository"))?;
std::env::set_current_dir(&root)?;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

By skipping set_current_dir when GIT_DIR or GIT_WORK_TREE is set, hk remains in the user's current working directory. This breaks many internal operations (like status or all_files) that expect the process to be at the repository root to correctly resolve relative paths returned by Git.

Additionally, if we do change the directory, any relative paths in GIT_DIR or GIT_WORK_TREE must be converted to absolute paths first, otherwise they will point to the wrong location after the cd.

        let root = crate::git_util::find_work_tree_root();

        if !has_git_env {
            let cwd = std::env::current_dir()?;
            if xx::file::find_up(&cwd, &[".git"]).is_none() {
                return Err(eyre!("failed to find git repository"));
            }
        }

        // Absolute-ify relative git env vars before changing directory
        for var in ["GIT_DIR", "GIT_WORK_TREE"] {
            if let Some(val) = std::env::var_os(var) {
                let p = std::path::Path::new(&val);
                if p.is_relative() {
                    std::env::set_var(var, std::env::current_dir()?.join(p));
                }
            }
        }

        std::env::set_current_dir(&root)?;

Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
let repo = if *env::HK_LIBGIT2 {
debug!("libgit2: true");
let repo = Repository::open(".").wrap_err("failed to open repository")?;
if let Some(index_file) = &*env::GIT_INDEX_FILE {
// sets index to .git/index.lock which is used in the case of `git commit -a`
let mut index = git2::Index::open(index_file).wrap_err("failed to get index")?;
repo.set_index(&mut index)?;
let repo = if has_git_env {
Repository::open_from_env().wrap_err("failed to open repository")?
} else {
Repository::open(".").wrap_err("failed to open repository")?
};
// libgit2 status/diff APIs refuse to operate on a bare repository.
// For bare-repo dotfile managers (YADM, etc.) the work tree is
// provided via GIT_WORK_TREE but libgit2 still flags the repo as
// bare — fall back to the shell-git path so those operations work.
if repo.is_bare() {
debug!("libgit2: bare repo detected, falling back to shell git");
None
} else {
if let Some(index_file) = &*env::GIT_INDEX_FILE {
// sets index to .git/index.lock which is used in the case of `git commit -a`
let mut index =
git2::Index::open(index_file).wrap_err("failed to get index")?;
repo.set_index(&mut index)?;
}
Some(repo)
}
Some(repo)
} else {
debug!("libgit2: false");
None
Expand Down
27 changes: 27 additions & 0 deletions src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,39 @@ use eyre::eyre;
use crate::Result;

/// Find the `.git` path from the current working directory by searching upward.
///
/// Honors `GIT_DIR` if set (used by bare-repo dotfile managers like YADM), in
/// which case the returned path may be a bare repository directory rather than
/// a `.git` file/dir.
pub fn find_git_path() -> Result<PathBuf> {
if let Some(git_dir) = std::env::var_os("GIT_DIR") {
let p = PathBuf::from(&git_dir);
let p = if p.is_absolute() {
p
} else {
std::env::current_dir()?.join(p)
};
return Ok(p);
}
let cwd = std::env::current_dir()?;
xx::file::find_up(&cwd, &[".git"])
.ok_or_else(|| eyre!("No .git found in this or any parent directory"))
}

/// Return the effective working-tree root, honoring `GIT_WORK_TREE` when set
/// (for bare-repo setups like YADM). Falls back to walking up for `.git`, and
/// finally to `cwd` if no repository is found.
pub fn find_work_tree_root() -> PathBuf {
let cwd = std::env::current_dir().unwrap_or_default();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using unwrap_or_default() on current_dir() returns an empty PathBuf if the call fails. This can lead to confusing behavior or incorrect path joining later. It's safer to handle the error or at least fall back to . explicitly.

Suggested change
let cwd = std::env::current_dir().unwrap_or_default();
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));

if let Some(wt) = std::env::var_os("GIT_WORK_TREE") {
let p = PathBuf::from(&wt);
return if p.is_absolute() { p } else { cwd.join(p) };
}
xx::file::find_up(&cwd, &[".git"])
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
.unwrap_or(cwd)
}

/// Given a `.git` path (found by find_up), resolve the actual git directory.
/// - If `.git` is a directory → return it as-is
/// - If `.git` is a file (worktree) → read it, parse "gitdir: <path>", resolve that path
Expand Down
7 changes: 2 additions & 5 deletions src/tera.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{path::Path, sync::LazyLock};

use crate::{Result, step::ShellType};
use crate::{Result, git_util, step::ShellType};
use itertools::Itertools;
use serde::Serialize;
use tera::Tera;
Expand All @@ -13,10 +13,7 @@ pub fn render(input: &str, ctx: &Context) -> Result<String> {

static BASE_CONTEXT: LazyLock<tera::Context> = LazyLock::new(|| {
let mut ctx = tera::Context::new();
let cwd = std::env::current_dir().expect("failed to get current directory");
let root = xx::file::find_up(&cwd, &[".git"])
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
.unwrap_or(cwd);
let root = git_util::find_work_tree_root();
ctx.insert("color", &console::colors_enabled_stderr());
ctx.insert("root", &root.display().to_string());
ctx
Expand Down
8 changes: 2 additions & 6 deletions src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use std::time::Instant;

use crate::{
Result,
Result, git_util,
step::RunType,
step::Step,
step_test::{RunKind, StepTest},
Expand Down Expand Up @@ -169,14 +169,10 @@ pub async fn run_test_named(step: &Step, name: &str, test: &StepTest) -> Result<
files = step.filter_files(&files)?;
}

let cwd = std::env::current_dir().unwrap_or_default();
let root = xx::file::find_up(&cwd, &[".git"])
.and_then(|p| p.parent().map(|p| p.to_path_buf()))
.unwrap_or(cwd);
let base_dir = if uses_sandbox {
sandbox.to_path_buf()
} else {
root
git_util::find_work_tree_root()
};
if let Some(fixture) = &test.fixture {
let src = PathBuf::from(fixture);
Expand Down
87 changes: 87 additions & 0 deletions test/bare_repo_env_vars.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#!/usr/bin/env bats

# Regression tests for #831 — support bare-repo dotfile managers (YADM, etc.)
# that set GIT_DIR and GIT_WORK_TREE instead of using a `.git` in the worktree.

setup() {
load 'test_helper/common_setup'
_common_setup

BARE_DIR="$TEST_TEMP_DIR/bare.git"
WORK_TREE="$TEST_TEMP_DIR/home"
git init --bare "$BARE_DIR"
mkdir -p "$WORK_TREE"

export GIT_DIR="$BARE_DIR"
export GIT_WORK_TREE="$WORK_TREE"
cd "$WORK_TREE"

echo "initial" > file.txt
git add file.txt
git commit -m "initial commit"
}

teardown() {
unset GIT_DIR
unset GIT_WORK_TREE
_common_teardown
}

_write_hk_config() {
cat <<EOF > hk.pkl
amends "$PKL_PATH/Config.pkl"
hooks {
["check"] { steps { ["echo"] { check = "echo checked {{files}}" } } }
["pre-commit"] { steps { ["echo"] { check = "echo pre-commit {{files}}" } } }
}
EOF
}

@test "hk builtins works with no repo config" {
# Outside any repo, with no hk.pkl, should not panic
cd "$TEST_TEMP_DIR"
unset GIT_DIR
unset GIT_WORK_TREE
run hk builtins
assert_success
assert_output --partial "prettier"
}

@test "hk check honors GIT_DIR/GIT_WORK_TREE" {
_write_hk_config
git add hk.pkl
git commit -m "add hk config"

run hk check --all
assert_success
assert_output --partial "checked"
}

@test "hk check with HK_LIBGIT2=0 honors GIT_DIR/GIT_WORK_TREE" {
_write_hk_config
git add hk.pkl
git commit -m "add hk config"

HK_LIBGIT2=0 run hk check --all
assert_success
assert_output --partial "checked"
}

@test "hk install writes hooks to the bare-repo hooks dir" {
_write_hk_config

run hk install
assert_success
assert_file_exists "$BARE_DIR/hooks/pre-commit"
}

@test "hk uninstall removes hooks from the bare-repo hooks dir" {
_write_hk_config

hk install
assert_file_exists "$BARE_DIR/hooks/pre-commit"

run hk uninstall
assert_success
assert_file_not_exists "$BARE_DIR/hooks/pre-commit"
}
Loading