phar-composer
phar-composer copied to clipboard
Add support for excluding files
Now that v0.0.2 lays the foundation for handling each sub-package independently, we should add an config option to exclude a set of files from the resulting phar.
Ideally, each project should be able to specify which files are to be excluded separately, with one project not affecting any other project's exclude config.
For now, this requirement can simply be avoided by using a simple build script as suggested in #91 to delete all unwanted files from a temporary build directory.
In the future, we may implement explicit support for excluding certain files. Some inspiration can be taken from Symfony's Finder component in https://github.com/symfony/finder/blob/bd866ffc62322350897dac99d1cf96b1b5879700/Gitignore.php and https://github.com/symfony/finder/blob/bd866ffc62322350897dac99d1cf96b1b5879700/Finder.php#L699. This feature is built into 4.3+, but we can't currently depend on this version without breaking BC. Being MIT-licensed, we can probably reuse some of this use without adding a dependency.
This is a proposal for how excludes might be implemented and configured. Would be good to know if you support this direction, desire a different approach, or want it left alone.
I'm not committing to making these changes. But I think my source review & notes below could be helpful if someone else wishes to, and may serve as documentation of existing file excludes.
<?php
$iterator = Finder::create()
->files()
->ignoreVCS(true)
->exclude(rtrim($this->getPathVendor(), '/'))
->notPath('/^composer\.phar/')
->notPath('/^phar-composer\.phar/')
->in($this->getDirectory());
It appears this excludes:
- .git
- vendor/
- composer.phar
- phar-composer.phar (but not versioned like phar-composer-1.4.0.phar)
This looks like a good place to add the functionality of excluding from some config or cli input.
It is made trickier by the fact that TargetPhar::addBundle() is called for the main package directory and each vendor dependency indiscriminately.
Implementing Excludes (proposal)
My main proposal would be to modify Package::bundle()
to accept an array $excludes = []
in the following format:
<?php
$excludes = [
// names of directories to exclude. 'exclude' is the method on Symfony's Finder. See https://github.com/symfony/symfony/blob/e3aeb7f08e86c1e7ff03d7ea167c0e85d928b3d3/src/Symfony/Component/Finder/Finder.php#L329
'exclude' => [ 'build', 'cache'],
// filename patterns to exclude. Same name as Symfony's Finder.
'notName' => ['*.md', '*.png'],
// notPath, notContains, and other symfony Finder methods could be implemented
];
$package->bundle($excludes);
Then bundle()
would exclude files with: foreach ($excludes as $method => $arg){ $finder->$method($arg); }
Configuring Excludes (proposal)
- Proposal 1: Add cli arguments for each of the finder commands
- Proposal 2: Add a
-config_file
cli option which takes a json path that will contain excludes as defined above
A problem here is vendor directories. In Proposal 2(config file), the json could easily use package names as keys.
Using CLI arguments like -notName "*.md" -notName "*.png"
seems pretty straightforward, but I'm not sure how a vendor name would be added without doing something very cumbersome. I'm not familiar with Symfony's CLI.
Personally, I favor Proposal 1 (cli arguments) and only supporting excludes on the root package because
- phar-composer does not use config files (README states "Zero additional configuration" as a feature)
- It is simpler than a json file
- If I configure excludes on a package that I'm dependent upon, it might provide unreliable results
I haven't looked at how the CLI integrates with everything else, so I'm not sure how the configuration would get from the App to PharComposer (which calls Package::bundle())
@ReedyBear Thank you for your feature proposal, really appreciate all the work you put into explaining your ideas here :+1:
Would be good to know if you support this direction, desire a different approach, or want it left alone.
You brought up some really good insights and I think this is a great foundation to move towards supporting this OOTB. I personally don't have much experience with this topic, as I'm mostly working on ReactPHP and Framework X, so I'm really interested seeing a pull request with your suggested implementation (plus tests, impact on the current behavior and everything around that).
Like @clue said above, I think we can also take some inspiration from https://github.com/symfony/finder/blob/bd866ffc62322350897dac99d1cf96b1b5879700/Gitignore.php and https://github.com/symfony/finder/blob/bd866ffc62322350897dac99d1cf96b1b5879700/Finder.php#L699, and re-implement a similar logic into the phar-composer project here.
Not sure when we'll have the time to work on this ourselves (currently involved in ReactPHP v3 and Framework X v0.17.0
), so contributions are welcome!