8000 [Finder] GitIgnore creates incorrect regex when ignoring 'everything in subdirectory but..' · Issue #37424 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Finder] GitIgnore creates incorrect regex when ignoring 'everything in subdirectory but..' #37424

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.

Sign up for GitHub

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

Jump to bottom
Closed
Jeroeny opened this issue Jun 26, 2020 · 4 comments
Closed

[Finder] GitIgnore creates incorrect regex when ignoring 'everything in subdirectory but..' #37424

Jeroeny opened this issue Jun 26, 2020 · 4 comments
Labels
Bug Finder Status: Needs Review

Comments

@Jeroeny
Copy link
Contributor
Jeroeny commented Jun 26, 2020

Symfony version(s) affected: >=4.3

Description

When you want to ignore everything in a certain directory, except some specific files,
these are the .gitignore rules that can be used:

/example/**
!/example/example.txt
!/example/packages/
!/example/packages/example.yaml

You might think /example/packages/ will be unignored by this, but that is not the case according to the .gitignore standard.
With this setup, any files in /example/packages/ besides /example/packages/example.yaml will be ignored. But the Gitignore::toRegex does not match this logic properly.

See also: https://git-scm.com/docs/gitignore and https://stackoverflow.com/a/61556103/3265437

How to reproduce
https://github.com/Jeroeny/reproduce/tree/gitign

Possible Solution
The regex building logic would have to be adjusted. I've looked into this but wasn't able to fix it yet.

Additional context
See the reproducer.
@Jeroeny Jeroeny added the Bug label Jun 26, 2020
@xabbuh xabbuh added the Finder label Jun 26, 2020
@Jeroeny
Copy link
Contributor Author
Jeroeny commented Jun 26, 2020

Added a possible fix to the reproducer: https://github.com/Jeroeny/reproduce/blob/gitign/Gitignore.php

@stof
Copy link
Member
stof commented Jun 26, 2020

I think the fix is not right either, because the way gitignore works is not that all negative rules (un-ignoring files again) are matched after positive rules (ignoring files). AFAIK, the order of rules is relevant.

@Jeroeny
Copy link
Contributor Author
Jeroeny commented Jun 26, 2020 •
edited
Loading

I think the fix is not right either, because the way gitignore works is not that all negative rules (un-ignoring files again) are matched after positive rules (ignoring files). AFAIK, the order of rules is relevant.

Indeed, the order is very important for the final result. I'm still testing the possible solution a bit, but haven't found a case yet where it's incorrect.

Edit: Now I have (incorrect in both versions):

!var/x.txt
var/x.txt
!/x.txt
/x.txt

git makes that ignore both x.txt and var/x.txt. Gitignore::regex says they are both not ignored. :(
Maybe when iterating over the lines, previous lines should be removed when a new line "matches" a previous line. Although I'm not sure what qualifies as "match" yet.

@Jeroeny
Copy link
Contributor Author
Jeroeny commented Jun 27, 2020

I had to change the way the regex is built up, to take lines' order into consideration. I made a new class in the reproducer that correctly matches files when they are re-ignored (or re-unignored) on lines after a previous line that (un-)ignored it: https://github.com/Jeroeny/reproduce/blob/gitign/GitignoreNew.php

The regex per positive .gitignore line is a group followed by a negative lookbehind with all the negations that were on lines áfter that line.

It seems to work so far.

@Jeroeny Jeroeny mentioned this issue Jul 11, 2020
Merged
@fabpot fabpot closed this as completed Jul 31, 2020
fabpot added a commit that referenced this issue Jul 31, 2020
@fabpot
bug #37560 [Finder] Fix GitIgnore parser when dealing with (sub)direc…
bea4319 
…tories and take order of lines into account (Jeroeny)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Finder] Fix GitIgnore parser when dealing with (sub)directories and take order of lines into account

| Q             | A
| ------------- | ---
| Branch?       |  4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37424
| License       | MIT

The new regex is made per positive `.gitignore` line. Which is a match group followed by a negative lookbehind with all the negations that were on lines after that line. This also fixes some other bugs that didn't match the `.gitignore` spec and two incorrect tests. I think it's likely that there are more edge cases this PR may not cover, but I haven't found them yet.

See the issue for more info.

Commits
-------

609dcf6 [Finder] Fix GitIgnore parser when dealing with (sub)directories and take order of lines into account
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Finder Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @stof @Jeroeny @xabbuh @carsonbot
0