Problem/Motivation
At the moment our container cache key is constructed from
$parts = ['service_container', $this->environment, \Drupal::VERSION, Settings::get('deployment_identifier'), PHP_OS, serialize(Settings::get('container_yamls'))];
This is great for core because it means that any updates that introduce new services just work. But it is painful for contrib because it has to always add empty updates to indicate that a container rebuild is necessary.
Steps to reproduce
Update any contrib module.
Proposed resolution
I think we can do better and use a hash of the composer lock file data in the container key. This will mean more container rebuilds but things will be more reliable.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #50 | 3509069-nr-bot.txt | 91 bytes | needs-review-queue-bot |
| #44 | 3509069-nr-bot.txt | 91 bytes | needs-review-queue-bot |
| #17 | 3509069-nr-bot.txt | 91 bytes | needs-review-queue-bot |
| #42 | Screenshot 2025-06-26 at 3.28.52 AM.png | 100.96 KB | godotislate |
Issue fork drupal-3509069
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:
- 3509069-use-a-better
compare
- 3509069-container-cache-key
changes, plain diff MR !11300
Comments
Comment #2
alexpottComment #3
alexpottI think we have some options to improve this. The simplest is to hash the composer installed versions raw data. This means that the container will be rebuilt on any change to composer. We could achieve the same thing if we used the lock hash but that would mean we would need to read composer.lock whereas using
InstalledVersions::getAllRawData()means that PHP can opcache everything. Another option would be have a composer plugin that could write the lock hash to a PHP file for us to read - in a similar way that composer writes the installed versions.There are some side effects of this approach. One is any change to your composer installed versions will trigger a container rebuild even if a the module, theme or library is not Drupal installed yet. Another is because we take the root project reference into account if your root composer is checked in to git and this repo contains your custom code then any change to custom code will cause a container rebuild (if you do a composer install afterwards). For me this is a good thing.
Comment #4
longwaveCan we discover the class name of the autoloader, as that appears to contain the lockfile hash? This is presumably to work around similar problems with opcode caching.
then in vendor/composer/autoload_real.php:
Comment #6
alexpottComment #7
longwaveThe class is only used for initialization so there is nothing holding on to it after the autoloader is set up but you can find the name
This is probably faster than serialize and hash? json_encode() is also faster than serialize() iirc (but json_decode is slower than unserialize)
Comment #8
alexpott@longwave this class does not always use the hash... locally on my core checkout where I work...
I have nfi why InitDrupal9 :D
Comment #9
alexpottFWIW generating the hash takes 0.00072503089904785 Seconds
And that suffix is not reliable... it's generated like this:
See AutoloaderGenerator.php - I guess mine is a carry-over?!!?! No idea tbh.
Comment #10
alexpottThe reason I decided to have a look at this is because of #3509039: Updating to 8.x-2.0-beta20 from 8.x-2.0-beta19 causes WSD and a feeling that it is unfair that core doesn't have to have empty updates for container rebuilds while contrib does. And that difference makes things harder to explain and remember.
Comment #11
alexpottRe #9 so removing vendor and regenerating gets me a class using the composer hash. But I think we've proved that that method is unreliable. I think if we want the most performant option we're going to need to have a composer plugin write this info for us into a php file.
Comment #12
longwaveDrupal9is a remnant from #3254149: Remove config.autoloader-suffix from composer.json - but yeah if it can be overridden and isn't guaranteed to change then it's not reliable here I guess.Comment #13
alexpottTurns out writing the hash out is not tricky at all.
Comment #14
alexpottI just realised we don't want the hash from the composer.lock file - that's the content hash ie. composer.json hash. What we need need is a hash of the lock file contents - ie. something that changes when a version changes. So I've updated the code to do that.
Comment #15
alexpottComment #16
longwave@joachim reminded me that this solution could also be used to solve #1792310: Wrong DRUPAL_ROOT with non-standard code structure
Comment #17
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #18
alexpottIt would be great to have some reviews on this...
Comment #19
joachim commented> public static function getCode(PackageInterface $root_package, InstalledRepositoryInterface $repository): string {
We'll need to add a parameter to that when #1792310: Wrong DRUPAL_ROOT with non-standard code structure joins in to add its own data to the DrupalInstalled class, probably the ManageOptions object. But that's fine to happen in that other issue, as the class with this method is marked as @internal.
> scaffold does not run if you install Drupal from core git repo
How do we resolve this?
Comment #20
alexpott> scaffold does not run if you install Drupal from core git repo
Well this issue resolves this for this case because we're calling the preAutoload dump from core code now so it's way better. I don't think scaffolding has to run but there being a common preAutoload dump is necessary and a good thing. This issue introduces that and will make #1792310: Wrong DRUPAL_ROOT with non-standard code structure easier as you'll be able to rely on the presence of the constant.
Comment #21
smustgrave commentedLeft a small comment on the MR.
Comment #22
ekes commentedThe autoloader not being updated is, from my understanding causing quite some confusion when geocoder plugins, which are not modules, and are installed only with composer, are sometimes not detected. #3153678: How to add Gecoder 3.x | 4.x providers
Comment #23
alexpottThis would be great to land in 11.3.0
Comment #24
godotislateA couple comments on the MR for minor docblock issues, and some nits that are fine to go without. I also tested this locally and the apcu prefix and cache container keys change as expected with changes to composer packages or running composer install after a git commit.
Do we need a CR for the above?
Also, is it possible there are projects out there that aren't built with core-recommended or scaffold? If so, would there be an error because the DrupalInstalled class doesn't exist?
Comment #25
alexpott@godotislate
Is a great question. Core is just such a use-case. However I think we should make drupal/core dependent on drupal/core-composer-scaffold as that makes everything simpler and is fundamentally correct. This allows the instructions to be quite simple - see the CR https://www.drupal.org/node/3531162
Comment #26
joachim commented> The DrupalInstalled.php file is located in the drupal in directory in the vendor directory.
This doesn't read right, not sure what it's supposed to say.
Comment #27
joachim commentedMade a few tweaks -- the README needs to mention this.
Also I'd like to bring more docs over from the Drupal root issue, which IIRC had more docs on the code that writes the file.
> file_put_contents($vendor_dir . '/drupal/DrupalInstalled.php'
What's the reason for putting it here rather than where core's files are put? Or in the webroot?
Comment #28
godotislateLooked at the CR and made edits, including the line mentioned in #26, but it'd be good to get other eyes on it now, too.
MR looks good to me as well.
A couple things outstanding from #27:
Is there anything left to be done here?
Only guessing at the reasoning, but for one, it'd involve a new .gitignore entry then, right?
Comment #29
joachim commented> Is there anything left to be done here?
Yes, I think so. I need to compare the two MRs.
> Only guessing at the reasoning, but for one, it'd involve a new .gitignore entry then, right?
It would be good to document the reasoning so future people don't have to guess :)
Comment #30
godotislateComment #31
joachim commented> Only guessing at the reasoning, but for one, it'd involve a new .gitignore entry then, right?
That is a good reason if that is the reason, but I'm not keen about putting something in vendor/$VENDOR. Things don't belong in that directly, they go in package subfolders.
What can't we put this in the web/core folder, which is gitignored in the recommended install template too?
Comment #32
alexpottvendor is where code is supposed to go and it is where our composer plugins are. This file is generated by a composer plugin. Also it's where composer put's its generated files.
We could add documentation along the lines of
But for me this documentation is not really necessary. Saying the composer places autoloadable files in vendor does not feel unexpected at all. Also any location outside of vendor will means we need to think git ignoring and deployment. You have to deploy your vendor directory somehow.
Maybe we only need to document on this issue? Ie. this comment.
Re the relationship between this and the drupal locations MR @joachim one question I have is do we ever expect a user to manual change the location written to the file or can we always rely on the composer plugin working it out correctly?
Comment #33
joachim commented> vendor is where code is supposed to go and it is where our composer plugins are. This file is generated by a composer plugin. Also it's where composer put's its generated files.
Composer manages /vendor, and it puts packages in /vendor/VENDOR/PACKAGE. /vendor/composer is a special case, because it's composer's own folder.
Putting our own thing in /vendor/drupal, at the VENDOR level is completely unexpected, and it's creating another Drupalism.
> Re the relationship between this and the drupal locations MR @joachim one question I have is do we ever expect a user to manual change the location written to the file or can we always rely on the composer plugin working it out correctly?
The idea with #1792310: Wrong DRUPAL_ROOT with non-standard code structure was that a developer might change properties in composer.json such as the webroot. The DrupalLocations / DrupalInstalled file would only ever be generated automatically, based on those properties in composer.json, and you wouldn't ever change it manually.
Comment #34
alexpottI disagree the composer creating files in vendor is unexpected. But let's look for precedence...
\Http\Discovery\Composer\Plugin::preAutoloadDump actually writes a file in vendor/composer in certain situations which is even more surprising to me... but whatevs.
\Nevay\SPI\Composer\Plugin::dumpGeneratedServiceProviderData also writes a file to vendor/composer....
Hmmm... there is a pattern here - I guess we should be writing to vendor/composer ... also both of those plugins have the classname start with Generated which seems like a pattern...
@joachim what do you think about moving the file to vendor/composer and calling it GeneratedDrupalInstalled? One thing that's interesting to be me is that these plugins still use their own namespace - so their classes are
\Nevay\SPI\GeneratedServiceProviderDataand\Http\Discovery\Strategy\GeneratedDiscoveryStrategy. even though they are in vendor/composer.I think given we can put it in whatever namespace we like maybe \Drupal\Composer\GeneratedDrupalData or something similar would work best.
Comment #35
alexpott@joachim that comment is a great addition about the file being generated.
Comment #36
godotislate@alexpott FWIW, I think the proposed changes in #34 are fine. I also think that it'd be good to get this in as early as possible for 11.3, and I don't think that the file location needs to be a blocker. We can create a follow-up as needed if there are problems with the file location, as long as it's resolved by 11.3.0-alpha1?
Comment #37
joachim commentedI would put it in the folder our own package is put in, which is WEBROOT/core. That's already going to be .gitignored.
> \Http\Discovery\Composer\Plugin::preAutoloadDump actually writes a file in vendor/composer in certain situations which is even more surprising to me... but whatevs.
I would talk to the maintainers of Composer about whether that is a good idea before doing it.
Comment #38
alexpottI've reached out to @naderman and @Jordi Boggiano (maintainers of Composer) in Symfony slack to ask. No reply yet.
Comment #39
alexpottHere's a transcript of my conversation with @Jordi Boggiano.
alexpott
@naderman @Jordi Boggiano I’ve got a composer plugin that needs to write a class during pre autoload dump. The contents of the class are based on information from composer. I’ve noticed that other composer plugins that do something similar write files to vendor/composer/ which feels really interesting to me and I don’t know why they do that. Is this the recommended place for plugins to write utility classes like this that depend on information that composer has?
Thanks for any pointers :slightly_smiling_face:
Jordi Boggiano
@alexpott hmm.. it is kinda the only reserved/safe place for composer to write things because we know for sure vendor/composer/autoload_real.php for example will never exist due to a package named composer/autoload_real.php as we control that vendor. Why people dump their stuff in there I do not know, can't say that I condone this but I assume they mostly do it because it feels like the place to dump random stuff because composer does it
Jordi Boggiano
it's fine as long as people use unique enough file names..
Jordi Boggiano
like GeneratedServiceProviderData and GeneratedDiscoveryStrategy are not strictly names I would consider namespaced/unique enough but hey, people take the chances they want to take
Jordi Boggiano
that's why plugins are great.. I don't need to maintain them if shit goes wrong:smile:
alexpott
@Jordi Boggiano thanks for reply. My own implementation dumped the file in my own namespace directory in vendor which feels safer. I was wondering why these other plugins did not take that approach and if there was a good reason because yeah the potential for file name clashes feels an odd risk to take.
Jordi Boggiano
yeah using your own vendor sounds safer to me
Comment #40
alexpottGiven the above discussion with @Jordi Boggiano, I still feel that using
vendor/drupalis the best place. I disagree that placing files in vendor is a Drupalism. I would argue that placing files outside of vendor is by far the bigger Drupalism. Code that is in vendor is much easier to place outside of the webroot and is therefore my secure. In an ideal world core would be in vendor and we would not be having this discussion.Comment #41
joachim commentedOk so that's /vendor/composer out :)
My concern with /vendor/drupal instead of /web/core is DX.
Could someone with this branch checked out confirm that an IDE can find the file containing the DrupalInstalled class from a place in the code where it's referenced? (PHP language features in my IDE hammer my laptop, sorry.)
If that's not the case, then it's not an obvious place for a developer to look for that file.
> In an ideal world core would be in vendor and we would not be having this discussion.
In which case, we'd put DrupalInstalled in vendor/drupal/core. Which is the package folder, which is where I'm proposing we put it -- just that our package folder goes in a funny place.
Once this issue is in, and #1792310: Wrong DRUPAL_ROOT with non-standard code structure is in, then we get a lot closer to having core in /vendor.
Comment #42
godotislatePHPStorm finds the class, no problem.

The ValidatableConfig job in the build is failing, but it seems like that's because the job doesn't run composer install again after the switch back to the latest MR commit, and not an issue specific to this MR.
RTBC +1.
Comment #43
joachim commented> scaffold does not run if you install Drupal from core git repo
Ok so I just thought of a way in which we fix this.
If we write this file to /web/core, then an initial version of DrupalInstalled.php can be in the git repo for core, in that location, containing default values for the constants.
Then the scaffold plugin overwrites it.
Comment #44
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #45
godotislateFor my core development workflow, this would mean there'd be a modified DrupalInstalled.php file in my repo a lot of the time, which is an annoyance (probably minor) when rebasing etc.
We could alternatively protect against DrupalInstalled somehow being missing with a
class_existscheck:class_exists(DrupalInstalled::class) ? DrupalInstalled::VERSIONS_HASH : \Drupal::VERSIONComment #46
joachim commented> For my core development workflow, this would mean there'd be a modified DrupalInstalled.php file in my repo a lot of the time
If you're installing Drupal core direct from a git clone, then scaffolding doesn't run, does it?
And anyway, I was suggesting that scaffolding would leave the file alone if it's the git version.
(Also, you already have composer.json modified whenever you install additional tools -- that's why I made https://github.com/joachim-n/drupal-core-development-project)
Comment #47
alexpottPlease let's not overwrite a file from core that would be the worst of all world. I really do not think there is an issue with vendor/drupal containing Drupal classes, not from an IDE or a conceptual basis.
Also with this change the root composer.json and a Drupal scaffold enabled site are running the same preAutoLoad dump script - which is a good thing because it'll improve the performance of sites because a site of classes that are required to make respond to a cache hit will now not be autoloaded they will just be included.
I rebased the MR to fix conflicts with 11.x
Comment #48
godotislateNightwatch test failed, needs rerunning most likely.
Comment #49
godotislateNightwatch passed, everything lgtm now.
Comment #50
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #51
alexpottFixed the merge conflict with 11.x
Comment #52
catchOne question on the MR, which was also discussed a bit back in February (but didn't look up the thread, just found it with ctrl-f for 'class_exists' on this page.
Comment #53
godotislateI posted. suggestions on the MR to use
class_existsper #52.Comment #54
ressaThanks for working on this, it sounds like it may help clear APCu caches, after switching from using a deprecated core module, to its contrib replacement?
Adding a few related issues, with examples of the problems a stale APCu cache can result in.
Comment #56
mxr576The
class_exists()failsafe check makes sense to me as well - I've applied that suggestion.I've also reviewed and tested the patch. The change record exists, so I'm removing the "Needs change record" tag.
The final implementation heavily relies on Composer, which raises an interesting question about potential new patterns for custom module development. Some developers (myself included) already register
web/modules/customas a local path repository and install custom modules via Composer. This approach:For custom modules under version control, I assume
$package->getSourceReference()returns the git commit hash on a project code hosted in VCS. If so, this would mean that changes to custom module service definitions would automatically trigger container identifier change, which seems like a beneficial side effect of this implementation.This workflow enhancement could become a recommended best practice for custom module development in Composer-based Drupal projects. Anyway, a potential follow up on this change.
The current test failure is unrelated: #3539366: Default DB transaction isolation set to read-committed breaks InstallerIsolationLevelExistingSettingsTest test
Comment #57
nicxvan commentedNeeds a rebase for the failure then i think it can be rtbc based on 56.
Comment #58
godotislateRebased. There's one
class_exists()suggestion that wasn't applied thought, and I left it as is in case it wasn't applied for a reasonComment #59
mxr576No, I have just missed it.
Comment #60
mxr576Comment #61
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #62
godotislateRebased for merge conflict.
Comment #63
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #64
mxr576Rebased, resolved conflicts on composer.json and composer.lock.
Comment #65
mxr576Comment #66
catchStill have some vague undefined reservations here, but hopefully those are unfounded, certainly nothing concrete any more - I think the class_exists() check resolves most of them anyway.
Committed/pushed to 11.x, thanks!