Skip to content

After #132: one regression (.* + !**/node_modules/**) and five dir/** directory queries #136

Description

@KaizenShogun

Following up on your ask in #132. I re-ran the bench against master at 5f14318, not the PR branch I measured on 3 Sep — four commits landed after the merge, and a number about a branch isn't a number about what ships.

Setup, briefly: 9,852 (.gitignore, path) pairs harvested from 43 real repositories, git check-ignore as the oracle, one subprocess per pathspec tree. The control is master at 93e0179, the first parent of the merge commit, so the comparison isolates #132 and nothing else.

backend 93e0179 (pre) 5f14318 (post)
simple 69 6
re2 65 6
hyperscan 65 6

By sets rather than counts: 64 fixed, 1 new, 5 pre-existing on simple (60 / 1 / 5 on re2 and hyperscan, whose pre-merge sets were already smaller). The six survivors are the same six paths in all three backends.

1. The regression

Two lines, one path:

GitIgnoreSpec.from_lines([".*", "!**/node_modules/**"]).match_file(
    "vendor/deps/npm/node_modules/.bin/x.txt")
# 5f14318: True    93e0179: False    git: not ignored

That's after greedy reduction from nodejs/node's 184-line .gitignore. And it isn't an artefact of check-ignore --no-index: with the file actually on disk in a real repo, git status --untracked-files=all lists it, match_tree_files(repo, negate=True) returns it on 93e0179, and returns nothing on 5f14318. So this one reaches the tree-walking API too.

The shape is #129 in reverse. .* excludes the ancestor .bin, but the later !**/node_modules/** re-includes the ancestor as well as the file. The ancestor accumulator records the exclusion of .bin and no later negation overturns it — the rule being that reinclusion of an ancestor has to be allowed to undo an earlier exclusion of that same ancestor, not only of the file. All three backends.

2. The five that were already there

Every one reduces to a single pattern line, every one over-ignores, all three backends:

repo minimal pattern query
kubernetes .settings/** .settings/
supabase **/.superset/** .superset/
tensorflow /tensorflow/lite/gen/** tensorflow/lite/gen/
tensorflow /tensorflow/lite/tools/make/downloads/** tensorflow/lite/tools/make/downloads/
tensorflow /tensorflow/lite/tools/make/gen/** tensorflow/lite/tools/make/gen/

match_file("X/") returns True for a pattern X/**. I want to be careful about the oracle here, because the obvious one is worthless: git check-ignore -v '.settings/' also reports it ignored and names .settings/** as the matching pattern — the trailing slash lets ** match the empty string, so that question answers itself.

The test that does mean something is whether git treats the directory as excluded, i.e. whether a negation inside it still works. Paired, with real files on disk:

.gitignore:  .settings/**            |  .gitignore:  .settings/
             !.settings/keep.txt     |               !.settings/keep.txt

git status -uall:  .settings/keep.txt|  git status -uall:  (nothing)

pathspec match_file(".settings/")         ->  True   |  True
pathspec match_file(".settings/keep.txt") ->  False  |  True   (both correct)
match_tree_files(negate=True)  -> ['.settings/keep.txt'] | []  (both correct)

dir/ and dir/** differ for git in exactly one respect — whether the directory itself is excluded — and match_file on the directory can't tell them apart.

Worth saying plainly, since it changes what the fix should be: match_tree_files gets all five right. They only bite callers that run their own os.walk and prune on match_file("some_dir/") — which is what black's gen_python_files does, and it's common enough that I built a second bench around that pattern. So these five may be a documentation matter (say that directory queries belong to the tree API) rather than a code one; your call. The regression in §1 I'd separate from that, since it hits match_tree_files too and it's new.

Bench, corpus and the reduction scripts are at https://github.com/KaizenShogun/gitignore-conformance. Happy to run any candidate fix through it, same as with #132.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions