Remove the Composer lock file from version control

[johnbillion]

WordPress develop includes a composer.json file in its root which is used to declare developer dependencies. Currently it's only used for WPCS but #46815 proposes using it for PHPUnit too in order to remove the dependency on a globally installed PHPUnit (and #47256 proposes using it for several external dependencies).

The existence of the composer.lock file in the VCS repo prevents PHPUnit from being added as a dependency because its version and its dependencies vary depending on the version of PHP that's in use, meaning the lock file also needs to vary. If the dependencies are installed (and the lock file is created and committed) using PHP 7.1 or higher then a user trying to install the dependencies using PHP 7.0 or lower will not be able to proceed because the PHP version dependency for PHPUnit 7 that's declared in the lock file won't be met.

The solution is to remove the lock file from version control so that any user on any supported PHP version can successfully install the dependencies. This will also allow the separate PHPUnit installation step on Travis CI to be removed.

In addition, composer.lock should be added to .gitignore and svn:ignore.

---

CCing some interested parties: @netweb @welcher @vinkla @Rarst @ayeshrajans @spacedmonkey @giuseppe.mazzapica

[ayeshrajans]

Thanks for the ping @johnbillion. I had commented on other Trac issues as well, and I still think it's a good idea to simplify the logic in the travis.yml and not install PHPUnit globally.

Removal of composer.lock file is a big step, and we should evaluate the pros and cons.

Reproducible builds

One of the major reasons to have a composer.lock in the first place is to make sure we get the exact same build in two composer install calls. Drupal and Joomla, both similar software as us and adopted Composer throughout the software before us, still keep their lock files in the repo.

With the proposal to use composer to install other dependencies such as sodium_compat, I think it's important to lock down the non-dev dependencies to a specific commit hash and review dependency updates one by one. This adds some friction, but it's worth it for a software that powers 30% of the web :)

Projects such as WordPlate encourage the use of Composer more and more, and I think as the WordPress Core, we should follow what the rest of the industry/community does.

Dev-dependencies

Although I suggest that we lock non-dev dependencies with a lock files, dev dependencies are a different story though.

Our major dependency that we cannot lock down is PHPUnit. The current approach to install a different version globally isn't an elegant solution, but I understand why that workaround is there.

As a first step, perhaps we can use a composer require phpunit/phpunit:^4.0|6.0|^7.0 in the travus.yml file, and let composer pick the appropriate phpunit version depending on the platform.

npm package lock files
In the repo, we have the package-lock.json file present. I think we should be consistent in regards to PHP and JS dependencies.

--

Bottom line is that, I think we should keep the composer.lock file considering the tendency of us using composer dependencies in WordPress such as sodium_compat, polyfills, etc. It's considered a good practice to use lock files in major software, and I think we should follow suit.

Our current phpunit installation approach could use some improvements such as the use if a single composer require phpunit/phpunit:^x|^y|^z command and remove the if/else blocks in the travis.yml file, and also not assume the global installation of dependencies.

[desrosj]

This still needs discussion and a patch. Going to punt this one.

[jrf]

I'd very much like to revive this ticket as the existence of the committed composer.lock file is - as @johnbillion states quite eloquently above - a blocker for easily testing cross-version PHP.

Reason to have a committed composer.lock file

Let's look at the (valid) reasons for projects to have a committed composer.lock file:

1. Runtime dependencies - i.e. things in require - which need to be locked to a specific version with which the software has been tested.

This reason does NOT apply to WordPress.
Yes, WordPress has runtime dependencies like Sodium Compat, PHPMailer, Requests and more. However, 1) those are not (currently) managed via Composer and 2) if that would change in the future, those could be locked at a fixed version constraint instead of allowing for a version range.

2. Contributors only need to worry about their contribution working with one particular set of dependencies.

This reason again does NOT apply to WordPress as all WordPress code needs to work cross-version, so only, for instance running the tests against PHP 7.4 using PHPUnit 7 and sending in a pull request/patch and expecting it to be accepted, even though the tests don't pass on other PHP versions, is not acceptable.

