The enclosed patch allows you to specify --use-existing to use an existing Drupal site at the specified --root. This will allow you to run 'drush make' instead of 'drush dl drupal' to generate your quick-drupal site.

With this patch, I am aiming at supporting the following syntax:

drush qd --make=mymakefile.make ...

I intend to do this with a pre-core-quick-drupal hoo that tests for the --make option, and then sets --use-existing and --root options appropriately before falling through to the regular qd implementation. This way, drush make integration can live in drush make. I don't expect that --use-existing would often be used by hand; maybe it could even be marked 'hidden'.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greg.1.anderson’s picture

Status: Needs review » Needs work

Comments still welcome, but I'm going to adjust this to better work with the pre-hook; I will post both the updated patch and the hook when ready.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
4.12 KB

The command hook that could go in drush make is shown below; the new patch is attached. Some minor work is still needed. The next question, though, is whether it would be better to separate the code like this, or just role drush make support into drush core, and fail at runtime if drush make is not available when --make is used?

function mycommand_drush_help_alter($command) {
  if ($command['command'] == 'core-quick-drupal') {
    $command['options']['make'] = 'Makefile to use. Makefile must specify which version of Drupal core to build.';
    $command['examples']['drush qd --make=mysite.make'] = 'Create and install a site from a makefile.';
  }
}

function drush_mycommand_pre_core_quick_drupal() {
  $makefile = drush_get_option('make');
  if (!empty($makefile)) {
    $root = drush_get_option(array('root', 'r'), getcwd() . '/' . basename(basename($makefile, '.bootstrap'), '.make') . '-drupal');
    // Invoke 'drush make $makefile'.
    $result = drush_invoke_process(array('root' => '', 'uri' => ''), 'make', array($makefile, $root), array('debug' => TRUE));
    if ($result['error_status'] != 0) {
      return drush_set_error('DRUSH_QD_MAKE', 'Could not make; aborting.');
    }
    drush_set_option('root', $root);
    drush_set_option('use-existing', TRUE);
  }
}
moshe weitzman’s picture

Neat feature. I like the implementation that you have shown. Hopefully drush make would accept it into their project.

greg.1.anderson’s picture

Status: Needs review » Fixed

Committed #2 in master; will continue with --make hook in drush_make queue.

Status: Fixed » Closed (fixed)

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

greg.1.anderson’s picture

Assigned: Unassigned » greg.1.anderson
Status: Closed (fixed) » Active

Now that Drush make is in Drush core, I need to circle back and put in the second part of #2.

rickmanelius’s picture

Hey @greg.1.anderson. This would be absolutely awesome... would the change be difficult to implement or is it essentially a copy/paste from #2 into the appropriate area?

greg.1.anderson’s picture

I have not looked at this in a while, but yes, the help alter stuff would just go directly in the qd command definition, and I think that you could just use the pre-hook in #2 as is, if you just renamed it to be part of the qd commandfile. You could also just paste the relevant code into the right part of the main function for qd, but some minor adjustments might be necessary.

rickmanelius’s picture

Almost there... there is some logic to test and we might want to use a subdirectory to store Drupal's docroot and sqlite db... but this is at least operational.

The only issue is #1542186: PHP 5.4 "Illegal string offset" warning when install with respect to Drupal 7.15 failing to install via the quick drupal command on php 5.4.4.

Let me know if there are any obvious tweaks on this and I can adjust.

greg.1.anderson’s picture

Status: Active » Needs work

Looks pretty good, although I have not tried it yet. As a point of style, since you have integrated the code into the drush_core_quick_drupal() function, it would be better to refactor to avoid drush_set_option('root', $root); and drush_set_option('use-existing', TRUE); (assign variable earlier & adjust them as needed).

rickmanelius’s picture

Hi @greg.1.anderson. Thanks for the feedback! After sitting with it some more, I realized that my brute force approach didn't leverage some of the code in the case where an use-existing wasn't being utilized. So I placed it in the same code block as !use-existing and arranged the variables into an order that will work for standard qd (e.g. downloading a fresh copy) OR when a makefile exists.

