Invalidate APCu Class Loader automatically if necessary

Quick look:

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -70,4 +73,37 @@ public static function ensureHtaccess(Event $event) {
+EOT;
+    file_put_contents($vendor_dir . '/DeploymentIdentifier.php', $lines . "\n");
+  }

This looks wrong

@webflo: Then it is my mistake. It looks strange to me, thats why I mentioned it.

Reviewing.

1. +++ b/core/composer.json
   @@ -149,6 +149,9 @@
   +      "Drupal\\Core\\Composer\\Composer::ensureHtaccess",
   +      "Drupal\\Core\\Composer\\Composer::dumpDeploymentIdentifier"
   

   If these two switch positions they are alphabetically ordered which is more acceptable for the control freak in me.

2. +++ b/core/lib/Drupal/Core/Composer/Composer.php
   @@ -70,4 +73,37 @@ public static function ensureHtaccess(Event $event) {
   +  /**
   +   * Generates the DeploymentIdentifier class.
   +   *
   +   * @param \Composer\Script\Event $event
   +   */
   

   It wouldn't hurt to explain why we are generating this class. Perhaps add a line like:

   "This class has a method that will return an ID that matches a particular Composer installation. It is used to ensure that PHP opcode caches will be invalidated whenever the Composer dependencies change."

3. +++ b/core/lib/Drupal/Core/Composer/Composer.php
   @@ -70,4 +73,37 @@ public static function ensureHtaccess(Event $event) {
   +<?php
   +
   +namespace Drupal\Core\Composer;
   

   Maybe put a /** @file */ docblock here so that the generated file is compliant with coding standards.

4. +++ b/core/lib/Drupal/Core/Composer/Composer.php
   @@ -70,4 +73,37 @@ public static function ensureHtaccess(Event $event) {
   +/**
   + * Provides the deployment identifier created by Composer.
   + *
   + * @see
   + */
   

   The @see is incomplete, link it to the generator static method.

5. +++ b/core/lib/Drupal/Core/Composer/Composer.php
   @@ -70,4 +73,37 @@ public static function ensureHtaccess(Event $event) {
   +  /**
   +   * @return string
   +   */
   +  public function getIdentifier() {
   

   Use {@inheritdoc} instead.

6. +++ b/core/lib/Drupal/Core/Composer/DeploymentIdentifierInterface.php
   @@ -0,0 +1,24 @@
   +  /**
   +   * Returns the deployment identifier.
   +   *
   +   * @return string
   +   */
   

   @return string
     The identifier.
   

This also needs a test. We cannot change the composer dependencies in a running test so the only thing we can do is to check that the identifier that is returned by the generated class matches the composer installation.

Addressing points in #14.

OK, so hang on... :-)

One of the problems being solved here is that someone just moves the vendor directory to another location, updates the top-level autoload.php, and calls it a day, which breaks APC.

This means that the strategy here fails in two ways:

1) If they never run composer dumpautoload then the script here will never run, and APC will barf.

2) Even if they do that, the hash in composer.lock will be the same, because it's just a hash of the contents of composer.json, so subsequent changes will not be reflected.

So if it's the kernel's job to catch this (which I'm not sure it is), then the Composer script has to use the non-APC autoloader to resolve a class that's in vendor (like, for instance, Symfony\Component\ClassLoader\ApcClassLoader) for it's file path. Then hash that file path along with the composer.lock hash for the unique ID. This way the moved file will have a different hash, and will also reflect the status of composer.json.

Also, I'd suggest that we write out the hash as JSON instead of a PHP file, and put it in vendor/drupal/deployment.json. This way it has a higher probability of being removed for reasons that would change its validity anyway.

So that solves the second problem, but now we have to solve the first. Which we can't. People have to run composer dumpautoload. We should also have update.php and rebuild.php call the same script code to generate the deployment.json.

One downside to this is that the user now has to merge their git pull, because we store vendor in the repo.

But since the goal is to ultimately remove vendor from the repo, then +1.

"But since the goal is to ultimately remove vendor from the repo, then +1."

But isn't the goal to be able to remove vendor from the repo, not require it? Or to remove vendor from the do drupal repo?

1) The dump-autoload event is trigger via composer install, composer update and composer dump-autoload . The class will be automatically by one of these commands. The class will be always available, because we will run composer install during Drupal.org Packaging and Drupal CI.

Right, but the point of this issue is that anyone could move vendor at any time anywhere, and that will break APC, so we have to invalidate APC so it can start caching again.

2) The hash from composer.json or composer.lock might not change but Settings::getApcuPrefix uses the site root as additional criteria to create unique prefixes for APCu.

Yes, true, but again: We don't expect the site root to change, we expect vendor/ to change.

We generate this class on the fly because its faster than decoding the JSON file. Also the class is auto-discoverable through composers class loader. The JSON file would hard-code the vendor directory to a specific location. Drupal should not know about the location of the vendor directory, because we inject the autoloader in our application.

Sorry to sound like a broken record (or, uh, skipping CD? or poorly ripped MP3?) but a changed vendor directory is exactly the problem being solved for here, unless I've completely missed the point of https://www.drupal.org/node/2380389#comment-10375583

If the JSON (or PHP or whatever) file isn't present, then we can manage APC and generate a new one.

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -24,6 +24,9 @@ public static function preAutoloadDump(Event $event) {
+    $autoload['files'] = array_merge($autoload['files'], [
+      'vendor/DeploymentIdentifier.php'
+    ]);

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -975,8 +976,9 @@ protected function initializeSettings(Request $request) {
+      $identifier = new DeploymentIdentifier();

Again, when the vendor directory gets moved, this will result in a fatal error because APC can't load the class. That's why we want JSON, and then to check if it's there, and then load it and compare hashes.

That's the special case of \Drupal which is just file-included by the autoloader.

I have an intuition that some of this is compounded by config.autoload-suffix in both composer.json files. If we remove that, then the autoloader class changes names with a hash when the autoloader is rebuilt. I have yet to figure out if that's enough to invalidate APC's cache though. It might be a suitable hash for dynamically adding as the prefix in the kernel, though.

Also, I've discovered that if you don't manually delete vendor/autoload.php it never gets rebuilt, even without the autoload-suffix. I think Composer just gives up too soon in that case.

I expect that the composer hash will change because we rebuild the autoloader

The composer.lock hash will only change when the composer.json file changes *and* you rebuild the lock file: composer update --lock

We could use the content hash as additional information.

Same deal. The content hash just pulls in the JSON, sorts the array, writes it as a file, and then hashes. (So the hash doesn't change when you just re-arrange lines.)

Somewhat more simple... Not sure how to test it.

Though I did apply #166 from #2380389: Use a single vendor directory in the root, apply this patch, and then reload, and it works. :-)

Might refactor to put this functionality in Settings::getApcuPrefix().

Even more simple. More documentation than code changes. :-)

Still works such that you can apply #166 from #2380389: Use a single vendor directory in the root, apply this patch, and your APC-enabled Drupal installation will simply reload.

Ideas on how to test this are welcome.

I don't think the recent approach in #39 works for a few reasons:

• I think if vendor is moved you'll have problems before getting to the APC prefix generation
• If you move vendor then you doing way more than just a normal deployment and part of planning this will be need to flush your APC caches.
• I does not solve the way more usual use-case of the autoload changing due to a composer upgrade and our APCu cache needing to be invalidated.

I do think we need an issue to flush APCu in rebuild.php

I think #36 might offer some protection against moving vendor but it'll need testing.

@alexpott:

I think if vendor is moved you'll have problems before getting to the APC prefix generation

#2380389: Use a single vendor directory in the root Moves the vendor directory and updates the autoloading and so forth. It works fine as long as you don't have APC on.

#39 does not solve the way more usual use-case of the autoload changing due to a composer upgrade and our APCu cache needing to be invalidated.

Agreed.

I do think we need an issue to flush APCu in rebuild.php

Let's rescope and add that here, since that's what's happening.

Moved the patch in #39 to #2380389: Use a single vendor directory in the root since it solves the immediate problem there.

Where this might help is with symlink deployment.

+++ b/core/vendor/DeploymentIdentifier.php
@@ -0,0 +1,26 @@
+    return hash('sha256', __DIR__ . ':e6a3c80757f0e2a34ecc1cb9332cdab1');

I don't have a better suggestion but changing this every time we modify composer.json seems like an annoying maintenance hurdle.

I don't have a better suggestion but changing this every time we modify composer.json seems like an annoying maintenance hurdle.

The idea is that DeploymentIdentifier.php is generated when you (or a script) type composer dumpautoload (or install or update).

I think in this issue to unblock #2380389: Use a single vendor directory in the root we should use more or less the same logic as https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21DrupalKer...

All of those things are or can be available before the APCu class loader is initialized and don't rely on anything in the vendor directory which is what prompted this issue in the first place.

This will work great if you update between tagged releases.

If you are running the 8.0.x branch and git pull within the timeframe of a tagged release, you might have to update $deployment_identifier in settings.php (or restart apache, call rebuild.php etc.). If you're running dev then that's fine. Same for dev tarballs.

All we need to do here is not break sites between betas or RCs/patch releases.

Also the deployment identifier here has exactly the same name as the one in settings.php already give or take snake casing, that's not great for documentation purposes especially since they're dealing with similar goals albeit the classloader vs. the container.

Also raising priority since the vendor dir change feels like it's RC deadline.

#51 looks to address everything in #50.

Er, never mind. I guess I just don't get the scope of this issue, especially before coffee.

Committed/pushed to 8.0.x, thanks!