So, having said that, I don't see a valid reason for having a committed composer.lock file.

Consistency with the NPM side of the project, to me, is not an argument as NPM and Composer/PHP are very different beasts and the use of a lock file for either should be evaluated on their own merits in each situation.

Reasons to remove it the committed composer.lock file

1. Make it easier for contributors to test against multiple different PHP versions locally.
2. Make it easier to reproduce the builds run on GH Actions locally without having to check in the GH Actions script what hacks are needed to get the builds to run.

Other considerations

You may say, but what about managing things like WordPressCS ? We don't want builds to start failing randomly because WPCS released a new version....
And yes, you are right to bring that up. Tools like WPCS should be fixed to a specific version when we remove the composer.lock file to prevent random build failures on a new release.
We should probably also add a fixed dependency to a specific PHP_CodeSniffer version as otherwise - with WPCS allowing for a wider range of PHPCS versions -, a new PHPCS release may have a similar effect.

In other words, that is something which can be managed via the composer.json file instead of via a lock file and tickets should be opened on a new release of those type of dependencies to do a managed update.

Moving forward

All in all, I'm very much in favour of removing the composer.lock file.

To do this, as far as I can see, a patch would be needed which covers the following:

• [ ] Remove the composer.lock file.
• [ ] Add the composer.lock file to .gitignore and svn:ignore
• [ ] Fix the version constraints for a subset of the dev dependencies - in particularly WPCS and PHPCS - in the composer.json
• [ ] Remove all "hacks" currently in place in the GH Actions workflow scripts to allow for running things cross-versions.
• [ ] Make sure there is still a passing build after that 😁

[hellofromTonya]

Moving this ticket into the 5.9 milestone as it's needed to unblock adding PHPUnit Polyfills (happening in ticket #46149).

## Composer: remove lock file (trac 47381)

As per the comment I left in the actual Trac ticket, there is currently no reason to have a composer.lock file as:

• External runtime dependencies are not managed via Composer.
• Managed updates of the non-runtime dependencies can be done by locking the version used in the composer.json file to a precise version instead of using a composer.lock file.
• And having the composer.lock file in place makes it a lot more difficult to run the tests against all supported PHP versions.

With that in mind, I'm now removing the lock file and adding it to .gitignore.
👉 Important: Whoever commits this change should make sure it is also added to .svnignore !

I've reviewed the current dev dependencies, the version constraints of those and the broader spectrum of what we know about these projects. Please find a summary for each below.

### PHPUnit

• The existing version constraint was incorrect as WP currently supports running the tests on both PHPUnit 5.x (for PHP 5.6 and 7.0), as well as 7.x (for PHP 7.1-7.4).
• The version used for PHPUnit was locked at 7.5.20, which:
  1. Makes it more difficult for contributors to test changes locally on anything but PHP 7.1-7.4.
  2. Necesitates all sorts of work-arounds in the CI unit test scripts to get the tests running on other PHP versions.
• As both PHPUnit 5.x as well as 7.x are no longer supported and no new versions within those majors will ever be released anymore, we don't have to be concerned about minor/patch updates.
• Removing the lock file has no impact for the 7.x version as WP is already using the latest version 7.5.20, but will allow for running composer install/composer update on PHP 5.6-7.0 and getting the right PHPUnit version installed.
• Running composer install/composer update on PHP 8.0 will still require the --ignore-platform-reqs addition until more recent PHPUnit versions will be supported. This is being addresses in Trac ticket 46149.
• Current work-arounds in CI can now be removed and CI can use Composer to install the appropriate PHPUnit version for test runs.

### WordPress Coding Standards

• The project uses SemVer and adheres to it within the context of a coding standard, i.e. new features (sniffs) can be introduced in minors, but not in patch versions.
• Updates to new patch versions of this plugin will, generally speaking, not have a significant impact on the developer workflow, so can be safely allowed.

These are mostly small tweaks and bugfixes.

