I've got a make file that's loading a profile (from git) That make file looks like this:
core = 6.x
api = 2
projects[drupal][type] = "core"
projects[myprojectname][type] = profile
projects[myprojectname][download][type] = git
projects[myprojectname][download][url] = git@github.com:username/myrepo.git
I'm running the make file with the command:
drush make --working-copy mymakefile.make
The profile also comes with it's own make file. The make file from the profile is auto-detected as expected, but it contains some "git" download types and the --working-copy option isn't being picked up from my original drush make command. So, my git projects (beyond the one in the example code above) are being cloned, but not as working-copies... Hope that makes sense...
I went through some of the drush code (for the first time) and added a few custom logs to see what was going on. I can confirm that the working-copy option is being detected by the first make file, but not the second. It looks like maybe the setting gets lost in the recurse() function? Something to do with the $context changing? Sorry, I didn't have a whole lot of time to really look into it... also, I'm still not sure if it's a bug or if I'm just messing it up somehow so I thought I'd post an issue and see if anyone else is having the same issue... or if I missed something.
Thanks!!
Comments
Comment #1
drclaw commentedI did some digging and it looks like my original options for drush make get lost when make_projects() is called on the first recursion. Specifically when drush_redispatch_get_options() is called to get the common options. It receives most of the options but are missing the ones specific to drush make. I examined the contexts at that point and found that the original options had been moved from the cli context into the stdin context... I managed to get it to work by merging the stdin options with the cli_options in drush_redispatch_get_options() but I'm not sure what kind of effect this has overall.
Anyway, just thought I'd post my findings in case it helps.
Thanks again!
Comment #2
moshe weitzman commentedWhen you declare an option, you can state what happens on redispatch. Not sure I have seen command options do this, but global options definately do: http://api.drush.ws/api/function/drush_get_global_options/master
Comment #3
moshe weitzman commentedPerhaps Greg can describe the best way to fix this. I can probably do the implementation.
Comment #4
greg.1.anderson commentedWhen a Drush command uses drush_invoke_process to call another Drush command, the cli options do in fact end up in the stdin context if the backend invoke method POST is used. If the redispatched command ends up calling yet another command via drush_invoke_process, then drush_redispatch_get_options will end up mostly empty. The solution suggested in #1 is certainly safe, but there is some question about backwards compatibility if we make this change. It's probably okay to just merge these contexts, as the stdin context is empty when Drush is called from the command line. The downside of this solution, though, is that script-provided options such as --backend and --invoke can no longer be distinguished from user-supplied options once they've passed through drush_redispatch_get_options once. This might not be as harsh as it sounds, as this distinction is also lost in backend invoke's GET method, which is the default. It is kind of rare for commands to use POST; Aegir and Make do it to pass through complex options (i.e. to preserve arrays and nested array structures).
A more complicated change would be to modify the code path in backend invoke that POSTs options to identify which options came from the cli, and which came from some other source, and also make the other end unravel this additional info and put the options back in the contexts they belong in (probably limited only to stdin and cli). This could be done in a backwards-compatible way by adding an item to the POST data that specifies which of the stdin options should instead be placed in the cli context. If this item is missing, then everything will end up in the stdin context, just as it does today.
Overall, though, I think it's sufficient to just merge the stdin and cli contexts in drush_redispatch_get_options() and leave it at that. It depends on how strict you want to be about backwards-compatibility for this feature that's just an edge case for most clients. Should test with Aegir first, I guess, but they have not finished their Drush 5 support yet, so maybe it's safe to move forward.
Comment #5
moshe weitzman commentedI'm a little hesitant to merge in drush_redispatch_get_options(). I think make_projects() can handle adding back in the cli options itself. I just committed the patch at http://drupalcode.org/project/drush.git/commitdiff/9729bd3fe7a9f88a2e0dd.... Maybe Greg can think of a better way. It feels a little odd to just slam in the new options, given the careful logic we just did in drush_redispatch_get_options().
@drclaw - thanks for the deep research here. Please stick around and submit patches when you see an improvement opportunity in Drush. Your talents are wanted! Also, please report back whether this commit fixes the bug.
Comment #6
greg.1.anderson commentedI think #5 is fine. The only improvement I can think of would be to make a wrapper function -- a version of drush_redispatch_get_options designed for use by commands that are themselves the target of a redispatch.
Comment #7
drclaw commentedHey guys,
Thanks for taking a look at this! I can confirm that the change in the patch works. Awesome!
Glad you found this helpful. I was worried that I was going to be way off base on the fix, but I guess it was close. =P I'll be sure to post again if something else comes up.
PS Oh yeah, I guess it goes without saying "Thanks for this module!" I spent 5 months without it while I was travelling (netbook, pc, too lazy to install) and I forgot how much easier life is with drush. =)