Problem/Motivation

The fix in #3031128: Update from 8.6.7 to 8.6.8 warnings - Drupal\Core\Extension\Extension has no unserializer is partial for Drush 8. This is because Drush 8 calls update_check_incompatibility() after certain static caches are set in for extension lists.

To test:

  1. Install Drupal 8.6.7
  2. Update codebase to Drupal 8.6.9
  3. Run drush updb (using version 8.1.18)
  4. Visit admin/reports/status

Proposed resolution

Also clear the profile extension list static cache in UpdateKernel::fixSerializedExtensionObjects()

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#16 3031740-16.patch7.25 KBalexpott
#16 13-16-interdiff.txt1.32 KBalexpott
#13 3031740-13.patch7.51 KBalexpott
#13 3031740-13.test-only.patch2.08 KBalexpott
#13 5-13-interdiff.txt5.6 KBalexpott
#7 drush8-fix-install-profile.php_.txt2.66 KBalexpott
#5 3031740-5.patch2.18 KBalexpott
#5 2-5-interdiff.txt1.09 KBalexpott
#2 3031740-2.patch1.09 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3031740-2.patch1.09 KB
tmanhollan’s picture

tmanhollan commented

This patch does avoid the problem on the status report screen initially, but after a cache rebuild, the behavior I described in #3031128-67: Update from 8.6.7 to 8.6.8 warnings - Drupal\Core\Extension\Extension has no unserializer still occurs. I get the two notices from system_requirements() and the line on the status report that lists the active installation profile shows the machine name of the profile with the details missing.

I'm digging into this a little deeper, so I'll report more details soon. This sounds like the same behavior reported by @John Pitcairn in #3031128-76: Update from 8.6.7 to 8.6.8 warnings - Drupal\Core\Extension\Extension has no unserializer.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

So #3 is reproducible using drush 8. Here are the steps:

  1. Install minimal using drupal 8.6.7
  2. Log in as user 1
  3. Update code base to drupal 8.6.9 and apply this patch
  4. Visit admin/reports/status
  5. Clear cache using drush
  6. Vist admin/reports/status

The problem is caused by the fact that the install profile gets removed from the list of installed modules. I'm pretty certain this is due to the very wrong and odd code found in update_fix_compatibility() that removes things from core.extension - see #2917600: update_fix_compatibility() puts sites into unrecoverable state for more on why what this function is doing with core.extension is wrong.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2-5-interdiff.txt1.09 KB
new 3031740-5.patch2.18 KB

The patch attached fixes #4 for the Drush 8 update path. So the question is what to do for sites that have upgraded with Drush 8 and now get these warnings. These sites have had their install profile removed from core.extension and that's potentially very bad.

tmanhollan’s picture

tmanhollan commented

Thanks, @alexpott. Confirmed the patch in #5 fixes all of the problems I've experienced. I caught the condition on dev and never updated anything permanent yet, so it's not an issue for me. But I agree that anyone who did update already and had the extension removed from the list has a potentially bad situation on their hands.

I don't know enough about the inner workings here to offer any specifics about how to proceed. But I noticed in my tests that when the profile was missing from core.extension, the status report screen still listed the proper profile's machine name. That was because drupal_get_profile() returned the proper extension, but system_get_info('module', $profile) returned an empty array. At least in my case, nothing was removed from the filesystem, so there shouldn't actually be anything missing. Not sure why drupal_get_profile() has it, but perhaps that could be used to rebuild whatever is missing.

I'm out of time for the day, but I can dig around there more tomorrow. If there are any additional specs from my setup that would be helpful to you, let me know.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new drush8-fix-install-profile.php_.txt2.66 KB

Here's a script that can fix a site after Drush 8 and update_fix_compatibility() have had their way. If we're going to fix it in a hook_update_N this is the kind of thing we're going to need to do.

You can run the script using drush scr drush8-fix-install-profile.php.txt

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Related issues: +#3031128: Update from 8.6.7 to 8.6.8 warnings - Drupal\Core\Extension\Extension has no unserializer
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Priority: Normal » Critical