• Removing the lock file has no impact as WP is already using the latest version 2.3.0.
• With the current version constraint, updates to new minors as well as majors will need to be specifically managed, which is a good thing as updates may require new "ignore annotations" to be added, allow for existing ones to be removed or for the ruleset to be adjusted.
• The next version is expected to be a major, 3.0.0 and will include additional external dependencies.

Once WPCS 3.0.0 has been released, we can re-evaluate whether the version constraint should be locked at a specific version or should allow for patch updates automatically.

• Side-note: I am one of the maintainers of this tool.

Also see: ​https://github.com/WordPress/WordPress-Coding-Standards/releases

### PHPCompatibility WP

Note: this is an exclusion ruleset for the underlying PHPCompatibility library, so the impact of updates to either needs to be considered.

#### PHPCompatibilityWP

• The project allows the full PHPCompatibility 9.* range under the hood. More about this below.
• Updates to new patch versions of this plugin will generally speaking have a positive impact on the developer workflow, as it means that new PHP features polyfilled by WP will no longer be flagged by the tool.
• Removing the lock file effectively updates the dependency from version 2.1.0 to 2.1.2 with the positive impact that a few new constants polyfilled in WP 5.8 are now accounted for (excluded from being flagged).
• Removing the lock file effectively updates an underlying dependency - PHPCompatibilityParagonie, which is also an exclusion ruleset - from version 1.3.0 to 1.3.1 without impact.
• With the current version constraint, updates to new minors as well as majors will need to be specifically managed, which is a good thing as updates may require new "ignore annotations" to be added, allow for existing ones to be removed or for the ruleset to be adjusted.
• The next version is expected to be a major, 3.0.0, and will include additional external dependencies and will coincide with the release of PHPCompatibility 10.0.0.
• Side-note: I am one of the maintainers of this tool.

Also see:

• ​https://github.com/PHPCompatibility/PHPCompatibilityWP/releases
• ​https://github.com/PHPCompatibility/PHPCompatibilityParagonie/releases

#### PHPCompatibility

• The project uses SemVer and adheres to it within the context of a coding standard, i.e. new features (sniffs) can be introduced in minors, but not in patch versions.
• Any and all updates to this tool _may_ impact the developer workflow as new things may get flagged.

While this may be inconvenient when this happens unexpectedly, IMO it is still a good thing as any php cross-version incompatibilities in the WordPress code base should be fixed as soon as possible.
This is different to WPCS in that new things flagged by WPCS will generally not impact end-users, while new things flagged by PHPCompatibility will impact end-users.

• It _could_ be considered to add this dependency explicitly to the composer.json file to manage updates to new versions with more control, but considering the end-user impact of things being flagged and the risk of creating version constraint conflicts with the "parent" package PHPCompatibilityWP and/or with WPCS, I would recommend against this.
• Removing the lock file has no impact as WP is already using the latest version 9.3.5.
• The next version is expected to be a major, 10.0.0 and will include additional external dependencies.

Once version 10.0.0 has been released, we can re-evaluate whether the package should be added to the composer.json file explicitly.

• Side-note: I am one of the maintainers of this tool.

Also see: ​https://github.com/PHPCompatibility/PHPCompatibility/releases/

### PHP_CodeSniffer

• Until now, this was not an explicit dependency.
• PHPCS is an underlying dependency for both WPCS as well as PHPCompatibility, which both only set a minimum, but no maximum, supported version within PHPCS majors as their version constraint for it.
• The project uses a variant to SemVer and changes in PHPCS can have a big impact on the results of both WPCS (included sniffs from PHPCS changing behaviour) as well as PHPCompatibility (tokenizer changes).
• Depending on what gets committed first, removing the lock file either has no impact or effectively updates the dependency from version 3.5.5 to 3.6.0 without impact (as per Trac 53477).
• All the same, as updates to PHPCS can have a significant impact on two of the developer workflows, I'd recommend making updates to this tool managed and have added it as an explicit dependency with a fixed version constraint to the composer.json file for that reason.
• Whenever the WPCS and/or PHPCompatibility dependency will be updated in the future, the version constraint for PHP_CodeSniffer should also be re-evaluated and possibly updated.
• Side-note: I am one the most active outside contributor to this tool.

