Problem/Motivation
When there is a large phpstan diff precommit hooks can oom.
#3529504: Fix phpstan errors in UpdatePathTestTrait
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/scripts/dev/c...
Steps to reproduce
Proposed resolution
- Add an option to set a -1 memory limit.
Remaining tasks
Should we add the memory bypass to anything else?
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3529507
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:
- 3529507-set-memory-limit
changes, plain diff MR !12351
Comments
Comment #2
nicxvan commentedComment #3
xjmDefaulting to -1 is probably bad for CI, but an option would be nice.
Comment #4
nicxvan commentedComment #6
nicxvan commentedComment #7
xjmRetitling to make it clear we are not, in fact, trying to take down GitLab.
Comment #8
smustgrave commentedSeems like a good improvement for developers.
Comment #9
alexpottI think we should remove the output as it is not telling you much and I'm not sure that it is helpful.
Comment #10
alexpottIf this helps a committer commit then +1 - I think the maintenance burden is light and if PHPStan stops supporting the option it'll tell us with an easy error to fix. Also happy to merge https://github.com/alexpott/d8githooks/pull/34/commits/6c03a0ceae34850a9... once this lands.
Comment #11
nicxvan commentedDo you mean the print statements?
I had thought it would be helpful to clarify that memory limit is set since i assume most core committers use your precommit hooks and might not see the issue.
I'm happy to remove it too, just clarifying.
Comment #12
alexpottYep the print statements - I'm not sure they add anything.
Comment #13
xjmThanks @alexpott! (@alexpott is the subsystem maintainer for this code.)
Comment #14
nicxvan commentedDone! Thanks
Comment #15
alexpottlooks good to me... will merge once this rtbc.
Comment #16
xjmWe are a small group and whoever commits the followup to the git hooks will probably just tell the team. :)
I'll try to manually test this together with the PR later.
Comment #17
smustgrave commentedFeedback from @alexpott appears to be addressed.
Comment #18
xjmSorry, it was NR for manual testing with the upstream PR. I should have been explicit. I can do this, but am also fine for someone else to if they beat me to it.
Comment #19
xjmTested manually.
Without fix
memory_limit = 256MResult:
With fix
memory_limit = 256Mprecommitin the d8githooks with the flag from that MR and committing the result.Result: Baseline update committed with full PHPStan scan.
Now it is RTBC. :)
Comment #20
alexpottCommitted and pushed 2ffd3412fe7 to 11.x and 4842c65b535 to 11.2.x and ca127551d4f to 10.6.x and 642eb99e04e to 10.5.x. Thanks!
Backported to 10.5.x as this is non-runtime code and it does not hurt to have everything in sync.
Comment #25
xjm