Skip to content

Commit 282fbfc

Browse files
DeusDataAmirF194
andcommitted
fix(discover): match an enclosing repository's ignore rules relative to their own directory
The previous commit found the enclosing repository when a git-less subfolder is indexed, but merged its patterns into the matcher whose rel_path is relative to repo_path. gitignore.c matches a rooted pattern with `glob_match_bounded(p->pattern, rel_path)`, so every anchored pattern of the enclosing repository was silently re-anchored onto the indexed subfolder. That is wrong in both directions, and both were checked against real `git check-ignore` run inside the subfolder: * enclosing root .gitignore `/secret.py`, indexing pkg/ git INDEXES pkg/secret.py — we HID it. A rooted pattern means "in the repository root", and re-anchored it became "in pkg". Silent index loss: the file is gone from the graph with no diagnostic, the worst failure mode discovery has. * enclosing .git/info/exclude `pkg/scratch/`, indexing pkg/ git IGNORES pkg/scratch/tmp.py — we INDEXED it. The pattern is anchored THROUGH pkg, so once rel_path lost the "pkg/" prefix it stopped matching and an excluded directory got walked. Only the enclosing root's .gitignore was consulted, too; git reads the .gitignore of every directory between the repository root and the indexed one, deeper overriding shallower. Mechanism. discover.c already had the chain that encodes "the deepest file with an opinion wins": gitignore_link_t carries a `prefix`, the walk-relative directory a nested matcher came from, which local_rel_path() strips off. An ancestor is the mirror image of that, so the chain is extended rather than duplicated: each link now also carries a `base`, the walk root's path relative to the directory the matcher came from, which gitignore_chain_result PREPENDS instead of stripping (discover.c:569, 610). Exactly one of the two offsets is ever non-empty, and both live in the link's flexible tail, so no allocation site is added. * ancestor_ignores_build() (discover.c:1282) loads the .gitignore of every directory from the enclosing repository root down to — but not including — the indexed one, shallow to deep, and chains them so the deepest wins, negations included. The enclosing repository's info/exclude is merged into the ROOT ancestor's matcher, which is where git anchors it, keeping the precedence info/exclude already has over .gitignore at the same level. * the indexed directory's own .gitignore stays its own matcher at the leaf of that chain (discover.c:1480), so it still overrides every ancestor. Nothing is merged into it any more, which also removes the earlier merge's failure mode of dropping the local patterns and keeping the ancestor's. * repo_path is canonicalized with cbm_canonical_path() before the lexical parent walk (discover.c:1414), so `cbm index pkg` and `cbm index /abs/path/pkg` discover the same files; pipeline.c strdups the caller's path unchanged. * walk_owned_gitignore_free() (discover.c:1019) is factored out of walk_dir and shared with the ancestor chain, which owns its matchers the same way. Proof on real input, before the suites. Indexing scripts/ of this checkout — a git-less subfolder whose root .gitignore carries the rooted patterns `/memlab-*` and `/soak*/`: before 87 files memlab-drive.py MISSING memlab-report.py MISSING after 89 files memlab-drive.py present memlab-report.py present `git check-ignore` inside scripts/ indexes both, so the two recovered files are exactly the ones git keeps. No file is lost in the other direction. RED on the previous commit, four new tests, each direction pinned to a `git check-ignore` verdict on an identical fixture: test_discover.c:1471: count == 1, expected 2 == 2 rooted_pattern_not_reanchored test_discover.c:1498: count == 2, expected 1 == 1 info_exclude_rooted_subpath test_discover.c:1526: count == 2, expected 1 == 1 intermediate_gitignore test_discover.c:1555: count == 0, expected 1 == 1 deeper_ancestor_negation_wins discover: 120 passed, 4 failed The three tests already on the branch use unanchored patterns, which match at any depth and so cannot see this bug; they pass before and after. GREEN: discover 124 passed, 0 failed; discover + gitignore + pipeline + git_context 425 passed, 0 failed. Revert-check: reverting only the discover.c hunks and rebuilding puts the four new tests RED again on the same four lines with the same counts (120 passed, 4 failed); restoring returns 425 passed. Memory-core linter: none grew (31 raw sites in discover.c before and after). clang-format clean. Co-authored-by: Amir Fathi <amirfathi.me@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent c8184ed commit 282fbfc

