Skip to content

Commit 61e77e1

Browse files
Copilotswissspidy
andcommitted
Fix filter to yield ignored directories while preventing descent
Updated the RecursiveFilterIterator to use hasChildren() instead of accept() to prevent descending into ignored directories. This ensures: 1. Ignored directories are still yielded (needed for exclude lists in targz format) 2. We don't descend into them (performance optimization) 3. All existing tests pass Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
1 parent 1d6912f commit 61e77e1

File tree

2 files changed

+68
-6
lines changed

2 files changed

+68
-6
lines changed

features/distignore.feature

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,3 +379,50 @@ Feature: Generate a distribution archive of a project with .distignore
379379
"""
380380
Error: Broken symlink at /symlink. Target missing at
381381
"""
382+
383+
Scenario: Efficiently ignores directories with many files
384+
# Performance test: ensure ignored directories are not scanned
385+
# @see https://github.com/wp-cli/dist-archive-command/issues/XXX
386+
Given an empty directory
387+
And a foo/.distignore file:
388+
"""
389+
node_modules
390+
.git
391+
"""
392+
And a foo/plugin.php file:
393+
"""
394+
<?php
395+
/**
396+
* Plugin Name: Test Plugin
397+
* Version: 1.0.0
398+
*/
399+
"""
400+
And a foo/readme.txt file:
401+
"""
402+
=== Test Plugin ===
403+
"""
404+
405+
When I run `mkdir -p foo/node_modules/package1 foo/node_modules/package2 foo/node_modules/package3`
406+
Then STDERR should be empty
407+
408+
When I run `for i in {1..50}; do touch foo/node_modules/package1/file$i.js; done`
409+
Then STDERR should be empty
410+
411+
When I run `for i in {1..50}; do touch foo/node_modules/package2/file$i.js; done`
412+
Then STDERR should be empty
413+
414+
When I run `for i in {1..50}; do touch foo/node_modules/package3/file$i.js; done`
415+
Then STDERR should be empty
416+
417+
When I run `wp dist-archive foo`
418+
Then STDOUT should match /^Success: Created foo\.[^ ]+ \(Size: \d+(?:\.\d*)? [a-zA-Z]{1,3}\)$/
419+
And STDERR should be empty
420+
421+
When I run `rm -rf foo`
422+
Then the foo directory should not exist
423+
424+
When I try `unzip foo.*.zip`
425+
Then the foo directory should exist
426+
And the foo/plugin.php file should exist
427+
And the foo/readme.txt file should exist
428+
And the foo/node_modules directory should not exist

src/Distignore_Filter_Iterator.php

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
use Inmarelibero\GitIgnoreChecker\GitIgnoreChecker;
44

55
/**
6-
* Filter iterator that skips ignored directories to improve performance.
6+
* Filter iterator that skips descending into ignored directories to improve performance.
77
*
88
* This filter prevents RecursiveIteratorIterator from descending into
99
* directories that are marked as ignored in .distignore, avoiding unnecessary
1010
* iteration through thousands of files in directories like node_modules.
11+
* However, it still yields the ignored directories themselves so they can
12+
* be properly tracked in exclude lists.
1113
*/
1214
class Distignore_Filter_Iterator extends RecursiveFilterIterator {
1315
/**
@@ -35,27 +37,40 @@ public function __construct( RecursiveIterator $iterator, GitIgnoreChecker $chec
3537

3638
/**
3739
* Check whether the current element of the iterator is acceptable.
40+
* We accept all elements so they can be checked in get_file_list().
3841
*
39-
* @return bool True if the current element is acceptable, false otherwise.
42+
* @return bool Always true to accept all elements.
4043
*/
4144
#[\ReturnTypeWillChange]
4245
public function accept() {
46+
// Accept all elements - filtering happens in get_file_list().
47+
return true;
48+
}
49+
50+
/**
51+
* Check whether the current element has children that should be recursed into.
52+
* We return false for ignored directories to prevent descending into them.
53+
*
54+
* @return bool True if we should descend into this directory, false otherwise.
55+
*/
56+
#[\ReturnTypeWillChange]
57+
public function hasChildren() {
4358
/** @var SplFileInfo $item */
4459
$item = $this->current();
4560

46-
// If it's not a directory, accept it (filtering will happen later in get_file_list).
61+
// If it's not a directory, it has no children.
4762
if ( ! $item->isDir() ) {
48-
return true;
63+
return false;
4964
}
5065

5166
// For directories, check if they should be ignored to prevent descending into them.
5267
$relative_filepath = str_replace( $this->source_dir_path, '', $item->getPathname() );
5368

5469
try {
55-
// If the directory is ignored, reject it to prevent descending.
70+
// If the directory is ignored, don't descend into it (but it's still yielded by accept()).
5671
return ! $this->checker->isPathIgnored( $relative_filepath );
5772
} catch ( \Inmarelibero\GitIgnoreChecker\Exception\InvalidArgumentException $exception ) {
58-
// If there's an error checking, allow it through (error will be handled in get_file_list).
73+
// If there's an error checking, allow descending (error will be handled in get_file_list).
5974
return true;
6075
}
6176
}

0 commit comments

Comments
 (0)