WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 5 days ago

#47381 new enhancement

Remove the Composer lock file from version control

Reported by: johnbillion Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-patch
Focuses: Cc:

Description

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

Change History (2)

#1 @ayeshrajans
3 weeks ago

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.

This ticket was mentioned in Slack in #cli by johnbillion. View the logs.


5 days ago

Note: See TracTickets for help on using tickets.