PHPCS config: Persistent caching during CI

[johnbillion]

Caching between linting runs with PHPCS was introduced in [45665]. We can go a step further and use a specific cache location and then instruct Travis CI to cache this between builds. This effectively reduces the linting time to a few seconds once the cache is warm.

Previously: #40539

[jrf]

Just checking: if I remember correctly, the caching feature checks if files have been changed since the last run and only runs phpcs on those changed files.

What I'm not so sure about is, whether the cache gets invalidated when PHPCS and/or external standards used with it, has been updated (i.e. composer update) and whether the cache gets invalidated if any of the used rulesets (project + underlying) change.

This should really need be verified in detail as if that is not the case, only changed files would get the benefit of updated rulesets/sniffs/fixes etc and the cache would quickly contain a lot of outdated information.

[johnbillion]

@jrf Excellent point and I'm going to check that before making the change. I would expect the cache interface to consider that, but definitely worth verifying.

[johnbillion]

cc @jdgrimes

[johnbillion]

According to ​https://github.com/squizlabs/PHP_CodeSniffer/issues/530 it appears that --cache and --cache=<file> behave differently. The former means the cache will not be invalidated if the standard changes, but the latter means it will.

[johnbillion]

Okay I've verified locally that changing the standard does invalidate the cache. I tested:

• Manually altering the rules in phpcs.xml.dist
• Altering (specifically, downgrading) the wp-coding-standards/wpcs version
• Applying a different standard via the --standard CLI flag

Anything else that might need testing?

[jrf]

@johnbillion I've ran some tests myself as well & had a look at the caching code. I concur with your conclusions.

The only thing I can think of which still needs addressing in the PHPCS caching code would be to take enabled PHP extensions into account, but that's not something we should be concerned about for this ticket.

I do wonder a little about the tests/cache/phpcs.json file name as it would suggest this has something to do with the (unit) tests, which it doesn't.

What about just letting it cache in the root directory where the ruleset lives as well to a file called .phpcs.cache ?

[desrosj]

49783.2.diff​ changes the cache file name to .phpcs-cache.json and moves it to the root directory as suggested by @jrf. I experimented with using a cache directory in root, but it seems that PHPCS wants the directory to exist.

49783.2.diff​ also adds a .phpcompat-cache.json file to cache PHP compatibility scans.

In my testing, though, I could only get the compatibility cache file to be created and work correctly.

[jrf]

@desrosj Been working on some related things. I have a feeling that @johnbillion used a directory for the cache file to allow Travis to cache the file as it seems that there is no option to cache individual files, just directories.

With that in mind, using a /[.]cache/ directory off the project root may not be a bad idea.

