Narrow the container cache key to the referenced %env.* variables#5927
Narrow the container cache key to the referenced %env.* variables#5927SanderMuller wants to merge 1 commit into
Conversation
8a335c8 to
c373f1f
Compare
| private function relevantEnvVariableNamesForCacheKey(): ?array | ||
| { | ||
| $names = self::BUILD_TIME_ENV_VARIABLES; | ||
| foreach ($this->allConfigFiles as $file) { |
There was a problem hiding this comment.
This isn't a great way to do this. I think the tracking of used %env... should be somewhere deeper in the DIC compiler. I'm not entirely sure how to implement it. This would have to be researched where Nette DI actually expands this parameters and whether we can efficiently tap into it.
There was a problem hiding this comment.
@ondrejmirtes shall I apply the suggestion in this comment below, or do you have another direction you want me to explore?
|
Agreed, scanning config text for I tried tapping into Nette's expansion directly first, and it's awkward. What does work cleanly is to stop baking env into the container at all: move The one case it doesn't cover is env read at build time via That removes the regex entirely. I can push it to this PR if the direction looks right. |
c373f1f to
9581033
Compare
|
@ondrejmirtes Reworked this per your note that the tracking belongs deeper in the DIC compiler rather than scanning config text. Instead of scanning for The one build-time exception is env read outside One behavior change worth calling out: because This is the direction I went with, but I am happy to take it a different way if you would prefer to keep the build-time error or have another approach in mind. |
|
I really don't know if it would work or not. In phpstan-src we're locked to nette/di 3.1.5 ( Lines 175 to 247 in 709654e |
PHPStan puts the whole environment into the DI container cache key, so any unrelated env change (CI, shell session, etc.) forces a full container recompile (phpstan/phpstan#14072). Keep env a static parameter and narrow only the cache key to the %env.*% variables the loaded configs actually reference, plus the env vars read at build time by CompilerExtensions (BUILD_TIME_ENV_VARIABLES). The full environment stays in the container parameters, so %env.*% resolution and the compiled container are unchanged. References are enumerated from the config as parsed by PHPStan's own NeonAdapter - the same parser the container compiler uses - not from raw config text: comments are ignored, and service entities become Statements so a %env.NAME% in a parameter value, a service argument or a factory is found the same way it is at compile time. The %env.NAME% grammar mirrors Nette's parameter-name grammar, so dashed names like %env.MY-VAR% are handled. A bare %env% reference, or a config that can't be parsed here, keeps the full environment in the key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9581033 to
e758aec
Compare
|
Thanks, you're right to be wary. I've dropped the dynamic-parameter direction. To find the referenced variables without a raw text scan, each config is parsed with PHPStan's own This runs against the locked nette/di 3.1.5, since |
Closes phpstan/phpstan#14072
PHPStan puts the whole environment into the DI container cache key, so any unrelated environment change (a different CI job, a new shell session, an editor integration exporting a variable) invalidates the key and forces a full container recompile.
This keeps
enva static parameter and narrows only the cache key to the%env.*%variables the loaded configs actually reference, plus the variables read at build time by CompilerExtensions (BUILD_TIME_ENV_VARIABLES, currentlyPHPSTAN_FNSR). The full environment stays in the container parameters, so%env.*%resolution and the compiled container are unchanged; only the cache key is narrowed.Enumerating the referenced variables
References are collected from the config as parsed by PHPStan's own
NeonAdapter(the same parser the container compiler uses), rather than from a raw text scan:%env.X%mentioned only in a comment no longer forces a recompile;Statements, so a%env.X%in a parameter value, a service argument or a factory is found the same way it is at compile time;%env.NAME%grammar mirrors Nette's parameter-name grammar (%([\w.-]*)%), so dashed names like%env.MY-VAR%are handled.A bare
%env%reference (the whole environment), or a config that cannot be parsed here, conservatively keeps the full environment in the key, so the change can never reuse a stale container.On keeping
envstaticenvstays a static parameter. An earlier revision of this PR made it a dynamic parameter; that breaks%env.X%.neonincludes (include paths are resolved against static params atloadConfiguration, which failse2e/bug-12606), and it is unclear how nette/di reconciles referencing a dynamic parameter from a static one. Keepingenvstatic means the container build is byte-identical to today, so the nette/di E2E tests are unaffected; the only observable change is that unrelated environment changes no longer force a recompile.