Command line (Drush) install fails on SQLite ($form_state['input'] is not always populated for programmatic form submissions)

Now why would the test bot ignore that?

Since I think this is related to the form API, I'm tentatively moving it there (at least to get opinions from people who look at that queue).

In practice, I think this is mainly an SQLite issue, since I found that even if you have multiple drivers, you can still use Drush to install with the first one (which is MySQL)... But if you try SQLite, what happens is that the MySQL form validation still runs, and since that is so different from the expected SQLite form validation, it doesn't work.

I think the fundamental problem is related to this code in install_settings_form():

  $form['actions']['save'] = array(
    '#type' => 'submit',
    '#value' => st('Save and continue'),
    '#limit_validation_errors' => array(
      array('driver'),
      array(isset($form_state['input']['driver']) ? $form_state['input']['driver'] : current($drivers_keys)),
    ),
    '#submit' => array('install_settings_form_submit'),
  );

The goal there is to limit the form validation to the database driver that is actually selected. But for programmatic form submissions, two problems seem to arise:

• $form_state['input'] isn't set, so it always winds up using the first driver
• Even if we fix that, #limit_validation_errors still doesn't seem to work because the form API code looks for it it in $form_state['triggering_element']['#limit_validation_errors'], but $form_state['triggering_element'] doesn't seem to get set for programmatic form submissions either.

So my question for FAPI gurus is... where is the bug? :) For programmatic form submissions done with drupal_form_submit(), is the caller supposed to fill in these obscure parts of $form_state themselves if they want limit validation errors to work, or shouldn't we try to make form API have a sensible default behavior here? (I see that there is already some special code around to allow this kind of thing to work with AJAX form submissions, but not programmatic ones.)

I guess the testbot didn't run because the status wasn't "needs review"?

Please review this! -> #218066: Prevent cross posting from reverting metadata fields

form_state['triggering_element'] doesn't seem to get set for programmatic form submissions either. <= huhwhat?

  $process_input = empty($element['#disabled']) && ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access'])));
    if ($process_input) {
      if (_form_button_was_clicked($element, $form_state)) {
        $form_state['triggering_element'] = $element;

so wtf. (_form_button_was_clicked does not look at programmed)

This patch isn't helping me on the current HEAD. It's just changing the error.

For what it's worth, this used to work, as of a couple of weeks ago. I know, because I wrote the patches for Drush to be able to support site-install (and just about everything else) with SQLite. As far as I can tell, Drush is still handling everything fine on its end; it's only after passing the values into install_drupal() that things are going sideways.

I have deleted Garrett's loooong error and attached here.

After doing some line-by-line debugging with MacGDBp (always my favorite part of programming), I've determined that this is in fact a Drush bug. Check out this issue in Drush's queue for more. Unsure of the state of the original patch now… perhaps the fix to Drush will make it redundant.

Was a drush issue. Namely, I was not setting 'op' => dt('Save and continue'), on each of the forms so the button that carries the #limit_validation_errors was not in form_state[#input] (David noted this symptom in #2).

I don't know why this started failing now though.

The latest dev version of Drush is still not working for me with SQLite - the MySQL validation is being run instead.

It's surprising that having Drush set the 'op' changed anything for you (was that with pgsql rather than sqlite by the way?) because there is only one button on the form. So it shouldn't be necessary to set it.

I'm pretty sure this is a core bug. I'm looking into it now.

OK, I think I figured it out. It turns out two things were (and still are) broken, both core and Drush.

The Drush bug was silly - I can't believe I didn't notice it originally. I'll post a followup patch for it at #983914: Drush site install command no longer works for Drupal 7 after this one.

The bug in core is just that programmatic form submissions aren't being treated the same way as regular form submissions, in terms of $form_state['input'] and button click detection. Strictly speaking, if we leave the 'op' stuff Moshe added to Drush, we only need the first hunk of this core patch, not the second one. But the second seems pretty sensible to me. If the special button click detection code needs to be there for Internet Explorer, we really might as well run it on programmatic form submissions too. (Basically, we should never let a form be processed without identifying a clicked button, and certainly a lot of people using drupal_form_submit() aren't going to remember to pass in 'op' every time, so doing this helps avoid a lot of weird edge-case bugs.)

With this patch to core and the other patch to Drush, I can successfully use Drush to install with SQLite.

I guess in theory this could use a test, but let's get some reviews first.

Both hunks look quite sensible. Thanks for tracking this down. If committed, I will remove the 'op' stuff from drush site-install.

Let's not do the second hunk for Drupal 7. It's a fairly big change, might lead to unexpected results. The first hunk is good but the comment is not. You need input first only because drupal_build_form does it that way and so the form builder function expects it that way.

OK, I removed the second hunk. It can be a separate followup issue. (It doesn't seem like a big change though, because I don't see how it's legitimate to write a programmatic form submission that relies on Drupal thinking the form was submitted without any of its buttons being clicked - that seems a little nonsensical.)

Also modified the code comment a bit to clarify it and mention the drupal_build_form() connection.

ok, we have consensus ... wait for green

Can we get some tests for this, please?

Let's try this.

After cvs upping both Drush and Drupal and applying this patch, I'm seemingly able to do a Drush install. Trying to actually access the site results in spinning until eventual timeout, with nothing being dumped to the PHP error log; not sure if that's an unrelated issue… We seem to be moving in the right direction, at least.

EDIT: It finally dumped some stuff into the PHP error log. It appears that variable_initalize() is calling itself recursively infinitely until the PHP maximum nested function limit is hit. Hmm. Will look into this in a bit and see if it's related.

As far as I can see , we are good to go.

Sounds like we're waiting on results from #22?

That sounds like an unrelated issue - I don't see how it could be related to this patch.

(Probably it comes from having the SQLite database file not writable by the webserver? That's easy to do if you did the install via Drush, because Drush typically runs as a different user. I'm pretty sure I saw something like that occur at one point.)

Agree - is unrelated.

Committed to HEAD. Thanks!

I moved the part of the patch that wasn't committed here to #1008644: Programmatic form submissions should always get a triggering element, just like regular form submissions do

The final patch didn't wind up having anything to do with #limit_validation_errors per se.

Sorry for the lack of follow-up with regards to #22 - work and the holidays (*cough* and Minecraft *cough*) kept me away from my issues for a bit. But yes, as it turns out, I had forgotten to check that the SQLite database file was writable by the web server. After correcting that, things are working correctly again.

Thanks a lot to all the incredibly smart and unquestionably attractive people in both the Drupal and Drush issue queues who have helped vastly improve the Drupal/SQLite experience in these last few weeks.