diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index 38c1b0163..ec186c05a 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -75,6 +75,10 @@ typedef struct { uint64_t last_dirty_sig; /* committed dirty-state signature */ uint64_t pending_dirty_sig; /* observed at check time */ char pending_head[CBM_SZ_128]; /* HEAD observed at check time */ + /* Hop from root_path up to the repository root ("" when they are the same), + * from `rev-parse --show-cdup`. Porcelain paths are repository-relative, so + * the signature needs this to stat them. Resolved once at baseline. */ + char repo_cdup[CBM_SZ_4K]; } project_state_t; /* ── Watcher struct ─────────────────────────────────────────────── */ @@ -452,6 +456,86 @@ static watcher_git_status_t git_repo_status(cbm_watcher_t *w, project_state_t *s return watcher_git_run(w, state, argv, 0, NULL); } +/* True when root_path carries its OWN repository marker: a `.git` directory, or + * a `.git` file (the gitlink form used by linked worktrees and initialized + * submodules). Deliberately a filesystem check, not a git invocation — the whole + * point is to learn something `rev-parse` cannot tell us, because it walks up. */ +static bool git_has_own_dot_git(const char *root_path) { + char path[CBM_SZ_4K]; + int written = snprintf(path, sizeof(path), "%s/.git", root_path); + if (written <= 0 || (size_t)written >= sizeof(path)) { + return false; + } + struct stat st; + return stat(path, &st) == 0 && (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode)); +} + +/* Does the ancestor repository actually track anything inside this directory? + * Emptiness distinguishes "a scratch folder that merely sits under a repo" from + * "a genuine subdirectory of one". Output is capped: we only care whether the + * first byte exists, never what it is. */ +static watcher_git_status_t git_tracks_anything_here(cbm_watcher_t *w, project_state_t *state, + bool *tracked_out) { + *tracked_out = false; + const char *argv[] = {"git", "-C", state->root_path, "ls-files", "-z", "--", ".", NULL}; + watcher_git_output_t output; + watcher_git_status_t status = watcher_git_run(w, state, argv, WATCHER_GIT_HEAD_MAX, &output); + if (status != WATCHER_GIT_OK) { + return status; + } + FILE *fp = cbm_fopen(output.path, "rb"); + if (fp) { + *tracked_out = fgetc(fp) != EOF; + (void)fclose(fp); + } + watcher_git_output_cleanup(&output); + return fp ? WATCHER_GIT_OK : WATCHER_GIT_SUPERVISION_FAILED; +} + +/* Relative hop from root_path up to the repository root, as `git rev-parse + * --show-cdup` reports it ("" at the root, "../" one level down, and so on). + * + * This matters because `git status --porcelain` prints paths relative to the + * REPOSITORY root, while the signature stats them relative to root_path. For a + * project watched at the repository root the two coincide and the bug is + * invisible; for a subdirectory project every stat silently misses, and the + * signature quietly degrades to text-only — losing the size/mtime component + * that makes an edit to an already-dirty file detectable. + * + * --show-cdup rather than --show-toplevel: under MSYS/Cygwin git, --show-toplevel + * returns a translated absolute path (/c/... or a drive-letter form) that does + * not join cleanly onto the native root_path we hold. A relative hop composes + * correctly on every platform because it never leaves our own path space. */ +static watcher_git_status_t git_repo_cdup(cbm_watcher_t *w, project_state_t *state, char *out, + size_t out_size) { + if (!out || out_size < 2) { + return WATCHER_GIT_SUPERVISION_FAILED; + } + out[0] = '\0'; + const char *argv[] = {"git", "-C", state->root_path, "rev-parse", "--show-cdup", NULL}; + watcher_git_output_t output; + watcher_git_status_t status = watcher_git_run(w, state, argv, WATCHER_GIT_HEAD_MAX, &output); + if (status != WATCHER_GIT_OK) { + return status; + } + FILE *file = cbm_fopen(output.path, "rb"); + bool read = file && fgets(out, (int)out_size, file) != NULL; + if (file) { + (void)fclose(file); + } + watcher_git_output_cleanup(&output); + if (!read) { + /* At the repository root git prints an empty line; that is success. */ + out[0] = '\0'; + return WATCHER_GIT_OK; + } + size_t len = strlen(out); + while (len > 0 && (out[len - 1] == '\n' || out[len - 1] == '\r')) { + out[--len] = '\0'; + } + return WATCHER_GIT_OK; +} + static watcher_git_status_t git_head(cbm_watcher_t *w, project_state_t *state, char *out, size_t out_size) { if (!out || out_size < 2) { @@ -511,9 +595,14 @@ static int64_t sig_stat_mtime_ns(const struct stat *st) { * of an already-dirty file still produces a new signature. A failed stat * (deleted file, quoting artifact) degrades to the entry text alone — the * deletion itself is represented by the porcelain status. */ -static uint64_t sig_fold_path_stat(uint64_t h, const char *root_path, const char *rel) { +/* `rel` is repository-relative (that is what porcelain prints), so it is joined + * through `cdup` — the hop from root_path up to the repository root — rather than + * onto root_path directly. cdup is "" when the project IS the repository root, + * which reduces this to the original join. */ +static uint64_t sig_fold_path_stat(uint64_t h, const char *root_path, const char *cdup, + const char *rel) { char abs[CBM_SZ_4K]; - snprintf(abs, sizeof(abs), "%s/%s", root_path, rel); + snprintf(abs, sizeof(abs), "%s/%s%s", root_path, cdup ? cdup : "", rel); struct stat st; if (stat(abs, &st) == 0) { int64_t mt = sig_stat_mtime_ns(&st); @@ -538,8 +627,16 @@ static watcher_git_status_t git_dirty_signature(cbm_watcher_t *w, project_state_ return WATCHER_GIT_SUPERVISION_FAILED; } *signature_out = 0; - const char *status_argv[] = {"git", "--no-optional-locks", "-C", state->root_path, - "status", "--porcelain", "-uall", "-z", + /* `-- .` scopes the report to the watched directory. Without it a project + * watched at a sub-package of a monorepo reindexes whenever any SIBLING + * package changes, because git reports the whole repository's dirty state + * regardless of -C. Paths stay repository-relative either way, which is + * what repo_cdup is for. */ + const char *status_argv[] = {"git", "--no-optional-locks", + "-C", state->root_path, + "status", "--porcelain", + "-uall", "-z", + "--", ".", NULL}; watcher_git_output_t output; watcher_git_status_t status = @@ -585,7 +682,7 @@ static watcher_git_status_t git_dirty_signature(cbm_watcher_t *w, project_state_ if (entry[0] == 'R' || entry[0] == 'C') { origin_token = true; } - h = sig_fold_path_stat(h, state->root_path, entry + 3); + h = sig_fold_path_stat(h, state->root_path, state->repo_cdup, entry + 3); } } elen = 0; @@ -638,7 +735,7 @@ static watcher_git_status_t git_dirty_signature(cbm_watcher_t *w, project_state_ h = sig_fold(h, line, len); h = sig_fold(h, "", 1); if (len > 3 && line[2] == ' ') { - h = sig_fold_path_stat(h, state->root_path, line + 3); + h = sig_fold_path_stat(h, state->root_path, state->repo_cdup, line + 3); } } parsed = !ferror(fp) && fclose(fp) == 0; @@ -1060,9 +1157,39 @@ static bool init_baseline(cbm_watcher_t *w, project_state_t *s) { return false; } s->is_git = repository_status == WATCHER_GIT_OK; + + /* `rev-parse --git-dir` walks UP, so an ordinary folder that merely happens + * to live under some unrelated repository answers yes. Treating it as a git + * project is what produced the runaway churn: it inherits the ancestor's + * dirty state, which is permanently non-empty and has nothing to do with + * this directory, so every poll looked like a change. + * + * A directory is only really git-managed here if it carries its own .git, + * or the ancestor repository actually tracks something inside it. A + * genuine sub-package of a monorepo passes the second test; a scratch or + * gitignored folder sitting under a repo fails both and is polled as a + * plain directory instead. */ + if (s->is_git && !git_has_own_dot_git(s->root_path)) { + bool tracked = false; + watcher_git_status_t tracked_status = git_tracks_anything_here(w, s, &tracked); + if (tracked_status != WATCHER_GIT_OK && tracked_status != WATCHER_GIT_COMMAND_FAILED) { + return false; + } + if (!tracked) { + s->is_git = false; + cbm_log_info("watcher.nested_non_git", "project", s->project_name, "path", + s->root_path); + } + } + s->baseline_done = true; if (s->is_git) { + watcher_git_status_t cdup_status = git_repo_cdup(w, s, s->repo_cdup, sizeof(s->repo_cdup)); + if (cdup_status != WATCHER_GIT_OK && cdup_status != WATCHER_GIT_COMMAND_FAILED) { + s->baseline_done = false; + return false; + } watcher_git_status_t head_status = git_head(w, s, s->last_head, sizeof(s->last_head)); if (head_status != WATCHER_GIT_OK && head_status != WATCHER_GIT_COMMAND_FAILED) { s->baseline_done = false; diff --git a/tests/test_watcher.c b/tests/test_watcher.c index cd4144f3e..14b602be1 100644 --- a/tests/test_watcher.c +++ b/tests/test_watcher.c @@ -1226,6 +1226,134 @@ TEST(watcher_detects_git_commit) { PASS(); } +/* A plain directory that merely SITS UNDER an unrelated repository is not a git + * project. `git rev-parse --git-dir` walks up, so it answers yes for such a + * folder, and the watcher then inherited the ancestor's dirty state — which is + * permanently non-empty and has nothing to do with this directory — and + * reindexed on every single poll forever (#841/#937: reporters measured this in + * hundreds of GB of writes per day). */ +TEST(watcher_nested_non_git_dir_does_not_inherit_ancestor_dirt) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_nested_XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + if (wt_git(tmpdir, "init -q") != 0) { + th_rmtree(tmpdir); + FAIL("git init failed"); + } + { + char p[300]; + th_write_file(wt_path(p, sizeof(p), tmpdir, "tracked.txt"), "hello\n"); + } + wt_git(tmpdir, "add tracked.txt"); + wt_git(tmpdir, "commit -q -m init"); + + /* Leave the ancestor permanently dirty — this is the condition that used to + * retrigger indexing on every poll of the nested directory. */ + { + char p[300]; + th_write_file(wt_path(p, sizeof(p), tmpdir, "dirty.txt"), "uncommitted\n"); + } + + /* A scratch directory inside it, tracked by nothing. */ + char nested[400]; + snprintf(nested, sizeof(nested), "%s/scratch", tmpdir); + if (!cbm_mkdir_p(nested, 0755)) { + th_rmtree(tmpdir); + FAIL("mkdir nested failed"); + } + { + char p[500]; + th_write_file(wt_path(p, sizeof(p), nested, "notes.md"), "scratch\n"); + } + + cbm_store_t *store = cbm_store_open_memory(); + cbm_watcher_t *w = cbm_watcher_new(store, index_callback, NULL); + cbm_watcher_watch(w, "nested-scratch", nested); + index_call_count = 0; + + cbm_watcher_poll_once(w); /* baseline */ + int after_baseline = index_call_count; + + /* Three polls with the ancestor still dirty and the nested dir untouched. + * Under the old classification each of these reindexed. */ + for (int i = 0; i < 3; i++) { + cbm_watcher_touch(w, "nested-scratch"); + cbm_watcher_poll_once(w); + } + ASSERT_EQ(index_call_count, after_baseline); + + cbm_watcher_free(w); + cbm_store_close(store); + th_rmtree(tmpdir); + PASS(); +} + +/* A genuine sub-package of a monorepo IS git-managed, and must still be watched + * as such — the nested-directory guard above must not disqualify it. It also + * must not react to a sibling package's changes: `git status` reports the whole + * repository regardless of -C, so without a `-- .` pathspec every package in a + * monorepo reindexes whenever any other one is edited. */ +TEST(watcher_monorepo_subdir_ignores_sibling_changes) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_mono_XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + if (wt_git(tmpdir, "init -q") != 0) { + th_rmtree(tmpdir); + FAIL("git init failed"); + } + char pkg_a[400]; + char pkg_b[400]; + snprintf(pkg_a, sizeof(pkg_a), "%s/pkg-a", tmpdir); + snprintf(pkg_b, sizeof(pkg_b), "%s/pkg-b", tmpdir); + if (!cbm_mkdir_p(pkg_a, 0755) || !cbm_mkdir_p(pkg_b, 0755)) { + th_rmtree(tmpdir); + FAIL("mkdir packages failed"); + } + { + char p[500]; + th_write_file(wt_path(p, sizeof(p), pkg_a, "a.txt"), "a\n"); + th_write_file(wt_path(p, sizeof(p), pkg_b, "b.txt"), "b\n"); + } + wt_git(tmpdir, "add -A"); + wt_git(tmpdir, "commit -q -m init"); + + cbm_store_t *store = cbm_store_open_memory(); + cbm_watcher_t *w = cbm_watcher_new(store, index_callback, NULL); + cbm_watcher_watch(w, "pkg-a", pkg_a); + index_call_count = 0; + + cbm_watcher_poll_once(w); /* baseline */ + int after_baseline = index_call_count; + + /* Edit the SIBLING package only. pkg-a is untouched. */ + { + char p[500]; + th_append_file(wt_path(p, sizeof(p), pkg_b, "b.txt"), "sibling edit\n"); + } + cbm_watcher_touch(w, "pkg-a"); + cbm_watcher_poll_once(w); + ASSERT_EQ(index_call_count, after_baseline); + + /* Editing pkg-a itself must still be seen — the scoping must not have + * silenced real changes. */ + { + char p[500]; + th_append_file(wt_path(p, sizeof(p), pkg_a, "a.txt"), "own edit\n"); + } + cbm_watcher_touch(w, "pkg-a"); + cbm_watcher_poll_once(w); + ASSERT_EQ(index_call_count, after_baseline + 1); + + cbm_watcher_free(w); + cbm_store_close(store); + th_rmtree(tmpdir); + PASS(); +} + /* SHA-256 repositories emit a 64-hex-character HEAD. The watcher must retain * the complete object ID (plus line terminator/NUL while reading it), otherwise * baseline initialization silently retries forever and auto-refresh never runs. */ @@ -3041,6 +3169,8 @@ SUITE(watcher) { /* Git change detection */ RUN_TEST(watcher_detects_git_commit); + RUN_TEST(watcher_nested_non_git_dir_does_not_inherit_ancestor_dirt); + RUN_TEST(watcher_monorepo_subdir_ignores_sibling_changes); RUN_TEST(watcher_detects_sha256_git_commit); RUN_TEST(watcher_detects_dirty_worktree); RUN_TEST(watcher_identical_watch_preserves_dirty_baseline);