Problem/Motivation
#3529507: Allow setting memory limit to -1 for phpstan precommit hooks added a useful setting to the core linting script that allows PHPStan to optionally use unlimited memory. This is especially useful for committer pre-commit checks, which run the script without calling it manually.
PHPCS also can OOM in the same way in the same script, so the same fix can also be applied to PHPCS:
The PHP_CodeSniffer "phpcs" command ran out of memory.
Either raise the "memory_limit" of PHP in the php.ini file or raise the memory limit at runtime
using `phpcs -d memory_limit=512M` (replace 512M with the desired memory limit).Steps to reproduce
- Set
php.inito a relatively low memory limit (e.g. 128 or 256 MB). - Make a change to
core/phpcs.xml.dist. - Run:
bash ./core/scripts/dev/commit-code-check.sh
Proposed resolution
Extend the flag added in #3529507: Allow setting memory limit to -1 for phpstan precommit hooks to also bypass the memory limit for PHPCS.
Remaining tasks
Needs patch and manual testing.
Followup needed for the committer toolchain.
User interface changes
N/A
Introduced terminology
None.
API changes
Should be none.
Data model changes
None.
Release notes snippet
Not needed.
Issue fork drupal-3538145
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3538145-optionally-bypass-memory
changes, plain diff MR !12859
Comments
Comment #2
xjmNB: I filed this as postponed and self-assigned because I may have someone look at it tomorrow.
Comment #4
joaopauloscho commentedI'm working on this.
Comment #6
joaopauloscho commentedPlease review the MR. Thanks!
Comment #8
smustgrave commentedSeems this already got reviewed by nicxvan
Comment #9
nicxvan commentedSorry I forgot to update the status!
Comment #10
joaopauloscho commentedPlease review again. Thanks.
Comment #11
nicxvan commentedLooks right to me now! Docs are here: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#specify...
I'll leave it to a committer to test since they have the tooling set up, but the code is rtbc from me pending a practical test.
Comment #12
nicxvan commentedAlso there shouldn't need to be a follow up in the committer toolchain is using the same flag.
Comment #13
smustgrave commentedSince it's been about 2 months and feedback appears to be have been addressed going to mark it.
Comment #14
xjmI went to look for this in the RTBC queue and looked at #3549208: Set memory limit for `composer phpcs` command by accident instead. 😀 Even if that goes in, I still think this change is worth making. (Haven't had a chance to test yet though; will do so when I can unless someone else beats me to it.)
Comment #16
xjmTested it by committing #3550065: Forbid '@encode' annotation.
Thanks @joaopauloscho and @nicxvan!