composer-installer: Order of `installed_paths` inconsistent between runs

Problem/Motivation

I am using this plugin to register several phpcs sniffs from different repos into my installed_paths with phpcs. I am also using Travis CI’s caching directives in combination with phpcs’ caching directives in order to cache the results of scans between runs for files that have not been modified. However, it appears that this plugin doesn’t register installed_paths in a consistent way between CI runs, which invalidates the phpcs cache, thus negating the benefits of caching between runs.

Expected behaviour

Runs of this plugin between CI jobs should produce consistent output for the value of installed_paths (specifically, the order of the paths).

Actual behaviour

The order of the paths in installed_paths is not always the same between CI runs. This is likely due to a race condition of when certain packages finish installing.

Steps to reproduce

Use a standard in composer.json that references multiple standards, or reference multiple standards in composer.json (see example below). Create a Travis CI job to run phpcs and echo the value of installed_paths. Note that it will not always be in the same order between runs.

Example for run 1:

"installed_paths": "../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs,../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs",

Example for run 2:

"installed_paths": "../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs,../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs",

Contents of composer.json:

{
  "require-dev": {
    "alleyinteractive/alley-coding-standards": "^0.2.0"
},

Contents of composer.json in alley-coding-standards:

{
  "name": "alleyinteractive/alley-coding-standards",
  "description": "PHPCS sniffs for Alley Interactive",
  "type": "phpcodesniffer-standard",
  "license": "GPL-2.0-or-later",
  "require": {
    "squizlabs/php_codesniffer": "^3.5.0",
    "dealerdirect/phpcodesniffer-composer-installer": "*",
    "wp-coding-standards/wpcs": "^2.3.0",
    "automattic/vipwpcs": "^2.1.0",
    "phpcompatibility/phpcompatibility-wp": "*"
  },
  "require-dev": {}
}

Proposed changes

Make the installed_paths value consistent between runs by alphabetizing the values. This will allow repos that use this plugin to leverage Travis CI and phpcs caching for unmodified files by ensuring that the configuration values in the cache file, specifically the value for installed_paths, is consistent between runs if nothing changed.

Environment

Question Answer
OS Linux, Ubuntu Bionic, running on Travis CI
PHP version 7.3.14
Composer version 2.0.8
PHP_CodeSniffer version 3.5.8
Dealerdirect PHPCS plugin version 0.7.0
Install type project local

Output of vendor/bin/phpcs --config-show:

Using config file: /home/travis/build/alleyinteractive/brookings/vendor/squizlabs/php_codesniffer/CodeSniffer.conf

Contents of CodeSniffer.conf:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs,../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs',
)
?>

Tested against master branch?

  • I have verified the issue still exists in the master branch.

I have not verified this against the master branch, but looking at the diff between 0.7.0 and the current HEAD, there shouldn’t be anything in that diff that would affect this issue.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 24 (24 by maintainers)

Most upvoted comments

I created a PR (#126) that should fix this.

Thanks for pointing that out! I’ve still not recovered the muscle-memory from my fingers 100% when typing. Fixed now. 👍

you guys ++you all++ (didn’t you just update the CoC ?)

With async running, the order of the packages as returned by $this->composer->getRepositoryManager()->getLocalRepository()->getPackages() may be different between runs - is that what you are saying ?

Yep—I don’t know all of the Composer internals, but something inherent to the async workings of Composer that would cause the order of the packages to be different on a first, fresh install, like in a CI environment, as you said.

OK, that’s actually a feature of my before_script, not a bug - I’m checking to see whether there are any files that changed that would need to be run through phpcs before installing it, and in the PR I’m debugging, there aren’t, so it skips the install. I’ve got a composer global require "phpunit/phpunit=6.1.*" earlier in the before_script which is why I’m seeing output from Composer at all. So the first time I run composer install in my project directory, it reports registering the sniffs (which I would expect, because it just set up the vendor directory for the first time on the container). Subsequent composer installs after the first one result in the nothing to update message.

This seems to be one of these edge cases we didn’t think about when the plugin was created, but it indeed seems like a valuable improvement to the current logic.

Just for context: When this plugin was created PHPCS did not have the caching feature yet. Result caching for PHPCS was introduced in PHPCS 3.0.0, but even then, the configuration settings weren’t taken into account when determining whether the cache was valid. That part was only introduced in PHPCS 3.3.0.

Thanks! Glad to help.