2 files changed

Lines changed: 309 additions & 43 deletions

File tree

‎src/discover/discover.c‎

Lines changed: 186 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -554,26 +554,40 @@ static const char *local_rel_path(const char *rel_path, const char *local_prefix
554554
* matcher borrows a pointer to the deepest link governing it, so a directory
555555
* without a .gitignore of its own simply shares its parent's link. Links live
556556
* on the heap (the frame stack is realloc'd and popped) and walk_dir owns them
557-
* through the `owned_next` list. `prefix` is the walk-relative directory the
558-
* matcher was loaded from ("" for the root). */
557+
* through the `owned_next` list.
558+
*
559+
* Every pattern is anchored to the directory its file came from, so each link
560+
* carries the offset between that directory and the walk-relative rel_path it
561+
* is asked about. The two offsets are mirror images and exactly one is ever
562+
* non-empty:
563+
* `prefix` — the matcher's directory is at or BELOW the walk root
564+
* ("" for the root itself): strip it from rel_path.
565+
* `base` — the matcher's directory is ABOVE the walk root (an enclosing
566+
* repository's ignore files, when a git-less subfolder is
567+
* indexed): prepend the walk root's path relative to it.
568+
* Both strings live in the flexible tail, `base` right after `prefix`. */
559569
typedef struct gitignore_link {
560570
const cbm_gitignore_t *gi;
561571
const struct gitignore_link *parent;
562572
struct gitignore_link *owned_next;
573+
const char *base;
563574
char prefix[];
564575
} gitignore_link_t;
565576

566577
static gitignore_link_t *gitignore_link_new(const cbm_gitignore_t *gi, const char *prefix,
567-
const gitignore_link_t *parent,
578+
const char *base, const gitignore_link_t *parent,
568579
gitignore_link_t **owned_links) {
569580
size_t prefix_size = strlen(prefix) + SKIP_ONE;
570-
gitignore_link_t *link = malloc(sizeof(*link) + prefix_size);
581+
size_t base_size = strlen(base) + SKIP_ONE;
582+
gitignore_link_t *link = malloc(sizeof(*link) + prefix_size + base_size);
571583
if (!link) {
572584
return NULL;
573585
}
574586
link->gi = gi;
575587
link->parent = parent;
576588
memcpy(link->prefix, prefix, prefix_size);
589+
memcpy(link->prefix + prefix_size, base, base_size);
590+
link->base = link->prefix + prefix_size;
577591
link->owned_next = *owned_links;
578592
*owned_links = link;
579593
return link;
@@ -594,9 +608,20 @@ static void gitignore_links_free(gitignore_link_t *link) {
594608
* re-included by a negation, 0 when no file mentions the path. Cost is one
595609
* match per .gitignore on the path — O(depth), never a rescan. */
596610
static int gitignore_chain_result(const gitignore_link_t *link, const char *rel_path, bool is_dir) {
611+
char based[CBM_SZ_4K];
597612
for (; link; link = link->parent) {
598-
int verdict =
599-
cbm_gitignore_match_result(link->gi, local_rel_path(rel_path, link->prefix), is_dir);
613+
const char *probe = local_rel_path(rel_path, link->prefix);
614+
if (link->base[0]) {
615+
/* Matcher from a directory above the walk root: ask it about the
616+
* path it would see, i.e. the one its own rooted patterns are
617+
* anchored against. */
618+
int written = snprintf(based, sizeof(based), "%s/%s", link->base, probe);
619+
if (written < 0 || (size_t)written >= sizeof(based)) {
620+
continue; /* longer than this matcher can address: no opinion */
621+
}
622+
probe = based;
623+
}
624+
int verdict = cbm_gitignore_match_result(link->gi, probe, is_dir);
600625
if (verdict != 0) {
601626
return verdict;
602627
}
@@ -988,9 +1013,20 @@ static bool walk_owned_gitignore_append(cbm_gitignore_t ***owned, size_t *count,
9881013
return true;
9891014
}
9901015

1016+
/* Release an owner array built by walk_owned_gitignore_append(). Shared by the
1017+
* walk and by the enclosing-repository chain, which own their matchers the
1018+
* same way. */
1019+
static void walk_owned_gitignore_free(cbm_gitignore_t **owned, size_t count) {
1020+
for (size_t i = 0; i < count; i++) {
1021+
cbm_gitignore_free(owned[i]);
1022+
}
1023+
free(owned);
1024+
}
1025+
9911026
static void walk_dir(const char *dir_path, const char *rel_prefix, const cbm_discover_opts_t *opts,
992-
const cbm_gitignore_t *gitignore, const cbm_gitignore_t *global_gi,
993-
const cbm_gitignore_t *cbmignore, file_list_t *out) {
1027+
const cbm_gitignore_t *gitignore, const gitignore_link_t *ancestors,
1028+
const cbm_gitignore_t *global_gi, const cbm_gitignore_t *cbmignore,
1029+
file_list_t *out) {
9941030
walk_stack_t ws = {
9951031
.frames = calloc(WALK_STACK_CAP, sizeof(walk_frame_t)), .top = 0, .cap = WALK_STACK_CAP};
9961032
if (!ws.frames) {
@@ -1012,8 +1048,12 @@ static void walk_dir(const char *dir_path, const char *rel_prefix, const cbm_dis
10121048
free(ws.frames);
10131049
return;
10141050
}
1051+
/* `ancestors` is the deepest link of the enclosing repository's chain (or
1052+
* NULL); the walk root's own matcher sits below it so it still wins. */
1053+
ws.frames[0].ignore_chain = ancestors;
10151054
if (gitignore) {
1016-
ws.frames[0].ignore_chain = gitignore_link_new(gitignore, rel_prefix, NULL, &owned_links);
1055+
ws.frames[0].ignore_chain =
1056+
gitignore_link_new(gitignore, rel_prefix, "", ancestors, &owned_links);
10171057
if (!ws.frames[0].ignore_chain) {
10181058
out->failed = true;
10191059
free(ws.frames);
@@ -1034,7 +1074,7 @@ static void walk_dir(const char *dir_path, const char *rel_prefix, const cbm_dis
10341074
}
10351075
/* owned_gis owns `loaded` from here on, even if the link fails. */
10361076
const gitignore_link_t *link =
1037-
gitignore_link_new(loaded, frame.prefix, frame.ignore_chain, &owned_links);
1077+
gitignore_link_new(loaded, frame.prefix, "", frame.ignore_chain, &owned_links);
10381078
if (!link) {
10391079
out->failed = true;
10401080
break;
@@ -1056,10 +1096,7 @@ static void walk_dir(const char *dir_path, const char *rel_prefix, const cbm_dis
10561096
}
10571097
cbm_closedir(d);
10581098
}
1059-
for (size_t i = 0; i < owned_count; i++) {
1060-
cbm_gitignore_free(owned_gis[i]);
1061-
}
1062-
free(owned_gis);
1099+
walk_owned_gitignore_free(owned_gis, owned_count);
10631100
gitignore_links_free(owned_links);
10641101
free(ws.frames);
10651102
}
@@ -1199,6 +1236,102 @@ static bool resolve_enclosing_git_root(const char *repo_path, char *ancestor_roo
11991236
}
12001237
}
12011238

1239+
/* The enclosing repository's ignore files, kept OUT of the indexed directory's
1240+
* own matcher. Merging them in would re-anchor every rooted pattern onto the
1241+
* indexed subfolder: "/secret.py" at the enclosing root would start hiding
1242+
* <subfolder>/secret.py (silent index loss), and "pkg/scratch/" would stop
1243+
* hiding pkg/scratch (a directory git excludes would be walked). Each matcher
1244+
* instead keeps its own directory as its anchor, via the link's `base`. */
1245+
typedef struct {
1246+
cbm_gitignore_t **owned;
1247+
size_t count;
1248+
size_t capacity;
1249+
const gitignore_link_t *chain; /* deepest ancestor link, or NULL */
1250+
gitignore_link_t *links; /* owner list backing `chain` */
1251+
} ancestor_ignores_t;
1252+
1253+
static void ancestor_ignores_free(ancestor_ignores_t *anc) {
1254+
walk_owned_gitignore_free(anc->owned, anc->count);
1255+
gitignore_links_free(anc->links);
1256+
}
1257+
1258+
/* Add one ancestor directory's matcher to the chain, deepest last so that the
1259+
* deepest file with an opinion wins (git's shallow-to-deep rule, negations
1260+
* included). Takes ownership of `gi`. */
1261+
static bool ancestor_ignores_push(ancestor_ignores_t *anc, cbm_gitignore_t *gi, const char *base) {
1262+
if (!walk_owned_gitignore_append(&anc->owned, &anc->count, &anc->capacity, gi)) {
1263+
cbm_gitignore_free(gi);
1264+
return false;
1265+
}
1266+
/* owned[] owns `gi` from here on, even if the link fails. */
1267+
const gitignore_link_t *link = gitignore_link_new(gi, "", base, anc->chain, &anc->links);
1268+
if (!link) {
1269+
return false;
1270+
}
1271+
anc->chain = link;
1272+
return true;
1273+
}
1274+
1275+
/* Load the .gitignore of EVERY directory from the enclosing repository root
1276+
* (inclusive) down to the indexed directory (exclusive) — git consults all of
1277+
* them, not just the root's. `root_extra` is that repository's
1278+
* <common>/info/exclude, which git anchors at its root like the root
1279+
* .gitignore; ownership is taken. `leaf` must be a canonical path under
1280+
* `root`, both with '/' separators. Returns false only on allocation
1281+
* failure. */
1282+
static bool ancestor_ignores_build(ancestor_ignores_t *anc, const char *root, const char *leaf,
1283+
cbm_gitignore_t *root_extra) {
1284+
size_t root_len = strlen(root);
1285+
if (strncmp(leaf, root, root_len) != 0) {
1286+
cbm_gitignore_free(root_extra);
1287+
return true;
1288+
}
1289+
const char *base = leaf + root_len;
1290+
while (*base == '/') {
1291+
base++;
1292+
}
1293+
1294+
char dir[CBM_SZ_4K];
1295+
int dir_len = snprintf(dir, sizeof(dir), "%s", root);
1296+
if (dir_len < 0 || (size_t)dir_len >= sizeof(dir) || !*base) {
1297+
cbm_gitignore_free(root_extra);
1298+
return dir_len >= 0 && (size_t)dir_len < sizeof(dir);
1299+
}
1300+
1301+
for (;;) {
1302+
char gi_path[CBM_SZ_4K];
1303+
path_join(gi_path, sizeof(gi_path), dir, ".gitignore");
1304+
cbm_gitignore_t *gi = cbm_gitignore_load(gi_path);
1305+
if (root_extra) {
1306+
/* First iteration: `dir` is the enclosing repository root, the one
1307+
* directory info/exclude shares an anchor with. Merged after the
1308+
* .gitignore patterns so it overrides them, the same precedence
1309+
* the two already have for an ordinary repository root. */
1310+
if (!gi) {
1311+
gi = root_extra;
1312+
} else {
1313+
(void)cbm_gitignore_merge(gi, root_extra);
1314+
cbm_gitignore_free(root_extra);
1315+
}
1316+
root_extra = NULL;
1317+
}
1318+
if (gi && !ancestor_ignores_push(anc, gi, base)) {
1319+
return false;
1320+
}
1321+
const char *slash = strchr(base, '/');
1322+
if (!slash) {
1323+
return true; /* the next directory down IS the indexed one */
1324+
}
1325+
int written = snprintf(dir + dir_len, sizeof(dir) - (size_t)dir_len, "/%.*s",
1326+
(int)(slash - base), base);
1327+
if (written < 0 || (size_t)written >= sizeof(dir) - (size_t)dir_len) {
1328+
return true; /* deeper directories are unaddressable: stop here */
1329+
}
1330+
dir_len += written;
1331+
base = slash + SKIP_ONE;
1332+
}
1333+
}
1334+
12021335
int cbm_discover(const char *repo_path, const cbm_discover_opts_t *opts, cbm_file_info_t **out,
12031336
int *count) {
12041337
return cbm_discover_ex(repo_path, opts, out, count, NULL, NULL);
@@ -1246,15 +1379,18 @@ static cbm_discover_status_t discover_impl(const char *repo_path, const cbm_disc
12461379

12471380
/* Load gitignore sources for ordinary repos AND linked worktrees.
12481381
* Sources merged in order (later patterns win on conflict):
1249-
* 1. <enclosing>/.gitignore: enclosing repo's root exclusions, only when
1250-
* repo_path itself has no .git of its own (see below)
1251-
* 2. <repo>/.gitignore: committed exclusions
1252-
* 3. <common>/info/exclude: per-clone exclusions, not committed
1382+
* 1. <repo>/.gitignore — committed exclusions
1383+
* 2. <common>/info/exclude — per-clone exclusions, not committed
12531384
* <common> is the git common dir, resolved via resolve_git_common_dir() so a
12541385
* worktree (where .git is a gitlink file) reads the shared info/exclude/config
1255-
* just like a normal checkout. All are folded into a single matcher so all
1386+
* just like a normal checkout. Both are folded into a single matcher so all
12561387
* downstream call paths remain unchanged. Fixes issue #489: OOM on repos whose
1257-
* worktrees are excluded only via .git/info/exclude (e.g. Sandcastle). */
1388+
* worktrees are excluded only via .git/info/exclude (e.g. Sandcastle).
1389+
*
1390+
* An ENCLOSING repository's ignore files (indexing a git-less subfolder,
1391+
* issue #510) are deliberately NOT folded in here: their patterns are
1392+
* anchored to their own directories, so they stay separate matchers on the
1393+
* chain with a `base` offset. See ancestor_ignores_build(). */
12581394
cbm_gitignore_t *gitignore = NULL;
12591395
char gi_path[CBM_SZ_4K];
12601396
struct stat gi_stat;
@@ -1265,41 +1401,47 @@ static cbm_discover_status_t discover_impl(const char *repo_path, const cbm_disc
12651401
bool is_git_repo = resolve_git_common_dir(repo_path, git_common_dir, sizeof(git_common_dir));
12661402
bool has_git_config = false;
12671403
/* repo_path itself is not a git repo root: walk upward for an enclosing one,
1268-
* exactly as `git` does when run from a subfolder. Its root .gitignore is
1269-
* loaded FIRST below (least specific), so the indexed directory's own
1270-
* .gitignore and info/exclude (both more specific) still override it on
1271-
* conflict. Fixes the remaining half of issue #510: only the indexed
1272-
* directory's own .gitignore was ever consulted, never an enclosing repo's. */
1404+
* exactly as `git` does when run from a subfolder. Fixes the remaining half
1405+
* of issue #510: only the indexed directory's own .gitignore was ever
1406+
* consulted, never an enclosing repo's. The walk is lexical, so repo_path is
1407+
* canonicalized first — otherwise `cbm index pkg` and `cbm index /abs/pkg`
1408+
* would discover different files from the same directory. */
1409+
ancestor_ignores_t ancestors = {0};
12731410
char enclosing_root[CBM_SZ_4K];
1274-
if (!is_git_repo &&
1275-
resolve_enclosing_git_root(repo_path, enclosing_root, sizeof(enclosing_root),
1276-
git_common_dir, sizeof(git_common_dir))) {
1277-
is_git_repo = true;
1278-
char enclosing_gi_path[CBM_SZ_4K];
1279-
path_join(enclosing_gi_path, sizeof(enclosing_gi_path), enclosing_root, ".gitignore");
1280-
gitignore = cbm_gitignore_load(enclosing_gi_path);
1411+
char canonical_repo[CBM_SZ_4K];
1412+
bool has_enclosing_repo = false;
1413+
if (!is_git_repo) {
1414+
if (!cbm_canonical_path(repo_path, canonical_repo, sizeof(canonical_repo))) {
1415+
snprintf(canonical_repo, sizeof(canonical_repo), "%s", repo_path);
1416+
}
1417+
cbm_normalize_path_sep(canonical_repo);
1418+
has_enclosing_repo =
1419+
resolve_enclosing_git_root(canonical_repo, enclosing_root, sizeof(enclosing_root),
1420+
git_common_dir, sizeof(git_common_dir));
1421+
is_git_repo = has_enclosing_repo;
12811422
}
12821423
/* Always honour the .gitignore at the indexed-directory root, even when the
12831424
* directory is not a git repo root (e.g. indexing a sub-package directly).
12841425
* Fixes issue #510: a root .gitignore was silently ignored without .git/. */
12851426
snprintf(gi_path, sizeof(gi_path), "%s/.gitignore", repo_path);
1286-
cbm_gitignore_t *local_gitignore = cbm_gitignore_load(gi_path);
1287-
if (local_gitignore) {
1288-
if (!gitignore) {
1289-
gitignore = local_gitignore;
1290-
} else {
1291-
(void)cbm_gitignore_merge(gitignore, local_gitignore);
1292-
cbm_gitignore_free(local_gitignore);
1293-
}
1294-
}
1427+
gitignore = cbm_gitignore_load(gi_path);
12951428
if (is_git_repo) {
12961429
path_join(gi_path, sizeof(gi_path), git_common_dir, "config");
12971430
has_git_config = wide_stat(gi_path, &gi_stat) == 0 && S_ISREG(gi_stat.st_mode);
12981431

12991432
char exc_path[CBM_SZ_4K];
13001433
path_join(exc_path, sizeof(exc_path), git_common_dir, "info/exclude");
13011434
cbm_gitignore_t *git_exclude = cbm_gitignore_load(exc_path);
1302-
if (git_exclude) {
1435+
if (has_enclosing_repo) {
1436+
/* git_common_dir belongs to the ENCLOSING repository, so its
1437+
* info/exclude is anchored at that repository's root, not at the
1438+
* indexed subfolder. Ownership passes to the ancestor chain. */
1439+
if (!ancestor_ignores_build(&ancestors, enclosing_root, canonical_repo, git_exclude)) {
1440+
ancestor_ignores_free(&ancestors);
1441+
cbm_gitignore_free(gitignore);
1442+
return CBM_DISCOVER_ERROR;
1443+
}
1444+
} else if (git_exclude) {
13031445
if (!gitignore) {
13041446
gitignore = git_exclude;
13051447
} else {
@@ -1335,9 +1477,10 @@ static cbm_discover_status_t discover_impl(const char *repo_path, const cbm_disc
13351477
.collect_ignored = !count_only && ignored_out != NULL,
13361478
};
13371479
walk_cache_dir_snapshot();
1338-
walk_dir(repo_path, "", opts, gitignore, global_gi, cbmignore, &fl);
1480+
walk_dir(repo_path, "", opts, gitignore, ancestors.chain, global_gi, cbmignore, &fl);
13391481

13401482
/* Cleanup */
1483+
ancestor_ignores_free(&ancestors);
13411484
cbm_gitignore_free(gitignore);
13421485
cbm_gitignore_free(global_gi);
13431486
cbm_gitignore_free(cbmignore);

0 commit comments

Comments
 (0)