Also see: ​https://github.com/squizlabs/php_codesniffer/releases

### The DealerDirect Composer plugin

• The project uses SemVer and adheres to it.
• Updates to new patch versions of this plugin will not have a significant impact on the developer workflow, so can be safely allowed.
• Removing the lock file effectively updates the dependency from version 0.7.0 to 0.7.1 without impact.
• With the current version constraint, updates to new minors - 0.8.0 or 1.0.0 - will need to be specifically allowed (as Composer treats minors below 1.0.0 as majors).
• As of WPCS 3.0.0/PHPCompatibilityWP 3.0.0, this dependency will no longer be needed as both WPCS as well as PHPCompatibilityWP will already require it and manage the supported versions.

Once either these majors is released and WP updates to it, the explicit dependency for this tool in the composer.json file should be removed.

• Side-note: I am one of the maintainers of this tool.

Also see: ​https://github.com/Dealerdirect/phpcodesniffer-composer-installer/releases

## Build/Test: use a custom autoloader for the PHPUnit 9.x mock object classes

... to prevent those being loaded automatically via the autoload-dev directives when a Composer installed PHPUnit 5.x version is used, as that would break the test run.

It is expected that this autoloader will be removed soon, as it should no longer be needed when the PHPUnit version constraints are widened.

Notes:

• The autoloader file will be loaded from the Test bootstrap.
• The autoloader will always be registered and directed to queue itself _before_ the Composer autoload file (which will already have been registered).
• The autoloader will only actually load the WP copies of the files/classes when PHP 8.0 in combination with PHPUnit 7.x is detected. In all other cases, the autoloader will bow out, which effectively then defers to the Composer autoload file to load the files as shipped with the installed PHPUnit version.

## WIP/ CI changes

_No description yet_ I have asked @desrosj for a second opinion and will finalize the commit after that.

Trac ticket: https://core.trac.wordpress.org/ticket/47381

[prbot]

​aristath commented on ​PR #1511:

I'm not familiar with the GHA caching options mentioned above, however, it looks like a reasonable request, the code makes sense, and testing the PR locally nothing breaks. :+1:

[prbot]

​jrfnl commented on ​PR #1511:

After today's additional feedback from @desrosj and having discussed this ticket in the ​livestreamed mob programming session, this PR is now ready for commit.

[jrf]

@SergeyBiryukov The GH PR has been updated and is now ready for commit. Would you like me to generate patch files for each of the commits ? Or one patch file containing all three commits ?

I have opened a separate ticket to further investigate switching out the Composer caching workflow steps for the predefined Composer caching action. See #53841
That ticket can be seen as a future iteration on the current changes, and is non-blocking.

[SergeyBiryukov]

In 51543:

Build/Test Tools: Remove the Composer lock file from version control.

This makes it easier to run unit tests against multiple different PHP versions.

There is currently no reason to have a composer.lock file as:

• External runtime dependencies are not managed via Composer.
• Managed updates of the non-runtime dependencies can be done by locking the version used in the composer.json file to a precise version instead of using a composer.lock file.
• Having the composer.lock file in place makes it a lot more difficult to run the tests against all supported PHP versions.

With these considerations in mind, the lock file is now removed from version control and added to .gitignore and svn:ignore.

Version constraints for the current dev dependencies are adjusted accordingly:

• PHPUnit now explicitly declares in its version constraints that PHPUnit 5.x, 6.x, and 7.x are supported. The minimum supported version for PHPUnit 5.x has been raised from 5.4 to 5.7, which in practice was already the version used for running the tests on PHP 5.6.
• PHPCompatibilityWP is effectively updated to version 2.1.2 with the positive impact that a few new constants polyfilled in WP 5.8 are now accounted for (excluded from being flagged).
• PHP_CodeSniffer is declared as an explicit dependency to ensure that updates to it will always be explicitly managed instead of inherited.
• The DealerDirect Composer plugin is effectively updated to version 0.7.1 without impact.

