D7: Reset drupal static caches when a module is enabled or disabled.

Issue also applies here:
#2088375: Subscription types statics in some circumstances cause "Undefined permission" error when applying feature

Adding D7patch. All credits to JvE.

+1

This causes lots of other apparently random issues when enabling other contrib modules, like #1681414: Enabling i18n modules fail in install profile

Marking this as bug (it is one) and as major since it affects several widely used contrib modules. I'd guess it has already cost thousands of dev-hours.

I'm updating the D7 patch. IMHO the static cache must be flushed before hook_install is invoked.

@JvE: would be nicer if you also migrated all the information of the bug that you mark as duplicate.

@torotil apologies, i linked it in that issue, i should have marked it / added additional info.

I know about more modules that are affected (D7) (rules, feature up to some level, etc), might it be a better idea to create a META issue? Because all these modules need patches and we need some list to track these.

Np. Thanks for marking the duplicate. I haven't found the D8 issue when reporting the D7 issue.

… might it be a better idea to create a META issue? Because all these modules need patches and we need some list to track these.

I don't think so. For some instances of the problem there is no easy workaround. So I think this must be solved in core. Also there it would be a waste of dev-time to find every single instance where the stale cache is a problem.

Let's elaborate a bit more on possible work arounds.

At least in D7 the order of hooks called when enabling a module is: hook_install, hook_enable, hook_module_installed, hook_module_enabled. Which workaround is possible depends on when the new information is first used (wrt the hooks), and whether the module being enabled is the one doing the caching.

A common case is that some module provides a hook and caches its results. Another module implements this hook and wants to use some functionality of its dependence in hook_install() (ie. a module that defines field-type and then creates a default field(instance) for this type). This is the worst case. The only way outside of core to solve this is in hook_install() - and that in every single module that implements the hook.

So what have we got?

• It's a 2 line patch in core, that is unlikely to affect any functionality.
• It affects multiple widely used contrib modules.
• It's likely that there are numerous not-yet-found issues out there due to this bug.
• Workarounds are much harder than the actual fix.
• In general those issues are rather hard to track down.

This is 5 good reasons to get this into core ASAP. So I'd rather go for marking contrib-bugs as duplicates of this.

I agree 100%, the meta was just an idea to facilitate contrib a bit. A list of commonly used modules + the status of their issue would be handy. With this, developers can see how other modules implement their changes for this patch and site builders have some insight into the status of the projects they are using regarding this patch. (patch available, fixed in dev, released). The meta was not for core, i'm all in favour of this patch, i'm using it, that's how i started to find other modules with related issues.

The testbot will test the D7 patch against D8 HEAD - and fail, because this first needs to be fixed in D8 (the issue is marked as a D8 issue), that's another reason i believe an additional post will come in hand. It might be some time before this gets committed, so people will be running a patched core + contrib for at least a couple months/years (see the initial post date).

#26 is still current, back to needs review.

Ah right - So why don't we use this issue as tracker for all contrib-bugs triggered by this?

Sure, that will also work for now. Some can always create the page at a later time (if the list explodes).

Agree with @Rob C, we don't want contrib module issues to be posted not followed up here. I think if we expect to ever get the patch in we should keep it clean, focused and up to the point.

So we may need a meta-issue, that maybe if we mark it for D7 we can also use to post and follow up on the D7 patch. If not, we can just link other issues here using "Related issues"

Also since this may have a wider impact on installation process, it may need some performance assessment, like: How does this change the installation time for Drupal core and/or distributions (install profiles) with many modules? (We can try for D7)

Relating #2346035 as it was caused by a similar issue - in that case - was resolved by clearing a specific static. Any generic solution for D8 here would make part of the commit in that issue redundant. (http://cgit.drupalcode.org/drupal/diff/core/lib/Drupal/Core/Extension/Mo...)

This issue is impossible to be marked as "Needs review" because the testbot always retests the D7 patch (#30) on D8 instead of the patch for D8 in #25. I try "hiding" the D7 patch for now.

Stil #26 should be reviewed and committed ASAP so that we can finally fix this in D7.

This issue is impossible to be marked as "Needs review" because the testbot always retests the D7 patch

Re-post the D8 patch.

@torotil #44: i had success in other issues by
* changing issue version to 7.x and uploading 7.x patch (filename indicating 7.x/8.x gives extra points ;-)
* reverting issue version to 8.x and uploading 8.x patch

Apparently it is now possible to configure custom testing parameters per patch. I did so for the D7 patch (#25). Not sure why it won't let me do the same for #30.

Downgrading to normal, there are 47 calls to drupal_static in 8.x now and we have no known issues with module install and those caches any more. Better to refactor the code using drupal_static() at this point.

I might have found an issue with this patch, but I'm still in the testing phases. But I can say is that when I undo the patch, everything works.

Currently during my install profile, with this patch applied, I get the following warnings:

Invalid argument supplied for foreach() common.inc:7229                                                                                                                                                                                                            [warning]
array_keys() expects parameter 1 to be array, null given common.inc:7249      

Which are drupal_schema_field_types() and drupal_schema_fields_sql(). Essentially, drupal_get_schema returns empty schemas when installing new modules. If I change the calls to drupal_get_schema() in those two functions to include $rebuild = true, everything works too.
But it only happens once my install profile reaches the redirect and entityform modules, so there may be a bug elsewhere (in my install profile or in a contrib module) which is just being brought to light due to this patch.

The result of that warning is fairly severe though, because those tables aren't getting created so a bit down the line everything breaks horrendously.

Anyway, I'll keep testing, but I was wondering why drupal_get_schema's 'fallback' when it has no cache fails at picking up schemas that are in the process of being created.

edit: it's about the patch in #30 for D7

@catch: There are still 247 calls to drupal_static() in D7 out there and at least some bugs caused by it in popular contrib modules. Do you suggest opening another bug report for D7 with a higher priority?

I have been bitten by this again. As this has been almost resolved in D8/D9 by switching away from drupal_static() (#59) I’m resetting the version and priority for D7.