This directory would need a .gitkeep file to ensure it exists and can be committed (as per John's earlier patch).
The cache files then need to be added to both .gitignore as well as .svnignore to make sure that the cache files themselves do not get committed.

Also: why is /phpcompat.xml added to .gitignore ? This is a user-specific override file, not a default file name, so should probably be in the user .git/info/exclude/ rather than the project .gitignore.
Or is there documentation/a make manual page which refers to using this as an overload file ?

[desrosj]

@jrf I added that because /phpcs.xml was present in the .gitignore already and assumed this was desired and missed when I introduced the new phpcompat.xml.dist file. If this is an incorrect assumption, I can remove that from the diff.

Also, I misread the documentation for the Travis config file, so that will need to be fixed. What if the cache folder is added to the root and the tests/cache folder is removed in favor of that one? It would consolidate all cache files in one place and not cause confusion.

[jrf]

I added that because /phpcs.xml was present in the .gitignore already and assumed this was desired and missed when I introduced the new phpcompat.xml.dist file. If this is an incorrect assumption, I can remove that from the diff.

@desrosj Yes, that assumption is incorrect.

To explain it: PHPCS will automatically pick up on a configuration file if it's called [.]phpcs.xml[.dist]. This means that if one of those file names is used, the user does not have to pass the --standard=... argument on the command-line.

Between the different file names, there is precedence, with .phpcs.xml having the highest and phpcs.xml.dist the lowest.
This allows for a project to have a phpcs.xml.dist file and an individual dev working on the project to have a local phpcs.xml file which will overrule the project file (but may include it) and will automatically be loaded as long as the --standard=... CLI argument is not used.
Using this "trick", devs can, for instance, have stricter settings for themselves, test new rules before adding them or ignore a particular issue (like Windows line endings) etc.

With that in mind, adding phpcs.xml and .phpcs.xml to the .gitignore file is best practice as that allows for devs using this automatic overload feature and prevents those local files from getting committed.

The phpcompat.xml.dist file is not a standard file name PHPCS will pick up on, so it will always have to be passed as an argument on the command-line to be used.
If a dev wants to locally overrule that file, they can name the local file whatever they want (safe for the above mentioned file names) as it won't get picked up automatically anyway and they will need to pass the filename of their local overload config file on the command-line via --standard=....

So, as there is no standard naming scheme for overloading the phpcompat.xml.dist file, adding an arbitrary name to the .gitignore, is, well, arbitrary, and this is probably better suited for the dev's own .git/info/exclude file.

What if the cache folder is added to the root and the tests/cache folder is removed in favor of that one? It would consolidate all cache files in one place and not cause confusion.

All in favour of that. To make it obvious its a CI/dev directory and does not contain project files, you may want to consider naming the directory /.cache/ as I suggested above.

[jrf]

The only thing I can think of which still needs addressing in the PHPCS caching code would be to take enabled PHP extensions into account, but that's not something we should be concerned about for this ticket.

FYI: I've opened a PR to address this: ​https://github.com/squizlabs/PHP_CodeSniffer/pull/2982

[jrf]

According to ​https://github.com/squizlabs/PHP_CodeSniffer/issues/530 it appears that --cache and --cache=<file> behave differently. The former means the cache will not be invalidated if the standard changes, but the latter means it will.

I've run some additional tests with this. Just adding the results here to document it.

When using --cache the cache does not get invalidated if the standard changes, instead a separate cache file for the files under the other standard is created.

So, in effect, the options actually behave the same when using them.

The difference is that when using a named cache file, you only have one cache file and it gets invalidated and rebuild completely when something has changed (either in the ruleset, the sniffs or PHPCS itself), while when using the --cache option, you have many different cache files depending on the options used in a run. When the same options are used however, an existing cache will be used.

[jrf]

FYI - I discovered another bug in the PHPCS caching. Only relevant for those people who use the --sniffs or --exclude CLI arguments.

Bug reported in PHPCS: ​https://github.com/squizlabs/PHP_CodeSniffer/issues/2992 and fix pulled: ​https://github.com/squizlabs/PHP_CodeSniffer/pull/2996

[jrf]

FYI: PR ​https://github.com/squizlabs/PHP_CodeSniffer/pull/2982 has been merged in the mean time and will be included in the PHPCS 3.5.6 release.

Issue ​https://github.com/squizlabs/PHP_CodeSniffer/issues/2992 has been confirmed, but needs a different fix than the one I initially pulled.

[desrosj]

The wheels are in motion to move from Travis to GitHub Actions (see #50401). We should turn our attention to how GitHub Actions can cache this between workflow runs. I'll add a to-do to look into this.

[johnbillion]

FYI I haven't found a great way to run this on GitHub Actions yet because GHA doesn't support always pulling in a cache file and simultaneously always updating it if there are changes, unlike Travis, because it uses the same cache key for fetching and saving the cache during a run. Unless this has changed recently, this means you need to pull in a somewhat stale cache.

Example: ​https://github.com/johnbillion/query-monitor/blob/fbe8124794d8b53edb3455c87ac39fc596b14fd4/.github/workflows/test.yml#L35-L44

This adds the needed steps to store the PHPCS caches for coding standards and PHP compatibility scanning across workflow runs.

It uses the date of the previous Monday for the cache key to ensure that the cache is regularly refreshed. As the week progresses, some files will need to be manually scanned for each run as they are updated, but this should will not result in big difference, unless many PHP files are updated over the course of a week.

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

Before adding caching in this PR, the ​PHPCS scan took just over a minute. After, it has been reduced by over 95% to less than 3 seconds.

The caching the PHP compatibility scanning resulted in similar reduction. It's not currently being shown in action runs for this PR because I believe there is a bad cache value. That cache will be flushed at some point today when the action runner's date changes into Monday and the key resets. I've tested and verified the improvement locally.

[desrosj]

I've set up the PR above to implement caching in GitHub Actions across workflow runs. I think it's ready to go. A second set of eyes and final agreement on using a .cache directory in the project root would be appreciated.

Moving this to the 5.9 milestone so it can be wrapped up.

[desrosj]

In 52179:

Build/Test Tools: Cache the results of PHP_CodeSniffer across workflow runs.

When the PHP_CodeSniffer runs, it produces a cache file. When a cache file is present, only changed files are rescanned, making subsequent scans significantly faster.

This adds the needed steps to the corresponding GitHub Actions workflows to cache these files across runs. The cache keys include the date of the previous Monday to ensure that the cache is flushed at least weekly.

Since GitHub Action caches cannot be updated once created, the scans will take slightly longer as the week progresses and more PHP files are updated. The date within the cache key can be updated to purge twice weekly if the scan time starts to approach the current scan times.

This change also introduces a .cache directory for all caching files related to build/test tools.

Props johnbillion, jrf.
Fixes #49783.

Merged in https://core.trac.wordpress.org/changeset/52183.

[desrosj]

In 58605:

Build/Test Tools: Make use of new reusable workflows for 5.6.

This updates the 5.6 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Run E2E tests with and without SCRIPT_DEBUG (#58661).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Configure Xdebug modes in the local Docker environment (#56022).
• Improvements to the healthcheck command for the mysql container (#58867).
• A fix to grunt clean to prevent script-loader-packages.php from being deleted (#53606).

Merges [51355], [51673], [52179], [53552], [53895], [56113], [56114], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.6 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #56022, #59416, #59805, #61213, #58661, #53606.

[desrosj]

In 58610:

Build/Test Tools: Make use of new reusable workflows for 5.5.

This updates the 5.5 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Improvements to the healthcheck command for the mysql container (#58867).
• A fix to grunt clean to prevent script-loader-packages.php from being deleted (#53606).
• Skip some tests when not in the primary branch (#50401).

Merges [49264], [51355], [51673], [52179], [53552], [53895], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.5 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #59416, #59805, #61213, #53606.

[desrosj]

In 58611:

Build/Test Tools: Make use of new reusable workflows for 5.4.

This updates the 5.4 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Improvements to the healthcheck command for the mysql container (#58867).
• Skip some tests when not in the primary branch (#50401).

Merges [49264], [51673], [52179], [53552], [53895], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.4 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #59416, #59805, #61213.

[desrosj]

In 58624:

Build/Test Tools: Make use of new reusable workflows for 5.3.

This updates the 5.3 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Improvements to the healthcheck command for the mysql container (#58867).
• Skip some tests when not in the primary branch (#50401).

Merges [49264], [51673], [52179], [53552], [53895], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.3 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #59416, #59805, #61213.

[desrosj]

In 58625:

Build/Test Tools: Make use of new reusable workflows for 5.2.

This updates the 5.2 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Improvements to the healthcheck command for the mysql container (#58867).
• Skip some tests when not in the primary branch (#50401).

Merges [49264], [51673], [52179], [53552], [53895], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.2 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #59416, #59805, #61213.

[desrosj]

In 58628:

Build/Test Tools: Make use of new reusable workflows for 5.0.

This updates the 5.0 branch to utilize the new reusable workflows in trunk introduced in [58165].

This also includes backports for a some additional improvements and bug fixes that are necessary for the local development environment to continue working long term:

• The image and platform properties for the mysql container have been updated to always prefer amd64 containers (#60822).
• macos-13 is now pinned for MacOS jobs instead of macos-latest (#61340).
• Migrating to Docker Compose V2 (#60901).
• Removing the version property from docker-compose.yml (#59416).
• Improvements to how artifacts and comments for Playground testing are generated.
• Removing SVN related commands causing failures (#61216).
• Updating the actions/github-scripts action to the latest version.
• Cache the results of PHP_CodeSniffer runs (#49783).
• Move the Memcached container into the Docker Compose config (#55700).
• Improvements to the healthcheck command for the mysql container (#58867).
• Skip some tests when not in the primary branch (#50401).

Merges [49264], [51673], [52179], [53552], [53895], [56464], [57918], [58157], [57124], [57125], [57249] to the 5.0 branch.

Props johnbillion, joemcgill, swissspidy, thelovekesh, narenin, mukesh27, JeffPaul, peterwilsoncc, zieladam, ockham, SergeyBiryukov, jorbin, Clorith, afragen, jrf.
See #49783, #58867, #61340, #60822, #61216, #60901, #61101, #59416, #59805, #61213.