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

Add support for excluding files #5

Open
clue opened this issue May 26, 2013 · 3 comments
Open

Add support for excluding files #5

clue opened this issue May 26, 2013 · 3 comments

Comments

@clue
Copy link
Owner

clue commented May 26, 2013

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.

@clue
Copy link
Owner Author

clue commented Nov 12, 2019

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.

@clue clue added this to the v1.2.0 milestone Nov 12, 2019
@clue clue modified the milestones: v1.2.0, v1.3.0 Dec 11, 2020
@clue clue removed this from the v1.3.0 milestone Dec 28, 2021
@ReedyBear
Copy link

ReedyBear commented Feb 4, 2024

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.

In Package::bundle():

<?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)

  1. Proposal 1: Add cli arguments for each of the finder commands
  2. 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

  1. phar-composer does not use config files (README states "Zero additional configuration" as a feature)
  2. It is simpler than a json file
  3. 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())

@SimonFrings
Copy link
Contributor

SimonFrings commented Apr 18, 2024

@ReedyBear Thank you for your feature proposal, really appreciate all the work you put into explaining your ideas here 👍

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!

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

No branches or pull requests

3 participants