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

Finder ->ignoreVCSIgnored(true) doesn't do what it claims #37781

Closed
GrahamCampbell opened this issue Aug 8, 2020 · 11 comments
Closed

Finder ->ignoreVCSIgnored(true) doesn't do what it claims #37781

GrahamCampbell opened this issue Aug 8, 2020 · 11 comments

Comments

@GrahamCampbell
Copy link
Contributor

Symfony version(s) affected: 5.1.3

Description
The ignoreVCSIgnored method says it can make the finder skip over all ignored files, however this is not the case. It assumes there is only exactly one .gitignore file in the root of the directory the finder is "in".

It is reasonable (and probably desirable) that the finder does not recursively search upwards for .gitignore files, however, unreasonable that it does not respect ignore files in subdirectories and mandates the "in" directory must have an ignore file.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@GrahamCampbell
Copy link
Contributor Author

Yes, this bug is still present. I actually don't use/need this feature, but I would vote for its deprecation and removal, given it doesn't work, and there's no obvious way to correct it.

@carsonbot carsonbot removed the Stalled label Feb 18, 2021
@mvorisek
Copy link
Contributor

mvorisek commented Apr 8, 2021

I belive fix can be easy - instead of generating the exclude regex from one .gitignore, find first all .gitignore files and generate excluded from all of them.

Here is the code:

// apply all .gitignore rules
$ignoreFinder = (new Finder())
    ->in(__DIR__)
    ->ignoreDotFiles(false)
    ->name('.gitignore');
foreach ($ignoreFinder as $ignoreFile) {
    $ignoreRegex = Gitignore::toRegex($ignoreFile->getContents());
    $finder->notPath($ignoreRegex);
}

but it fails on https://github.com/PrestaShop/PrestaShop repo with:

preg_match(): Compilation failed: lookbehind assertion is not fixed length

error message. This needs to be addressed first. I made a PR #40763 for it.

@a-r-m-i-n
Copy link

I can confirm this issue, with ignoring child .gitignore files.

I've tried to implement it on my own:

  1. Search for all .gitignore files (excluding root)
  2. Make finder call for each found directory and append iterator to a collection finder instance
  3. Now run the original (root) finder but with excluded dirs (for each found .gitignore)

This does not work properly, because the first step finds .gitignore files which should be excluded. And this is what this issue is basically about. So recursion should fix the issue here, I just don't know how ;)

@GrahamCampbell
Copy link
Contributor Author

Recursion cannot fix it, because git also looks at parent directories recursively too...

@GrahamCampbell
Copy link
Contributor Author

This is basically a no-fix, and I'd recommend deprecating and removing this function, since it cannot be implemented in a sane way.

@mvorisek
Copy link
Contributor

traversing with dirname up too the repo root is easy and quick

@GrahamCampbell
Copy link
Contributor Author

Not if the user is not allowed to do that.

@mvorisek
Copy link
Contributor

why? This is how native git works, root repo is located even if cwd in not root repo

@a-r-m-i-n
Copy link

Why did you close the issue? There is a merge request, I wanted to test, which looks promising.

@Wirone
Copy link
Contributor

Wirone commented Mar 16, 2023

For those subscribed to this issue, you may find #49703 interesting 🙂.

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

Successfully merging a pull request may close this issue.

6 participants