Support for custom form parameters for drupal 7 was added here http://drupal.org/node/1198956 and it was mentioned that Drupal 6 would be difficult because of d6 form handling. However, I think it's possible to add limited support to the level of maybe only handling addition of some values to the install_configure_form. It's just a POST array which the drupal 6 drush installer passes into the configure form, so if you know that your install profile adds something to that form, then it should be easy enough to add the same elements to that POST array in a similar way to how the D7 version adds elements to the form values.

I may be missing something, but I will attempt to get a simple case working as time allows.

Comments

moshe weitzman’s picture

Sure, go for it. Thanks Alan.

greg.1.anderson’s picture

Beware that the d6 site install does not handle install stage transitions correctly. It mostly works for most profiles, but you may run into these bugs when dealing with more advanced profiles -- e.g. those with extra stages to handle additional forms to submit.

alan evans’s picture

This handles the simplest case of a profile that adds a couple of form fields to the install configure form. The merging of values is borrowed directly from the drupal 7 install code.

I've given it a brief test under exactly that condition - a profile that adds 2 form fields to the install configure form, and it seems to work as intended (the form value gets carried over correctly)

greg.1.anderson’s picture

Do you have a pointer to an install profile with a custom field or two on the install configure form for testing?

greg.1.anderson’s picture

Status: Active » Needs review
alan evans’s picture

StatusFileSize
new674 bytes

Here's an example profile where this might be useful - it just adds a couple of form fields to the install configure form and sets variables up with those. Obviously real profiles might do something more complex.

An example install command I'd use for this particular install profile would be:

$ drush si --db-su=root --db-su-pw='***' --db-url=mysql://alandtest:pwqioehpoiuqherp918y234pr@127.0.0.1/d6insttest example myopt1="anything" myopt2=1
You are about to create a sites/default/files directory and create a sites/default/settings.php file and CREATE  the 'd6insttest' database. Do you want to continue? (y/n): y
Starting Drupal installation. This takes a few seconds ...                                                                                                                   [ok]
Installation complete.  User name: admin  User password: kBSkmjfyyK                                                                                                          [ok]
$ drush vget myopt
myopt1: "anything"
myopt2: "1"

I have the feeling if this gets accepted, that some small changes will be needed to the help sections (which currently in a few places mentions D7 support only). Please let me know whether this is going in the right direction and any code changes you'd like to see and I'll start looking for the needed help text changes once that's settled.

moshe weitzman’s picture

Hi Alan. yes, this all looks good. Please do update the help text and then I'll commit.

For a next project, perhaps you could look at site-install and see where our test coverage is lacking. We use site-install to install drupal over and over in the tests but we obviously don't touch all the corner cases.

greg.1.anderson’s picture

It would be nice to have a failing test for install profiles that add additional phases. I think this works in d7, and fails in d6.

alan evans’s picture

Adding patch including small changes to help texts. I grepped a bit for "D7" and "drupal" and only found these strings that seem relevant. Only one small style change to the code.

I'll put testing etc. on my list - shouldn't be too long before I can get to looking at that.

I do wonder if there's a way to get multistep forms to work in D6 ... will have a think about that too but honestly I don't know how much demand there would be for supporting that.

moshe weitzman’s picture

This looks ready to me once its has a test. Thanks Alan.

alan evans’s picture

Sorry this is taking so long, the tests have me kind of stumped. Spent a lot of time working on D6 install tests yesterday and thought I had them working, then cleared the drush cache and found that the tests weren't working after all. I must have somehow got something in cache working which didn't work with a clean cache.

I'll get there, might have to rework a bit though. I think I might be relying too heavily on the built-in setupDrupal method, which is probably not entirely appropriate to testing installs.

alan evans’s picture

aaaaargh I cannot believe I did that ... the reason the test was failing was because I was passing in a random value for the integer argument in the example profile. But the example profile only defines [0-2] as allowed values.

That leads me to another issue ... drush si reports "Installation complete" on D6 sites even if validation failed on the configure form. Hooray for tests!

alan evans’s picture

StatusFileSize
new10.56 KB

Drupal 6 Site install options haven't changed since the last patch, just added some tests for a basic D6 install and for one with extra options, had to also include the example profile.

Discovered another bug along the way that was part of the cause of the earlier confusion: drush si on drupal 6 reports installation succeeded, even if the configure form failed validation.

moshe weitzman’s picture

Status: Needs review » Needs work
// D6 installation requires that the db_url is in the settings file. Putting
+    // it on the command line isn't enough.

Thats news to me. Are you sure?

I think we should just get rid of testSetupDrupal() and just do testExtraConfigurationOptions(). We can remove the call to setupBeforeClass in that case. Removing the testSetupDrupal() speeds us up a bit as well.

Lets put randomString method into Drush_TestCase instead of in this file.

Otherwise, looks RTBC

moshe weitzman’s picture

Status: Needs work » Patch (to be ported)

Committed to master.

I fixed up a bunch of sql and archive code in order to cope archive/restore/install of a D6 site with no defined DB. Thats what the test has here. I think those fixes were sufficiently intrusive that I'm not likely to backport the test. We could conceivably backport the feature.

moshe weitzman’s picture

Added change notice: http://drupal.org/node/1407156

alan evans’s picture

Thanks for finishing this one up.

msonnabaum’s picture

Status: Patch (to be ported) » Fixed

Skipping backport since 5 is out.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.