To summarize: during major version upgrades, if a module introduces a new table, it's not created until the update is run; but the standard way to upgrade modules is to first re-enable the module, then run updates. Because module enablement calls a cache clear, it attempts to clear a non-existent table and dies in a very ugly way.

This is effectively a duplicate of a core bug #1477932: Allow cache() and cache_clear_all() to fail gracefully if a database cache bin does not exist, but it might be nice to implement a stopgap in the meantime.

Patch attached.

CommentFileSizeAuthor
#5 libraries-major_version_upgrade_cache-1578618-5.patch473 bytesiamEAP
#2 libraries-major_version_upgrade_cache-1578618-2.patch514 bytesiamEAP
libraries-major_version_upgrade_cache-0-0.patch353 bytesiamEAP
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

We should definitely include a comment referencing the core issue.
Otherwise this looks good.

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented
FileSize
libraries-major_version_upgrade_cache-1578618-2.patch514 bytes

With comments.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

I tried writing upgrade tests for this, but that didn't work. At least I couldn't get them to fail with the status quo. Apparently SimpleTest additionally clears some caches at the right place, which update.php doesn't.

I'll try to commit this in the next few days. I think the comment can be shortened. Simply

// @todo Remove this
// @see http://drupal.org/node/123456

should be fine.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Shorten, yes. But at minimum a concrete and concise hint/summary for why a strange workaround is needed. That hint should be concise for the actual workaround, not a summary for the more generic problem space.

E.g.:

// @todo When upgrading from 1.x, update.php attempts to flush caches
//   before the cache table has been created.
// @see http://drupal.org/node/123456

Thus, in the not so unlikely case that this core bug still exists when Libraries API goes 3.x (or alternatively, when no one uses 1.x anymore), then this kind of comment sufficiently clarifies that the workaround can be safely removed, even though the generic Drupal core bug still exists.

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented
FileSize
libraries-major_version_upgrade_cache-1578618-5.patch473 bytes

More concise summary in comment.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Fixed 
+++ b/libraries.module
@@ -9,7 +9,11 @@
+  // @see http://drupal.org/node/123456

You obviously wanted to replace that with a real URL...

Anyway, took the comment from #4 and fixed the issue URL, and committed to 7.x-2.x.

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented

Heh. Whoops. Thanks, sun.

Status: Fixed » Closed (fixed)

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