diff --git a/Makefile.cbm b/Makefile.cbm index e14a77ab0..0a2d16b94 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -340,6 +340,7 @@ TEST_DISCOVER_SRCS = \ tests/test_language.c \ tests/test_userconfig.c \ tests/test_gitignore.c \ + tests/test_git_context.c \ tests/test_discover.c TEST_GRAPH_BUFFER_SRCS = tests/test_graph_buffer.c diff --git a/src/git/git_context.c b/src/git/git_context.c index 99ae17cf3..14f48714d 100644 --- a/src/git/git_context.c +++ b/src/git/git_context.c @@ -126,15 +126,57 @@ static char *join_root_relative(const char *root, const char *rel) { return out; } -static char *derive_canonical_root(const char *worktree_root, const char *git_common_dir) { - const char *src = git_common_dir && git_common_dir[0] ? git_common_dir : worktree_root; - if (!src) { - return git_strdup(""); - } - - char *root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(worktree_root, src); - if (!root) { - return NULL; +/* Derive the canonical repo root. + * + * Preferred (git 2.31+): abs_common_dir is `git rev-parse --path-format=absolute + * --git-common-dir` — git's OWN absolute, canonical common-dir. Because a main + * repo and its linked worktree both ask the same git binary, they resolve to the + * IDENTICAL path, so canonical_root is consistent across worktrees AND platforms + * with no manual join or realpath/_fullpath — which diverged under msys vs native + * path representations (#659 root cause, and the worktree==main-root breakage in + * test_pipeline.c git_context_linked_worktree). git also resolves the relative + * ".." internally, so the subdirectory case (#659) is fixed here too. + * + * Fallback (git < 2.31, no --path-format → abs_common_dir empty): the relative + * --git-common-dir is relative to input_path (the -C dir), so join against it and + * realpath-normalize the "..". Unix only — on Windows git emits an absolute + * common-dir so this branch isn't reached in practice, and _fullpath there + * reintroduces the msys divergence. */ +static char *derive_canonical_root(const char *input_path, const char *worktree_root, + const char *git_common_dir, const char *abs_common_dir) { + char *root = NULL; + if (abs_common_dir && abs_common_dir[0] && path_is_absolute(abs_common_dir)) { + root = git_strdup(abs_common_dir); + if (!root) { + return NULL; + } + } else { + const char *src = git_common_dir && git_common_dir[0] ? git_common_dir : worktree_root; + if (!src) { + return git_strdup(""); + } +#ifndef _WIN32 + root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(input_path, src); + if (!root) { + return NULL; + } + { + char resolved[4096]; + if (realpath(root, resolved) != NULL) { + free(root); + root = git_strdup(resolved); + if (!root) { + return NULL; + } + } + } +#else + (void)input_path; + root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(worktree_root, src); + if (!root) { + return NULL; + } +#endif } size_t len = strlen(root); @@ -251,7 +293,13 @@ int cbm_git_context_resolve(const char *path, cbm_git_context_t *out) { out->is_worktree = out->git_dir && out->git_common_dir && strcmp(out->git_dir, out->git_common_dir) != 0; - out->canonical_root = derive_canonical_root(out->worktree_root, out->git_common_dir); + /* git 2.31+ canonical absolute common-dir (best-effort; NULL on older git, + * where derive_canonical_root falls back to the relative common-dir). */ + char *abs_common_dir = NULL; + (void)git_capture(path, "rev-parse --path-format=absolute --git-common-dir", &abs_common_dir); + out->canonical_root = + derive_canonical_root(path, out->worktree_root, out->git_common_dir, abs_common_dir); + free(abs_common_dir); out->branch_slug = slug_from_branch(out->branch, out->is_detached); if (git_capture(path, "merge-base HEAD @{upstream}", &out->base_sha) != 0) { out->base_sha = git_strdup(""); diff --git a/tests/test_git_context.c b/tests/test_git_context.c new file mode 100644 index 000000000..a384651a5 --- /dev/null +++ b/tests/test_git_context.c @@ -0,0 +1,241 @@ +/* + * test_git_context.c — Tests for cbm_git_context_resolve(), focusing on + * the canonical_root derivation for git worktrees and subdirectory projects. + * + * Issue #659: canonical_root was computed incorrectly for linked worktrees + * and projects indexed from a subdirectory of the repository root. + * git rev-parse --git-common-dir outputs a path relative to the -C directory + * (input_path), not to worktree_root. Joining it with worktree_root and then + * string-stripping "/.git" left unresolved ".." components in the result. + * + * These tests shell out to `git`, so they SKIP_PLATFORM on Windows (the CI + * shell there cannot init a repo via system()). + * + * Reproduce-first guard: canonical_root_subdir is the genuine RED-without-the-fix + * guard — a repo indexed from a subdirectory yields a relative --git-common-dir + * ("../.git"), so the unfixed code returns an un-normalized "/subdir/.." + * (verified FAIL on the unfixed derive_canonical_root; GREEN with the realpath + * normalization). canonical_root_linked_worktree is a SUPPORTING INVARIANT, not + * the #659 reproducer: on git that emits an *absolute* --git-common-dir for a + * linked worktree (e.g. 2.48.x) the bug does not manifest there, so that test + * passes with or without the fix. It still enforces the worktree->main-root + * invariant and would catch the bug on git builds that emit a relative + * worktree common-dir. canonical_root_repo_root is a baseline (no `..` to + * normalize), not a guard. + */ +#include "test_framework.h" +#include "test_helpers.h" +#include "git/git_context.h" + +#include +#include + +#ifndef _WIN32 +#include +#endif + +/* These helpers shell out to git and are only used by the non-Windows test + * bodies below; on Windows every test SKIP_PLATFORMs, so guard them here too or + * they'd be unused-static functions and fail the -Werror build. */ +#ifndef _WIN32 +/* Run a git command inside dir, return 0 on success. */ +static int git_run(const char *dir, const char *args) { + char cmd[1024]; + snprintf(cmd, sizeof(cmd), "git -C \"%s\" %s >/dev/null 2>&1", dir, args); + return system(cmd); +} + +/* Create a minimal git repo at dir (init + empty commit so HEAD exists). */ +static int make_git_repo(const char *dir) { + if (th_mkdir_p(dir) != 0) return -1; + if (git_run(dir, "init -q") != 0) return -1; + if (git_run(dir, "config user.email test@example.com") != 0) return -1; + if (git_run(dir, "config user.name Test") != 0) return -1; + /* Create a file so HEAD points to a real commit. */ + char path[1024]; + snprintf(path, sizeof(path), "%s/.keep", dir); + th_write_file(path, ""); + if (git_run(dir, "add .keep") != 0) return -1; + if (git_run(dir, "commit -q -m init") != 0) return -1; + return 0; +} +#endif /* _WIN32 */ + +/* ── canonical_root: normal repo indexed from its root ──────────── */ + +TEST(canonical_root_repo_root) { +#ifdef _WIN32 + SKIP_PLATFORM("git-based canonical_root test not supported on Windows CI"); +#else + char *tmp = th_mktempdir("cbm_gitctx"); + if (!tmp) FAIL("th_mktempdir returned NULL"); + + if (make_git_repo(tmp) != 0) { + th_rmtree(tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(tmp, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("cbm_git_context_resolve failed or not a git repo"); + } + + char expected[4096]; + if (realpath(tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("realpath(tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + + cbm_git_context_free(&ctx); + th_rmtree(tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── canonical_root: indexed from a subdirectory (issue #659) ───── + * THE reproduce-first guard: from a subdir, --git-common-dir is relative, so the + * unfixed derive_canonical_root joins it against worktree_root and strips "/.git" + * textually, leaving canonical_root = "/subdir/.." (or "/..") instead + * of "". Verified RED on the unfixed code, GREEN with the realpath fix. */ + +TEST(canonical_root_subdir) { +#ifdef _WIN32 + SKIP_PLATFORM("git-based canonical_root test not supported on Windows CI"); +#else + char *tmp = th_mktempdir("cbm_gitctx"); + if (!tmp) FAIL("th_mktempdir returned NULL"); + + if (make_git_repo(tmp) != 0) { + th_rmtree(tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + /* Create a subdirectory inside the repo. */ + char subdir[1024]; + snprintf(subdir, sizeof(subdir), "%s/scripts", tmp); + if (th_mkdir_p(subdir) != 0) { + th_rmtree(tmp); + FAIL("failed to create subdir"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(subdir, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("cbm_git_context_resolve on subdir failed or not a git repo"); + } + + /* canonical_root must equal the repo root, NOT "/.." or "/..". */ + char expected[4096]; + if (realpath(tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("realpath(tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + + /* Sanity: canonical_root must not contain ".." or end with a slash. */ + ASSERT(strstr(ctx.canonical_root, "..") == NULL); + ASSERT(ctx.canonical_root[strlen(ctx.canonical_root) - 1] != '/'); + + cbm_git_context_free(&ctx); + th_rmtree(tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── canonical_root: linked git worktree (supporting invariant) ──── + * NOT the #659 reproducer on modern git: git that emits an *absolute* + * --git-common-dir for a linked worktree (e.g. 2.48.x) takes the path_is_absolute + * branch, so the bug does not manifest and this passes with or without the fix. + * It is kept as an invariant — canonical_root of a linked worktree must equal the + * MAIN repo root (never the worktree root or its parent) — and would fail on a git + * build that emits a *relative* worktree common-dir. The genuine RED-without-fix + * guard for #659 is canonical_root_subdir above. */ + +TEST(canonical_root_linked_worktree) { +#ifdef _WIN32 + SKIP_PLATFORM("git worktree test not implemented for Windows"); +#else + /* th_mktempdir() returns a static buffer — copy before the second call. */ + char main_tmp[256]; + char *raw = th_mktempdir("cbm_main"); + if (!raw) FAIL("th_mktempdir returned NULL"); + strncpy(main_tmp, raw, sizeof(main_tmp) - 1); + main_tmp[sizeof(main_tmp) - 1] = '\0'; + + char wt_tmp[256]; + raw = th_mktempdir("cbm_worktree"); + if (!raw) FAIL("th_mktempdir returned NULL"); + strncpy(wt_tmp, raw, sizeof(wt_tmp) - 1); + wt_tmp[sizeof(wt_tmp) - 1] = '\0'; + + /* Remove the worktree dir first — git worktree add creates it. */ + th_rmtree(wt_tmp); + + if (make_git_repo(main_tmp) != 0) { + th_rmtree(main_tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + /* Create a branch for the worktree. */ + if (git_run(main_tmp, "branch wt-branch") != 0) { + th_rmtree(main_tmp); + FAIL("failed to create branch for worktree"); + } + + /* Add a linked worktree. */ + char wt_cmd[1024]; + snprintf(wt_cmd, sizeof(wt_cmd), "worktree add \"%s\" wt-branch", wt_tmp); + if (git_run(main_tmp, wt_cmd) != 0) { + th_rmtree(wt_tmp); + th_rmtree(main_tmp); + SKIP_PLATFORM("git worktree add unavailable (git 2.5+ required)"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(wt_tmp, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + FAIL("cbm_git_context_resolve on linked worktree failed"); + } + + /* canonical_root must be the MAIN repo root, not the worktree root or its parent. */ + char expected[4096]; + if (realpath(main_tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + FAIL("realpath(main_tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + ASSERT(strstr(ctx.canonical_root, "..") == NULL); + + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── Suite ──────────────────────────────────────────────────────── */ + +SUITE(git_context) { + RUN_TEST(canonical_root_repo_root); + RUN_TEST(canonical_root_subdir); + RUN_TEST(canonical_root_linked_worktree); +} diff --git a/tests/test_main.c b/tests/test_main.c index 824b91fe8..d0cca85d7 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -58,6 +58,7 @@ extern void suite_mcp(void); extern void suite_language(void); extern void suite_userconfig(void); extern void suite_gitignore(void); +extern void suite_git_context(void); extern void suite_discover(void); extern void suite_graph_buffer(void); extern void suite_registry(void); @@ -173,6 +174,7 @@ int main(int argc, char **argv) { RUN_SELECTED_SUITE(language); RUN_SELECTED_SUITE(userconfig); RUN_SELECTED_SUITE(gitignore); + RUN_SELECTED_SUITE(git_context); RUN_SELECTED_SUITE(discover); /* Graph Buffer (M7) */