Skip to content

Commit dfc6434

Browse files
committed
bug symfony#54691 [Finder] Also consider .git inside the basedir of in() directory (nickvergessen)
This PR was merged into the 6.4 branch. Discussion ---------- [Finder] Also consider .git inside the basedir of in() directory | Q | A | ------------- | --- | Branch? |6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix described below | License | MIT ### Bug description Previously when the `in()` directory was the root of your repository the `.gitignore` was skipped from consideration, which meant potentially all local files are ignored. E.g. in the Nextcloud community we have the server locally with an `.gitignore` for the `apps/` directory. Now when an app developer develops their own app locally, they put their own git repository inside this ignored `apps/` folder. The CS Fixer rules we use since a long time are: ```php (new Finder()) ->ignoreVCSIgnored(true) ->notPath('l10n') ->notPath('vendor') ->notPath('vendor-bin') ->in(__DIR__); ``` and this worked pretty well with 5.4 and previous versions of the finder. Now that the Nextcloud project finally dropped PHP 8.0 for the latest version and we get the update to Symfony 6.4 the bug become visible locally, but our CI still works as we don't need the "server" repository as a parent when running the CS fixer. ### Locally - nested inside "server" repository ``` ~/Nextcloud/server/apps/spreed$ composer run cs:check > php-cs-fixer fix --dry-run --diff PHP CS Fixer 3.54.0 15 Keys Accelerate by Fabien Potencier, Dariusz Ruminski and contributors. PHP runtime: 8.2.18 Loaded config default from "/home/nickv/Nextcloud/30/server/appsbabies/spreed/.php-cs-fixer.dist.php". Using cache file ".php-cs-fixer.cache". 0 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] ``` :x: No files found, all ignored by `server/.gitignore` ### CI without a parent repository ``` Run composer run cs:check || ( echo 'Please run `composer run cs:fix` to format your code' && exit 1 ) > php-cs-fixer fix --dry-run --diff PHP CS Fixer 3.54.0 15 Keys Accelerate by Fabien Potencier, Dariusz Ruminski and contributors. PHP runtime: 8.1.2-1ubuntu2.13 Loaded config default from "/home/runner/actions-runner/_work/spreed/spreed/.php-cs-fixer.dist.php". 0/502 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% ``` :white_check_mark: Correct file list found ### 5.4 - From my POV this is a regression from symfony#43239 (so 6.1+ only) and it becomes more "obvious" when backporting the unit test to 5.4 as it passes there without patching `VcsIgnoredFilterIterator.php`. - Therefore cc-ing `@julienfalque` as author, to clarify if this was intentional Commits ------- af870c6 [Finder] Also consider .git inside the basedir of in() directory
2 parents 0704c26 + af870c6 commit dfc6434

File tree

2 files changed

+55
-5
lines changed

2 files changed

+55
-5
lines changed

src/Symfony/Component/Finder/Iterator/VcsIgnoredFilterIterator.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ public function __construct(\Iterator $iterator, string $baseDir)
3737
{
3838
$this->baseDir = $this->normalizePath($baseDir);
3939

40-
foreach ($this->parentDirectoriesUpwards($this->baseDir) as $parentDirectory) {
41-
if (@is_dir("{$parentDirectory}/.git")) {
42-
$this->baseDir = $parentDirectory;
40+
foreach ([$this->baseDir, ...$this->parentDirectoriesUpwards($this->baseDir)] as $directory) {
41+
if (@is_dir("{$directory}/.git")) {
42+
$this->baseDir = $directory;
4343
break;
4444
}
4545
}

src/Symfony/Component/Finder/Tests/Iterator/VcsIgnoredFilterIteratorTest.php

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ protected function tearDown(): void
3434
*
3535
* @dataProvider getAcceptData
3636
*/
37-
public function testAccept(array $gitIgnoreFiles, array $otherFileNames, array $expectedResult)
37+
public function testAccept(array $gitIgnoreFiles, array $otherFileNames, array $expectedResult, string $baseDir = '')
3838
{
3939
$otherFileNames = $this->toAbsolute($otherFileNames);
4040
foreach ($otherFileNames as $path) {
@@ -51,7 +51,8 @@ public function testAccept(array $gitIgnoreFiles, array $otherFileNames, array $
5151

5252
$inner = new InnerNameIterator($otherFileNames);
5353

54-
$iterator = new VcsIgnoredFilterIterator($inner, $this->tmpDir);
54+
$baseDir = $this->tmpDir.('' !== $baseDir ? '/'.$baseDir : '');
55+
$iterator = new VcsIgnoredFilterIterator($inner, $baseDir);
5556

5657
$this->assertIterator($this->toAbsolute($expectedResult), $iterator);
5758
}
@@ -74,6 +75,55 @@ public static function getAcceptData(): iterable
7475
],
7576
];
7677

78+
yield 'simple file - .gitignore and in() from repository root' => [
79+
[
80+
'.gitignore' => 'a.txt',
81+
],
82+
[
83+
'.git',
84+
'a.txt',
85+
'b.txt',
86+
'dir/',
87+
'dir/a.txt',
88+
],
89+
[
90+
'.git',
91+
'b.txt',
92+
'dir',
93+
],
94+
];
95+
96+
yield 'nested git repositories only consider .gitignore files of the most inner repository' => [
97+
[
98+
'.gitignore' => "nested/*\na.txt",
99+
'nested/.gitignore' => 'c.txt',
100+
'nested/dir/.gitignore' => 'f.txt',
101+
],
102+
[
103+
'.git',
104+
'a.txt',
105+
'b.txt',
106+
'nested/',
107+
'nested/.git',
108+
'nested/c.txt',
109+
'nested/d.txt',
110+
'nested/dir/',
111+
'nested/dir/e.txt',
112+
'nested/dir/f.txt',
113+
],
114+
[
115+
'.git',
116+
'a.txt',
117+
'b.txt',
118+
'nested',
119+
'nested/.git',
120+
'nested/d.txt',
121+
'nested/dir',
122+
'nested/dir/e.txt',
123+
],
124+
'nested',
125+
];
126+
77127
yield 'simple file at root' => [
78128
[
79129
'.gitignore' => '/a.txt',

0 commit comments

Comments
 (0)