While inverstigating #464556: Installer requirements check doesn't appear we found that #310467: Slimmer hook_theme() had broken the installer because drupal_function_exists was unable to include a file with a required theme function because we where in MAINTENANCE_MODE.
The attached patch provides a possible solution to this(as well as an alternate fix for #464556: Installer requirements check doesn't appear) by providing a hook modules can use to setup a basic registry during MAINTENANCE_MODE.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | maint_files.patch | 1.5 KB | chx |
| #17 | maint_files.patch | 1.5 KB | chx |
| #16 | maint_files.patch | 1.5 KB | chx |
| #13 | maint_files.patch | 1.35 KB | chx |
| #4 | 464624_registry_alter.patch | 3.77 KB | neclimdul |
Comments
Comment #1
Anonymous (not verified) commentedadding registry tag.
Comment #2
webchickHm. I'm not sure about this...
It's introducing a registry hook. But only for SOME functions, not all of them. And worse, the way that you register functions here is entirely different than how you register functions in things like hook_theme(), which normally is the only place you need to registr theme functions. I thought ftheme_... was a typo, but it turns out that's how you have to do it. :\
I'm trying to envision what the API docs for hook_maintenance_registry() looks like that does not involve lots and lots of head-scratching, and am having a really hard time...
Comment #3
webchickMind you, I'm also not a huge fan of "just move it to theme.maintenance.inc!" proposed at #464556: Installer requirements check doesn't appear since that obviously won't work for contrib. But I wonder if there's a way we can do something like this without needing to expose module developers to the inner-workings of the registry.
Comment #4
neclimdulI got a similar sentiment from chx on irc. I started working toward a more complicated hook and it just got more and more of a mess the more I tried to make it developer friendly.
So, I've tried a bit of a different approach. It also ends up providing additional functionality to people really interested in breaking^w changing things with a full blow alter hook on the registry. To better support this I've expanded the lookup cache to be split out into a structure instead of the compressed key previously used that caused some confusion.
Comment #5
neclimdulPS - chx suggested the original hook provide a list of files that will always be included in main mode. I think that will probably end up being its own mess with confusion arising around which maint mode we're in and if the db is available. This might still exists a little in the current patch but its also should be something that can be managed at the alter level.
Comment #7
Anonymous (not verified) commentedlooks like a typo.
this will get called on every cache miss, which could be expensive. is there a way to do this once per registry build, or at least, once per request?
Comment #8
webchickOk. Call me wacky, but why don't we just include_once 'modules/system/system.admin.inc' before the theme function call in install.php?
Comment #9
neclimdulBah, aparently the interaction with this and the db system is pretty hairy because it happens when drupal_alter isn't available sometimes and other times when registered modules aren't available so the alter actually fails calling functions that don't currently exist...
I'm not really sure how to deal with this ATM.
Comment #10
johnalbinsubscribe
Comment #11
moshe weitzman commentedI'm not seeing a great solution here either. I think we should fix the installer quickly in #464556: Installer requirements check doesn't appear. this issue is gonna take a while.
Comment #12
catchThis looks like a pain. I remembered it'd already been discussed before somewhat in http://drupal.org/node/332303#comment-1143991 although that's a very far reaching solution (even if it's just backing up the registry).
Comment #13
chx commentedSo what was the problem with the files hook?
Comment #14
webchickWell, the first problem is:
Fatal error: Call to undefined function drupal_get_path() in /Users/webchick/Sites/core/modules/system/system.module on line 2347
Comment #16
chx commentedPffft, i meant theoretically, not the actual implementation, that's a miniscule detail.
Comment #17
chx commentedComment #18
Anonymous (not verified) commentedthis looks good, i'm happy with the idea. one typo:
should be 'included' not 'incldude'.
Comment #19
chx commentedComment #20
Anonymous (not verified) commentedpatched my install with the #19, no more notice on the requirements page. RTBC.
Comment #21
webchickMinor things:
Let's get trailing ()s there so that they hot-link on api.drupal.org.
Non-minor things:
Documentation needs work. Let's flesh out the use case for this hook a bit (under what circumstances does Drupal get into maintenance mode?) and why you might need to use it (providing a theme function which must be called during installation is one. are there others?).
Comment #22
webchickComment #23
sunchx pointed me here. I'll have a look this.
Comment #24
sunSince this issue can only be replicated with the patch in #310467-26: Slimmer hook_theme(), I've posted a new patch over there (different approach though). As mentioned over there, I need to know whether I should go back to chx's hook approach.
Comment #25
sunLet's move this over there?