When you try to install the latest D7 HEAD via the command line (e.g., via the Drush wrapper or your own custom script) as of today you get this:
DatabaseConnectionNotDefinedException: The specified database connection is not defined: default in Database::openConnection() (line 1513 of /home/droth/web/drupal/includes/database/database.inc).
This appears to be an unintended consequence of #756762: AJAX should follow same rules for whether to call drupal_rebuild_form() as non-AJAX submissions. What happens is that the installer uses drupal_form_submit() to submit the database configuration form, and this now hits a call to drupal_theme_initialize() which tries to access the database before it's ready.
I believe the attached patch is a correct fix. The previous install code avoided initializing the maintenance theme for command-line installations based on the idea that it shouldn't be necessary, but that assumption is probably not correct. It ought to be harmless to initialize it regardless of whether we need it, and that fixes the bug.
Comment | File | Size | Author |
---|---|---|---|
#11 | drupal.theme-suggestions.11.patch | 3.88 KB | sun |
#10 | drupal.theme-suggestions.10.patch | 3.97 KB | sun |
#4 | drupal.form-programmed-theme.4.patch | 2.65 KB | sun |
#3 | drupal.form-programmed-theme.3.patch | 2.65 KB | sun |
command-line-installer-maintenance-theme.patch | 1.24 KB | David_Rothstein | |
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedBefore anyone asks - yes, it is probably theoretically possible to write a test for this :) But it might not be easy: #630446: Allow SimpleTest to test the non-interactive installer
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commenteddrupal_form_submit() always calls into the theme? That seems wrong. Or is this form special in some way? I have no problem with the proposed change but I'd like to understand what we've done.
Comment #3
sunThat's an unintended and unexpected change. Actually not introduced through the stated issue, but #757154: Base form_id via hook_forms() not taken into account for #validate, #submit, hook_form_FORMID_alter()
@moshe: We need to handle #theme suggestions somehow. In that issue, I already proposed that, if #theme is an array, and none of the suggestions is found, we need to fall back to element_children rendering. Fixing that would also fix this issue (as we'd no longer have to initialize the theme in the first place).
Attached patch is a stop-gap fix only.
Comment #4
sunerr. Of course, the opposite ;)
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commented@sun - theme suggestions have to do with form rendering, not form submission. it is a bit dodgy that drupal_prepare_form() needs to initialize theme at all. it should be able to unconditionally set #theme_wrappers = array($form_name, 'form') and be done with it ... i'm not too familiar with this code though.
Comment #6
sunok, to clarify:
Hope this clarifies it a bit. Happy to clarify more, if needed. :)
Comment #7
moshe weitzman CreditAttribution: moshe weitzman commentedThats a good summary of this theme code which indeed has been there since 4.7. But I think all this themeing in drupal_prepare_form() is awkward. In D8 we should consider treating forms like everything else in the $page array. That is, if you want to theme a form you have to:
1. register a theme function/template
2. set #theme during hook_form_alter (this is new)
With that, we can rip all the themeing out of drupal_prepare_form()
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedYup, the coupling between the form API and the theme system - via the call to drupal_theme_initialize() or equivalent in form.inc - has indeed been around a long, long time.... I'll trust Moshe on the dates :)
However, my original post is correct: #756762: AJAX should follow same rules for whether to call drupal_rebuild_form() as non-AJAX submissions is the commit that broke the command-line installation (I checked by rolling back the patch). It looks like the reason is that it moved that theme initialization code from drupal_build_form() to drupal_prepare_form(), thereby putting it in the path of programmatic form submissions.
I don't think the patch in #4 is correct, because the $form array gets passed all over the place (for example, to alter hooks), so we don't want $form to be different in the case of programmatic form submissions and non-programmatic ones... that could lead to unexpected results (e.g. if the alteration functions ever checked the value of #theme and acted based on it). Perhaps that is what @sun meant by a stop-gap fix only.
Anyway, if we could get that theme initialization code back into drupal_build_form() somehow that would be great, but I'm not sure we can do that easily or have to do that here -- it sounds like a non-critical followup. I think my original patch should be included at least as part of anything we commit here; the reason is that even if the form API winds up not initializing the theme system, some code that gets triggered somewhere else in the installer could easily do it. Always using the maintenance theme prevents that problem more generally.
Comment #9
sunTechnically, I'd just like to remove that watchdog() from theme() in case it doesn't find any of the specified suggestions. The watchdog() can be kept if the specified $hook is a string, i.e., when we're asked for a particular theme function and that function does not exist. But if theme() gets an array of suggestions, it should not bail out in the first place, since suggestions are just suggestions.
Doing that would allow us to remove the entire theme initialization from drupal_prepare_form(), and it would only apply automated suggestions in case #theme is not already set.
--
Aside from that, given David's reasoning in #8, I think that his patch is additionally a good idea to do -- although I fear it may slow down non-interactive installers.
Comment #10
sunThis is what I mean.
Comment #11
sunBut obviously, I got the order of #theme suggestions in drupal_prepare_form() wrong.
Comment #12
moshe weitzman CreditAttribution: moshe weitzman commentedNow thats what I'm talking about! Very nice untangling of theme and form APIs, and fixes the bug at hand.
Comment #13
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.