Given that Drush 8 and update_fix_compatibility() are causing an irreversible data loss (without running https://www.drupal.org/files/issues/2019-02-09/drush8-fix-install-profil... script) unfortunately I think we have to make this a critical issue. If the install profile contains modules then this data loss could completely break your site.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Drush 8 and the new Extension class » Drush 8 and update_fix_compatibility() cause data loss

Updating issue title to reflect bug-iness and criticality.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Drush 8 and update_fix_compatibility() cause data loss » Updating to 8.6.8 or 8.6.9 with Drush 8 causes data loss via update_fix_compatibility()
tmanhollan’s picture

tmanhollan commented

#7 corrects the problem as I experience it when updating without the patch from #5. And updating with the patch from #5 avoids the problem entirely. I tested both the patch and the script against a clean install and against a real project and both appear to be effective resolutions of the problem.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 5-13-interdiff.txt5.6 KB
new 3031740-13.test-only.patch2.08 KB
new 3031740-13.patch7.51 KB

Here's a patch that includes an update function to fix broken sites.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
  1. +++ b/core/modules/system/system.install
@@ -2173,3 +2174,73 @@ function system_update_8501() {
+    // @todo Decide if we should do:
+    //   $extension_config->set('profile', '')->save(TRUE);
+    return t('The %install_profile configured in core.extension is not available.', ['%install_profile' => $install_profile]);

    We have to decide whether we fix this case. Note that this would not be cuased by the Drush 8 update path problem so /shrug - but we know from other issues that this is a problem on certain sites. /me goes digging for node IDs. 

  2. +++ b/core/modules/system/system.install
@@ -2173,3 +2174,73 @@ function system_update_8501() {
+  // @todo I don't think that it is necessary to rebuild everything here because
+  //   a cache clear happens at the end of hook_update_N processing.
+  // drupal_flush_all_caches();

    I think we don't need to do this. Other hook_update_N's should not make assumptions about the cache state either.

  3. For me the biggest question is what about a site where re-enabling a profile might cause other issues - i.e. it wasn't enabled prior to the Drush 8 update path problems because someone manually hacked the installed module list to remove it.

The last submitted patch, 13: 3031740-13.test-only.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Related issues: +#1170362: Install profile is disabled for lots of different reasons and core doesn't allow for that, +#3032139: Remove the install profile from the core.extension:module value
StatusFileSize
new 13-16-interdiff.txt1.32 KB
new 3031740-16.patch7.25 KB

Discussed with @catch a bit. Who said

#14.1 I think we should leave it but make sure there’s a proper issue about it. #14.2 probably leave it out yes. #14.3 nfi

On #14.1 and #14.3 we already have #1170362: Install profile is disabled for lots of different reasons and core doesn't allow for that which as discussed many solutions to this problem. @catch pointed out that have both the profile key and it listed in the module value is problematic. I agree and have opened an issue to discuss removing profile from core.extension:module so we don't have to continue trying to maintain this see #3032139: Remove the install profile from the core.extension:module value

We also discussed if we could detect if Drush 8 was involved but neither of us was comfortable with the fallout of doing that.

alexpott credited catch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
catch’s picture

catch
he/him
Primary language English
commented

Happy with #16, but let's get more eyes on this since it'll be our second hotfix release in two weeks.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue tags: +Regression, +SA-CORE-2019-002 regression

Tagging this because it's tied to the ongoing saga of fixing SA-CORE-2019-002.

tmanhollan’s picture

tmanhollan commented

#16 works for me. I don't have any input on your concerns from #14, but your evaluation seems reasonable.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: -SA-CORE-2019-002 regression

@DamienMcKenna: This has nothing to do with SA-CORE-2019-002, it is a problem that was introduced in 8.6.8, a regular non-security update. SA-CORE-2019-002 was 8.6.6.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Bumping to RTBC, could still do with an extra set of eyes since it's not clear whether #20-22 reviewed the patch.

tmanhollan’s picture

tmanhollan commented

Sorry, I should have elaborated. My comments in #21 were made after testing patch #16. I agree, though it wouldn't hurt for others to review it.

Here are the details of my setup and tests. I'm using composer for the code, drush 8.1.18 for database updates, my test environment uses a lando drupal8 recipe with php 7.2.

First pass: Update without the patch to produce the corruption in the profile, then test that hook_update_n fixes it.

  1. Clean install using custom profile at 8.6.7
  2. Log in as user 1, view status report screen
  3. Update to 8.6.9
  4. drush updb
  5. Observe error on status report screen ("TypeError: Argument 1 ... ")
  6. drush cr
  7. Status report screen loads, but with notices ("Undefined index: ... in system_requirements() ... ") and missing installation profile
  8. Apply patch from #16
  9. drush updb
  10. Observe successful execution of system_update_8601
  11. Status report screen loads without problems and installation profile is restored

Second pass: Direct upgrade with patch to test that it avoids the problem

  1. Clean install using custom profile at 8.6.7
  2. Log in as user 1, view status report screen
  3. Update to 8.6.9 + patch from #16
  4. drush updb
  5. Observe successful execution of system_update_8601
  6. Status report screen loads normally, including the proper installation profile
collinhaines’s picture

collinhaines commented

Upgraded to 8.6.9 from 8.6.7 with Drush 8.1.17 and received warnings and errors mentioned in this thread.

Applied the patch from #16 and drush updb ran successfully.

I will note that I did receive Warning: Invalid argument supplied for foreach() in Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDerivatives() when viewing the status report screen immediately after database update but I believe this to be related to a different issue not caused here.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the additional reviews. Committed and pushed 9340bf9755 to 8.7.x and 5e2c514e63 to 8.6.x. Thanks!

  • catch committed 9340bf9 on 8.7.x 
    Issue #3031740 by alexpott, tmanhollan, catch, collinhaines: Updating to...

  • catch committed 5e2c514 on 8.6.x 
    Issue #3031740 by alexpott, tmanhollan, catch, collinhaines: Updating to...
peterhebert’s picture

peterhebert commented

So was commit 5e2c514 included in the 8.6.9 release? or did it get applied only to the 8.6.x dev branch? Wondering if I should wait for an 8.6.10 release to update my sites...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@peterhebert if you are using Drush 8 definitely wait for 8.6.10

tmanhollan’s picture

tmanhollan commented

Thanks to all for resolving this.

phernand42’s picture

phernand42 commented

#16 worked for me as well.

Should have probably waited a bit before applying update since we are still on Drush 8 ourselves. Glad to know there will be a fix in 8.6.10

ARRC-Drupal-Chick’s picture

ARRC-Drupal-Chick commented

I have tried to update to 8.6.9 from 8.6.7 twice today. Both times after replacing the files and running update.php, I get:
Error: Call to undefined method Drupal\Core\Update\UpdateKernel::fixSerializedExtensionObjects() in <site directory>\core\includes\update.inc

The first time I tried from 8.6.7 direct to 8.6.9. Then, thinking there was something in 8.6.8 that might have fixed the issue, I restored to 8.6.7 and did 8.6.8, all good, then 8.6.9 again and it failed.

I cannot use Drush.

My environment (which is somewhat unique):

  • Windows 10
  • Apache 2.4.33 (Win64)
  • PHP 7.2.5
  • MS SQL Server 2012

I have had no issues with this configuration until 8.6.9. We started with Drupal 7 in 2015.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@ARRC-Drupal-Chick thanks for the information. The error you're getting sounds like somehow the codebase got out-sync UpdateKernel::fixSerializedExtensionObjects() exists in both 8.6.8 and 8.6.9 and if you are not using Drush 8 then everything should be working. If it is not then more information would be fantastic.

ARRC-Drupal-Chick’s picture

ARRC-Drupal-Chick commented

@alexpott, sounds about right. Thank you.

After restoring the database and 8.6.7 code, I was able to 'fix' the problem by doing a fresh cache clear just before updating the code and executing update.php.

That appears to have resolved the problem.

Sometimes cache clearing fixes a problem and sometimes, when something strange happens, running update.php (even if you haven't updated anything) clears up issues.

So, I was able to update all 7 non-production sites to Drupal 8.6.9 and am ready for the production one with a slick script to do it all quickly.

  • xjm committed f7c27eb on 8.6.x 
    Revert "Issue #3031740 by alexpott, tmanhollan, catch, collinhaines:...
xjm’s picture

xjm
she/her
Primary language English
commented

This has been temporarily reverted from 8.6.x only but will be recommitted before the next bugfix release.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Fixed » Reviewed & tested by the community
bzrudi71’s picture

bzrudi71 commented

Hmmh, this is very confusing ;)

This has been temporarily reverted from 8.6.x only but will be recommitted before the next bugfix release.

Wonder what this does mean for todays security release? We are on 8.6.7 waiting for 8.6.10 because of @alexpotts note in #30:

... if you are using Drush 8 definitely wait for 8.6.10

So will we be able to run the security update coming from 8.6.7 or will there be patches avail?

Thanks!

vermario’s picture

vermario commented

It would be really good to know if today's security update is still compatible with drush8, because switching is a bigger task than just applying the update.

pavlosdan’s picture

pavlosdan
he/him/his
Primary language Greek
Location Vancouver
commented

Same questions as #39 and #40.

pavlosdan’s picture

pavlosdan
he/him/his
Primary language Greek
Location Vancouver
commented

edit for clarity and to avoid confusion
For those concerned on how to get a security release as a patch without upgrading:

Asked in the #security-team in slack and got the suggestion to get a patch from https://cgit.drupalcode.org/drupal/log/ -> pick specific commit -> click *patch*.
Thanks @pingwin4eg.

As per @catch's comment below we shouldn't need to do this.

catch’s picture

catch
he/him
Primary language English
commented

The revert was just to make the release process smoother, drush 8 users shouldn't need to take any additional steps to update.

xjm’s picture

xjm
she/her
Primary language English
commented

To clarify, this was reverted so that the fix can be included in today's release (because of specifics of how core releases are tagged). The security release today will include this fix and that it why we had to remove it from 8.6.x HEAD for a few hours.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

This is included as part of the commit for https://www.drupal.org/sa-core-2019-003 to ensure that sites can update to 8.6.10 safely. Marking back to fixed. (Issue credit is still granted.)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.