wp-cli / dist-archive-command

Create a distribution .zip or .tar.gz based on a plugin or theme's .distignore file
https://developer.wordpress.org/cli/commands/dist-archive/
MIT License
47 stars 23 forks source link

Use `.gitignore` syntax for `.distignore` #78

Closed BrianHenryIE closed 11 months ago

BrianHenryIE commented 1 year ago

I worked on this at WCUS Contributor Day (https://github.com/wp-cli/wp-cli/issues/5832).

Fixes #44.

It was easier than expected to modify the inmarelibero/gitignore-checker library to use .distignore instead of .gitignore. I've opened a PR and it has no breaking changes and is short and easy to read, so hopefully it will be merged. I do notice the maintainer hasn't been active on GitHub in a few months, but he does have 100+ repos, so I'm still optimistic: https://github.com/inmarelibero/gitignore-checker/pull/11

There was an additional PR needed where comments and empty lines needed to be ignored: https://github.com/inmarelibero/gitignore-checker/pull/12

So this PR also has require-dev cweagans/composer-patches to apply the patches for those PRs as we need them (it's a very popular library I've used extensively).

Where the diff looks as though Behat tests were deleted in dist-archive.feature, they were moved to distignore.feature (and history preserved) and additional tests written based on the comments in #44. PHPUnit tests were removed because the function being tested has been removed (and the library used in its place).

There were no breaking changes with existing tests for .distignore rules.

inmarelibero/gitignore-checker requires PHP 7.1.

BrianHenryIE commented 1 year ago

tar is failing to exclude files on Linux. I tried a few things but it's not working yet. I'll think on it for a while, and hopefully get a local Ubuntu instance set up so I can test changes without needing to run on GitHub Actions.

swissspidy commented 1 year ago

Sweet, both PRs got merged already!

danielbachhuber commented 1 year ago

Nice! Thanks for working on this, @BrianHenryIE

BrianHenryIE commented 1 year ago

This is working correctly now. The failing workflows are all for PHP <7.1, as expected.

The solution to the Linux file exclusions was two part, stop using ^ and $ characters because it's not actually a regex, it's a glob. And use the --anchored parameter so files in subdirectories do not match rules for files whose shorter paths is a subset of the subdir file.

danielbachhuber commented 12 months ago

@wojsmol Is there an easy way to exclude the < PHP 7.1 builds from the testing matrix?

BrianHenryIE commented 12 months ago

Here's some rough work that might help, untested:

jq '.require.php' composer.json | sed 's/[^0-9.]//g' will pull out 7.1 from composer.json. It will be problematic if the syntax ^7.4|^8.0 is used. jq '.require.php' composer.json | sed 's/[^0-9.|]//g' | jq -Rc 'split("|") | min' will take ^7.4|^8.0, strip characters and treat 7.4|8.0 as a list, then return the minimum version.

- name: Early exit
  if: ${{ matrix.php }} < $(jq '.require.php' composer.json | sed 's/[^0-9.|]//g' | jq -Rc 'split("|") | min')
  run: |
    gh run cancel ${{ github.run_id }}
    gh run watch ${{ github.run_id }}
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

See: https://stackoverflow.com/a/75809743

Exactly what I was looking for to skip a step/job and also mark it as skipped, not success or failed.

And "may need actions: 'write' permission".

danielbachhuber commented 11 months ago

@BrianHenryIE I ended up taking the hacky route:

I created an issue to improve upon this later: https://github.com/wp-cli/.github/issues/72

danielbachhuber commented 11 months ago

Ugh, I guess composer install still fails 😕 Which makes sense...

schlessera commented 11 months ago

I came up with a solution for the test workflows, and while it is not pretty, it does the job of not failing the tests in an unexpected way. See discussion here: https://wordpress.slack.com/archives/C02RP4T41/p1695112126777159?thread_ts=1695076992.280459&cid=C02RP4T41

schlessera commented 11 months ago

Great work, @BrianHenryIE !

danielbachhuber commented 11 months ago

@schlessera It looks like the testing.yml workflow file got clobbered... https://github.com/wp-cli/dist-archive-command/commit/88c56923bb9750134b6e8fc14b5182c0d29f6361

I think it makes less sense to disable automatic syncing for this repository. What about instead reading the minimum PHP version from a .env file or similar?

schlessera commented 11 months ago

@danielbachhuber I'm wondering if the automatic syncing of testing.yml is still needed at all, given that the changes are pretty much all made in reusable-tesing.yml now...

danielbachhuber commented 11 months ago

@schlessera I think I'd prefer to keep it syncing automatically, if possible: