Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Looks like this was introduced when fixing #182675: Normal page caching disables hook_boot() and hook_exit().
#17 in that issue also points out that the loading of module.inc was moved. Since it's not needed for a page served from the cache when in aggressive mode, this patch also fixes that (=> should give minor performance improvment. Note that 5.x similarly has module.inc included just before http://api.drupal.org/api/function/bootstrap_invoke_all/5 in http://api.drupal.org/api/function/_drupal_cache_init/5).
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | 323474-hook-boot-D7-followup_0.patch | 1.84 KB | gpk |
| #22 | 323474-hook-boot-D7_1.patch | 6.57 KB | catch |
| #21 | 323474-hook-boot-D7.patch | 5.4 KB | Dave Reid |
| #18 | 323474-hook-boot-D7.patch | 1.06 KB | Dave Reid |
| #18 | 323474-hook-boot-D6.patch | 1.19 KB | Dave Reid |
Comments
Comment #1
gpk CreditAttribution: gpk commentedOh yes there's a patch here ;)
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #3
gpk CreditAttribution: gpk commentedI assume the test system doesn't like the CRs I seem to get running diff on Win?
Trying again..
Bug also present in 6.x BTW.
Comment #4
gpk CreditAttribution: gpk commentedComment #5
Dave ReidI can confirm this behavior. If cache is set aggressive and a user is logged in, hook_boot() will not fire but hook_exit() will. Note, this patch also skips loading the './includes/module.inc' if the page has been cached.
Patches attached for D6 and D7 revised from #3 to not change the comment about loading the page cache.
Comment #6
gpk CreditAttribution: gpk commented@5: Thanks for reinstating the comment before the module.inc line, omitted in error. Very minor point but not sure why you've reverted the comment about loading the page cache?
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #8
Dave ReidSigh...that's the not last submitted patch.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #10
Dave ReidComon testing bot...let's not fight. If anyone from testing.d.org reads, please see why the patch from #5 is not considered 'the last test' and is not being tested?
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #12
catch-d6.patch shouldn't get tested. -d7.patch probably should, but I wonder if it's getting caught by the same regexp. Easy way to find out.
Comment #13
catchComment #14
gpk CreditAttribution: gpk commented#12 now showing in the queue http://testing.drupal.org/pifr/node/1/323474. So maybe it did get caught by the regex.
Comment #15
hunmonk CreditAttribution: hunmonk commentedd.o experienced some database issues earlier today, and i'm guessing 323474-hook-boot-d7.patch in #5 got lost in the fray.
i've re-tested our regex, and it does allow -d7.patch extensions to be submitted for testing, so chalk this one up to a freak accident.
Comment #17
Dave ReidFailed due to #74645: modify file_scan_directory to include a regex for the nomask.. Setting back to code needs review.
Comment #18
Dave ReidReposting patches (D7 had a small offset) so I can get a green light from testing.d.org and hopefully get this committed soon. It's an easy fix.
Comment #19
catchCould we write a simpletest for this - enable aggressive caching, make sure cache is clear, module which implements hook_boot, watch everything blow up?
Comment #20
Dave ReidSure thing...let me get one written quick. Will repost patch with test.
Comment #21
Dave ReidThere we are! Simple little test included that passes 100% (and would have 1 failure without the patch). Note that the D6 patch is still valid in #18.
Comment #22
catchPatch looks good, tests pass. Re-rolled only with extraneous phpdoc removed + a line added for the new class. No credit on commit please.
Comment #23
Dries CreditAttribution: Dries commentedI've committed this to CVS HEAD. I assume that the DRUPAL-6 patch should also go in still?
Comment #24
gpk CreditAttribution: gpk commented@23: Yes, #18 still good for D6.
Attached followup patch corrects a garbled comment in the tests, plus a couple of other doc tweaks in the tests.. temporarily resetting version..
(@21: Cool, I was wondering how this could be tested.)
[update: patch removed]
Comment #25
gpk CreditAttribution: gpk commentedProper patch this time..
Comment #26
webchickDoc cleanup committed to D7.
Moving back to D6 for consideration of #18.
Comment #27
Dave ReidPatch in #18 still applies to latest 6.x and is ready to be committed.
Comment #28
Gábor HojtsyCommitted to Drupal 6.x as well.