- Issue created by @alexpott
- π¬π§United Kingdom alexpott πͺπΊπ
I 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.
- π¬π§United Kingdom longwave UK
Can 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.
"content-hash": "7f3c203e5297306f9277da08961993f2",
then in vendor/composer/autoload_real.php:
class ComposerAutoloaderInit7f3c203e5297306f9277da08961993f2
- π¬π§United Kingdom longwave UK
The 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
> array_filter(get_declared_classes(), fn($class) => str_starts_with($class, 'ComposerAutoloaderInit')); = [ 257 => "ComposerAutoloaderInit7f3c203e5297306f9277da08961993f2", ]
This is probably faster than serialize and hash?
- π¬π§United Kingdom alexpott πͺπΊπ
@longwave this class does not always use the hash... locally on my core checkout where I work...
array_filter(get_declared_classes(), fn($class) => str_starts_with($class, 'ComposerAutoloaderInit')); = [ 241 => "ComposerAutoloaderInitDrupal9", ]
I have nfi why InitDrupal9 :D
- π¬π§United Kingdom alexpott πͺπΊπ
FWIW generating the hash takes 0.00072503089904785 Seconds
And that suffix is not reliable... it's generated like this:
$suffix = $config->get('autoloader-suffix'); // carry over existing autoload.php's suffix if possible and none is configured if (null === $suffix && Filesystem::isReadable($vendorPath.'/autoload.php')) { $content = (string) file_get_contents($vendorPath.'/autoload.php'); if (Preg::isMatch('{ComposerAutoloaderInit([^:\s]+)::}', $content, $match)) { $suffix = $match[1]; } } if (null === $suffix) { $suffix = $locker !== null && $locker->isLocked() ? $locker->getLockData()['content-hash'] : bin2hex(random_bytes(16)); }
See AutoloaderGenerator.php - I guess mine is a carry-over?!!?! No idea tbh.
- π¬π§United Kingdom alexpott πͺπΊπ
The reason I decided to have a look at this is because of π Updating to 8.x-2.0-beta20 from 8.x-2.0-beta19 causes WSD Active 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.
- π¬π§United Kingdom alexpott πͺπΊπ
Re #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.
- π¬π§United Kingdom longwave UK
Drupal9
is 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. - π¬π§United Kingdom alexpott πͺπΊπ
Turns out writing the hash out is not tricky at all.
- π¬π§United Kingdom alexpott πͺπΊπ
I 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.
- π¬π§United Kingdom longwave UK
@joachim reminded me that this solution could also be used to solve π Wrong DRUPAL_ROOT with non-standard code structure Needs review
The 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.
- π¬π§United Kingdom alexpott πͺπΊπ
It would be great to have some reviews on this...
- π¬π§United Kingdom joachim
> public static function getCode(PackageInterface $root_package, InstalledRepositoryInterface $repository): string {
We'll need to add a parameter to that when π Wrong DRUPAL_ROOT with non-standard code structure Needs review 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?
- π¬π§United Kingdom 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 π Wrong DRUPAL_ROOT with non-standard code structure Needs review easier as you'll be able to rely on the presence of the constant.