Exclude directory paths from file system search #401
Conversation
Directories can also match the glob search pattern if their names end in ".php", which will cause a read error later since the ContentRetriever implementers are expecting files. As far as I know, the only way to fix this is to do an additional check to ensure the URI is not of a directory. This resolves #306.
soren121
changed the title
Codecov Report
@@ Coverage Diff @@
## master #401 +/- ##
=========================================
Coverage ? 84.24%
Complexity ? 699
=========================================
Files ? 54
Lines ? 1568
Branches ? 0
=========================================
Hits ? 1321
Misses ? 247
Partials ? 0
|
Upon further inspection, it seems that the workspace should never contain a directory, so it should be safe.
felixfbecker
left a comment
felixfbecker
left a comment
There was a problem hiding this comment.
Accepting as a fix, but an additional stat on every file is not the best. Maybe the implementation should be changed to not rely on globs anymore...
|
I agree. For what it's worth, I tested on a codebase of ~3500 PHP files and the performance (in seconds) was the same as it was without the is_dir check. |
|
Maybe it should utilize the symfony finder which is also capable of using native commands to speedup the process |
|
What does it provide over |
|
we need to benchmark whether the "native finders" are faster/more effiecient or not. or was your question related to features the sfFinder can provide? |
|
maybe a non-glob (php native) finder would be enough though |
|
What are "native finders"? |
|
symfony uses |
|
I couldn't find that in Symfony's codebase, could you give a code pointer? |
|
hmm it seems this feature was deprecated and removed later on |
* Exclude directories from file system search Directories can also match the glob search pattern if their names end in ".php", which will cause a read error later since the ContentRetriever implementers are expecting files. As far as I know, the only way to fix this is to do an additional check to ensure the URI is not of a directory. This resolves felixfbecker#306.
4 participants
Directories can also match the glob search pattern if their names end in ".php", which will cause a read error later since the ContentRetriever implementers are expecting files. This behavior can be reproduced by installing the Composer package "mtdowling/jmespath.php".
As far as I know, the only way to fix this is to do an additional check to ensure the URI is not of a directory.
This resolves #306.