Follow-up to [42960], [46290], [47881], [48957].

Props jrf, johnbillion, desrosj, ayeshrajans, aristath, hellofromTonya, SergeyBiryukov.
See #47381.

[SergeyBiryukov]

In 51544:

Build/Test Tools: Use a custom autoloader for the PHPUnit 9.x mock object classes.

This prevents the classes from being loaded automatically via the autoload-dev directives when a Composer-installed PHPUnit 5.x or 6.x version is used, as that would break the test run.

It is expected that this autoloader will be removed soon, as it should no longer be needed when the PHPUnit version constraints are widened.

Notes:

• The autoloader file will be loaded from the Test bootstrap.
• The autoloader will always be registered and directed to queue itself _before_ the Composer autoload file (which will already have been registered).
• The autoloader will only actually load the WP copies of the files/classes when PHP 8.0 in combination with PHPUnit 7.x is detected. In all other cases, the autoloader will bow out, which effectively then defers to the Composer autoload file to load the files as shipped with the installed PHPUnit version.

Follow-up to [48957], [49037], [51543].

Props jrf.
See #47381.

[SergeyBiryukov]

In 51545:

Build/Test Tools: Switch to always running the tests via Composer.

Previously the tests were run via a PHPUnit Phar file for PHP 5.6–7.4, with PHP 8.0 using a Composer-installed version of PHPUnit.

Running the tests via a Phar without the need for a composer install is (marginally) faster in overall build time, however, this commit is part of a larger chain of changes which will make the test suite PHPUnit cross-version compatible.

With an eye on those upcoming changes, which will allow us to run the tests on the most appropriate PHPUnit version for each supported PHP version, it is opportune to switch to using a Composer-installed version of PHPUnit for all PHP versions supported by WordPress. Previously this was not possible without additional conditional update commands being run, due to the composer.lock file being in place and being locked at PHPUnit 7.5.20.

Switching over to using the Composer-installed PHPUnit version, with that PHPUnit version adjusting based on the PHP version, allows for some minor simplifications in the GitHub Actions script.

This means we need additional measures to make sure that the Composer cache file does not go too far out of date as that would significantly slow down the builds.

By adding a "Last Monday" date to the cache key, in combination with the pre-existing OS, PHP version and the hash of the composer.json file, we can guarantee that:

1. There will be a cache created for each OS/PHP combination.
2. These caches will be replaced whenever a change is made to the composer.json file.
3. These caches will be replaced every Monday of each week ensuring that the cache file does not go too far out of date.

Note: The NPM script test:php is now no longer needed during the builds. However, to prevent breaking the workflow of contributors who may be used to having the command available, the command remains available.

In a future iteration we may be able to replace the caching of the Composer dependencies with the Composer cache action as offered on the GitHub marketplace, which would further simplify the script.

Follow-up to [42960], [46290], [47881], [48957], [49037], [51543], [51544].

Props jrf, desrosj.
Fixes #47381.

[prbot]

​jrfnl commented on ​PR #1511:

Closing as committed via https://core.trac.wordpress.org/changeset/51543, https://core.trac.wordpress.org/changeset/51544 and https://core.trac.wordpress.org/changeset/51545

[jrf]

Reopening the ticket to allow for (potentially) backporting [51543] and [51545] to a select number of older WP branches in combination with a select group of the commits which will be made for #46149.

This backport is intended to make it easier for future security backports, which are bundled with tests, to be backported without having to adjust those patches for supported PHPUnit versions.

This backport will also make it easier for plugins and themes, which have integration tests based on the WP test suite, to still run their test suite in combination with multiple WP versions.

[hellofromTonya]

Reclosing as backports to WordPress 5.8 back to 5.2 are complete. See #53911.