Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix negative ignore rules with patterns #4296

Merged
merged 5 commits into from
Aug 25, 2017

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Jul 7, 2017

Ignore rules allow for reverting a previously ignored rule by prefixing
it with an exclamation mark. As such, a negative rule can only override
previously ignored files. While computing all ignore patterns, we try to
use this fact to optimize away some negative rules which do not override
any previous patterns, as they won't change the outcome anyway.

In some cases, though, this optimization causes us to get the actual
ignores wrong for some files. This may happen whenever the pattern
contains a wildcard, as we are unable to reason about whether a pattern
overrides a previous pattern in a sane way. This happens for example in
the case where a gitignore file contains ".c" and "!src/.c", where we
wouldn't un-ignore files inside of the "src/" subdirectory.

In this case, the first solution coming to mind may be to just strip the
"src/" prefix and simply compare the basenames. While that would work
here, it would stop working as soon as the basename pattern itself is
different, like for example with "x.c" and "!src/.c. As such, we
settle for the easier fix of just not optimizing away rules that contain
a wildcard.


This should fix #4295. Edit: Oops. I mean #3224

@pks-t
Copy link
Member Author

pks-t commented Jul 7, 2017

I've added the test from #3224 by @carlosmn

@pks-t
Copy link
Member Author

pks-t commented Jul 10, 2017

I've added another commit to fix an issue with case-insensitive negative ignores, reported in #4295.

Going forward, I'll most likely remove our optimization which drops negative ignores whenever they do not undo a previous pattern. This optimization has caused too much grief by now and is probably not even worth it. I'll leave this for a future PR, though.

@pks-t
Copy link
Member Author

pks-t commented Aug 25, 2017

Rebased to re-trigger CI and spark some interest ;)

pks-t added 5 commits August 25, 2017 18:00
Ignore rules allow for reverting a previously ignored rule by prefixing
it with an exclamation mark. As such, a negative rule can only override
previously ignored files. While computing all ignore patterns, we try to
use this fact to optimize away some negative rules which do not override
any previous patterns, as they won't change the outcome anyway.

In some cases, though, this optimization causes us to get the actual
ignores wrong for some files. This may happen whenever the pattern
contains a wildcard, as we are unable to reason about whether a pattern
overrides a previous pattern in a sane way. This happens for example in
the case where a gitignore file contains "*.c" and "!src/*.c", where we
wouldn't un-ignore files inside of the "src/" subdirectory.

In this case, the first solution coming to mind may be to just strip the
"src/" prefix and simply compare the basenames. While that would work
here, it would stop working as soon as the basename pattern itself is
different, like for example with "*x.c" and "!src/*.c. As such, we
settle for the easier fix of just not optimizing away rules that contain
a wildcard.
When computing negative ignores, we throw away any rule which does not
undo a previous rule to optimize. But on case insensitive file systems,
we need to keep in mind that a negative ignore can also undo a previous
rule with different case, which we did not yet honor while determining
whether a rule undoes a previous one. So in the following example, we
fail to unignore the "/Case" directory:

    /case
    !/Case

Make both paths checking whether a plain- or wildcard-based rule undo a
previous rule aware of case-insensitivity. This fixes the described
issue.
@pks-t pks-t force-pushed the pks/pattern-based-gitignore branch from 501b96e to 2d9ff8f Compare August 25, 2017 16:00
@ethomson
Copy link
Member

This optimization has caused too much grief by now and is probably not even worth it.

Agreed. Merging this now, but I think this is probably the right long-term mission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Another .gitignore issue versus standard Git
2 participants