Skip to content
Snippets Groups Projects

Use a better container cache key

Open Use a better container cache key
issue/drupal-3509069:3509069-container-cache-key into 11.x
4 unresolved threads
Open Alex Pott requested to merge issue/drupal-3509069:3509069-container-cache-key into 11.x
4 unresolved threads

Closes #3509069

Merge request reports

Members who can merge are allowed to add commits.

Merged results pipeline #448353 failed

Stage: 🪄 Lint
Stage: 🗜️ Test
Pipeline: drupal

#448355

    Merged results pipeline failed for 61186a1c

    Code Quality is loading
    Test summary results are being parsed
    Full report
    Ready to merge by members who can write to the target branch.
    Loading

    Activity

    Filter activity
    • Approvals
    • Assignees & reviewers
    • Comments (from bots)
    • Comments (from users)
    • Commits & branches
    • Edits
    • Labels
    • Lock status
    • Mentions
    • Merge request status
    • Tracking
  • catch @catch started a thread on an old version of the diff
    • core/lib/Drupal/Core/DrupalKernel.php
    839 840 * The cache key used for the service container.
    840 841 */
    841 842 protected function getContainerCacheKey() {
    842 $parts = ['service_container', $this->environment, \Drupal::VERSION, Settings::get('deployment_identifier'), PHP_OS, serialize(Settings::get('container_yamls'))];
    843 $parts = ['service_container', $this->environment, DrupalInstalled::getVersionsHash(), Settings::get('deployment_identifier'), PHP_OS, serialize(Settings::get('container_yamls'))];
    • C
      catch @catch
      Maintainer

      This needs a class_exists - there could be permissions issues writing the file if it's created by package manager, or could there be a race condition where this code is called before the class is first created, maybe?

      But otherwise this is very clever and would simplify a lot of things.

    • A
      Alex Pott @alexpott
      Author Maintainer

      Yeah I need to look into the failures and find out why they are happening. I think package manager has to be able to write this - otherwise it can't write Composer\InstalledVersions file either and that'd be a massive bug.

    • A
      Alex Pott @alexpott
      Author Maintainer

      I've resolved this by listening to the pre autolaod dump in the scaffold plugin and moving the code there.

    • Alex Pott changed this line in version 8 of the diff

      changed this line in version 8 of the diff

    • Please register or sign in to reply
  • Alex Pott added 1 commit

    added 1 commit

    Compare with previous version

  • Alex Pott added 1 commit

    added 1 commit

    • bd038abb - Move the pre autoload dump code to somewhere its runs for all Drupal projects

    Compare with previous version

  • Alex Pott added 1 commit

    added 1 commit

    Compare with previous version

  • Alex Pott @alexpott started a thread on the diff
    • core/lib/Drupal/Core/Composer/Composer.php
    19 20 * The event.
    20 21 */
    21 22 public static function preAutoloadDump(Event $event) {
    22 // Get the configured vendor directory.
    23 $vendor_dir = $event->getComposer()->getConfig()->get('vendor-dir');
    • Comment on lines 21 to -23
      A
      Alex Pott @alexpott
      Author Maintainer

      So interestingly none of this was ever running for sites built from core recommended because this only works via the scripts in the root composer.json of the core repo. I think moving this code to scaffold will solve this - but scaffold does not run if you install Drupal from core git repo... so we need to link the two... fun.

    • Please register or sign in to reply
  • Alex Pott @alexpott started a thread on the diff
    • composer/Plugin/Scaffold/Plugin.php
    192 $autoload['classmap'] = array_merge($autoload['classmap'], [
    193 $vendor_dir . '/symfony/http-kernel/HttpKernel.php',
    194 $vendor_dir . '/symfony/http-kernel/HttpKernelInterface.php',
    195 $vendor_dir . '/symfony/http-kernel/TerminableInterface.php',
    196 ]);
    197 }
    198 if ($repository->findPackage('symfony/dependency-injection', $constraint)) {
    199 $autoload['classmap'] = array_merge($autoload['classmap'], [
    200 $vendor_dir . '/symfony/dependency-injection/ContainerInterface.php',
    201 ]);
    202 }
    203 if ($repository->findPackage('psr/container', $constraint)) {
    204 $autoload['classmap'] = array_merge($autoload['classmap'], [
    205 $vendor_dir . '/psr/container/src/ContainerInterface.php',
    206 ]);
    207 }
  • Dave Long
  • Alex Pott added 1 commit

    added 1 commit

    Compare with previous version

  • Alex Pott added 104 commits

    added 104 commits

    Compare with previous version

  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • composer/Plugin/Scaffold/DrupalInstalledTemplate.php 0 → 100644
    6 use Composer\Repository\InstalledRepositoryInterface;
    7
    8 /**
    9 * Produces code for the DrupalInstalled file.
    10 *
    11 * @internal
    12 */
    13 class DrupalInstalledTemplate {
    14
    15 /**
    16 * Gets the code for the DrupalInstaller class.
    17 *
    18 * @return string
    19 * The PHP code to write to the Drupal locations class.
    20 */
    21 public static function getCode(PackageInterface $root_package, InstalledRepositoryInterface $repository): string {
  • Stephen Mustgrave left review comments

    left review comments

    • Please register or sign in to reply