See attached. Comments welcome!

rickmanelius’s picture

Status: Needs work » Needs review

Forgot to set status...

kid_icarus’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
461 bytes
3.19 KB

What a killer feature! Works as expected, I tested with --use-existing, --root, --use-name, and it all worked as expected. In my opinion #11 did a great job at addressing #10. The good ol' code sniffer picked up one little thing:

+++ b/commands/core/core.drush.inc
@@ -831,13 +833,23 @@ function drush_core_quick_drupal() {
+    else{

There needs to be a space between else and {

Attached is a cleanup of that, I'm going to go ahead and mark this as rtbc :)

rickmanelius’s picture

Sweet pickles. Thanks @kid_icarus!

moshe weitzman’s picture

Status: Reviewed & tested by the community » Needs work

Looks good. Only two minor nits:

  1. Hard codes array('debug' => TRUE) in call to make. We should pass along value of original process. Not sure how we typically do that.
  2. The new option declaration have an array value and implement example-value so the help is a bit clearer.
greg.1.anderson’s picture

FileSize
5.96 KB

Here is an updated patch that takes care of the points in #15. The beginnings of a test case that does not pass yet is also included.

I wonder, if the makefile specified by the --makefile option does -not- include an installation profile, should qd try to enable the modules that were built? Presumably, the makefile usually (?) will have an installation profile, so if you tell qd to use the appropriate profile, it will enable the correct modules. For quick makefiles with no profile, though, it might be handy to have the modules auto-enabled. Not sure how hard that is.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
9.04 KB

Test is now complete & working, and projects downloaded via a --makefile are now enabled, just as projects downloaded via commandline args are (modules named after project are enabled, use --enable to explicitly name modules to enable). Since I changed some stuff I thought I'd pass through 'needs review' again prior to committing.

rickmanelius’s picture

Status: Needs review » Reviewed & tested by the community

Hi greg.1.anderson.

I just ran it with a simple makefile code and it worked as expected. There is an issue with the patch not being applied, but thats separate issue altogether. Thanks!

; This file was auto-generated by drush make
core = 7.x

api = 2
projects[drupal][version] = "7.x"


; Modules
projects[addressfield][version] = "1.0-beta3"

projects[admin_menu][version] = "3.0-rc3"

projects[ctools][version] = "1.2"

projects[devel][version] = "1.3"

projects[entity][version] = "1.0-rc3"

projects[omega_tools][version] = "3.0-rc4"

projects[panels][version] = "3.3"

projects[rules][version] = "2.2"

projects[views][version] = "3.4"

; Themes
projects[adaptivetheme][version] = "3.0"

projects[omega][version] = "3.1"

projects[zen][version] = "3.1"

;projects[drupal][patch][] = "http://drupal.org/files/illegal_string_offset-1542186-7.patch"
greg.1.anderson’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks, all.

moshe weitzman’s picture

Status: Fixed » Patch (to be ported)

Assume we want to backport this.

greg.1.anderson’s picture

Status: Patch (to be ported) » Fixed

Backported. Thanks, git cherry-pick.

Status: Fixed » Closed (fixed)

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

Jackinloadup’s picture

Status: Closed (fixed) » Needs work
+++ b/commands/core/core.drush.incundefined
@@ -834,18 +837,38 @@ function drush_core_quick_drupal() {
+      $result = drush_invoke_process('@none', 'make', array($makefile, $root));

I don't believe Drush is passing -v (verbose) to drush make.

User story

drush quick-drupal-core was stalling and -v wasn't being clear on what it was stalling on. It stated "Command dispatch completed" as the last item.

Using drush make with -v allowed me to see that there was a library that was hanging (due to ckeditor's servers being down)

Amazing work! Thank you so much!

Testing out install profiles is so much easier with this tool.

greg.1.anderson’s picture

Status: Needs work » Fixed

Glad to hear you solved your problem.

Status: Fixed » Closed (fixed)

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