Make downloads the wrong tarball for profiles

pm-download accepts a --variant option which is a switch for core/no-core/profile-only. Perhaps make should be setting that option.

We had the same exact problem. Thankfully I searched before submitting the issue.

@moshe, are you suggesting that we would specify the variant in the makefile, or that make itself should be smart enough to not download the core version if a profile is specified in a make file?

I would think that make command could specify it with no makefile change needed. I'm not 100% familiar with the problem tho so kinda shooting from the hip.

Given the above makefile, I think drush did the proper thing which was to download the distribution i.e. the core file. However, with the following makefile it should recognize that the download request is for the profile and download the profile-only version. Currently it still downloads the core file.

api = 2.x
core = 7.x
projects[drupal] = 7.12

projects[openpublish][type] = profile
projects[openpublish] = 1.0-alpha6

The resulting drush make

drush make openpublish-site.make /tmp/testing        
drupal-7.12 downloaded from http://ftp.drupal.org/files/projects/drupal-7.12.tar.gz                                                                                   [ok]
openpublish-7.x-1.0-alpha6 downloaded from http://ftp.drupal.org/files/projects/openpublish-7.x-1.0-alpha6-core.tar.gz      

I'm hoping this can get some attention before the final release of 5.0.

No, even in the first case, it's doing the wrong thing. The .make file in the original post already specified core:

projects[drupal] = 7.12

If you just want a tarball of the full distribution including core, download the tarball, don't use drush make. ;) If you're referring to a profile in a .make file, it's because you're already building core yourself or you're building with --no-core. Either way, we *never* want drush make to use the full distribution tarball for a profile that's referenced in a .make file. I don't think there needs to be any magic conditional logic. We just want to ensure that if we're dealing with a profile *at all* that we use the profile-only variant.

That said, yes, this should definitely be fixed before 5.0. Bumping priority and tagging accordingly. Also, this is relevant to the drupal.org distribution initiative, so tagging for that, too.

Okay, this patch seems to work fine. However, there are a few problems:

A) The recursion.make test is now failing. I assume that's because the test's md5 is assuming we're getting all of core in there. ;) Haven't looked closely, but I think it's that the expected output is buggy and now that we're getting the right thing, the test fails. This needs investigation and probably just a fix to the test.

B) Sadly, there's no good way to do this cleanly since the download stuff in drush make doesn't have enough info. make_download_pm() only gets these parameters: $name, $download, $download_location. So, there's no way to know if we're dealing with a profile, module, or what. I've run into this before. A deeper refactoring is obviously out of scope (although it'd sure be nice if we were just passing in everything we know instead of just the 'download' chunk of the .make file for the project we're building). Thankfully, --variant only matters if you're dealing with a profile, and pm-download happily ignores it if you're not dealing with a profile, so I think this is safe (enough). But, it'd be nice to clean this up some day.

C) Something is lying about what it's doing. ;) When I run the .make file from the OP with this patch applied, I'm seeing this in the output:

openpublish-7.x-1.0-alpha5 downloaded from                                  [ok]
http://ftp.drupal.org/files/projects/openpublish-7.x-1.0-alpha5-core.tar.gz

However, I verified it's *not* using the -core.tar.gz version. When I run drush dl directly, I'm just seeing this:

% drush pm-download openpublish --variant=profile-only
Project openpublish (7.x-1.0-alpha6) downloaded to                     [success]
/.../openpublish.
Project openpublish contains:                                          [success]
 - 1 profile: openpublish
 - 2 themes: openpublish_starterkit, frame
 - 14 modules: phase2_profile, openpublish_site_page,

So, it doesn't *seem* like this is a pm bug, but rather something in drush make itself. Haven't investigated more closely yet.

But, I wanted to at least get an initial patch up for review in case anyone wants to run with or look into any of these issues.

Thanks,
-Derek

Patch #6 worked for me. Oddly enough it reported downloading the -core file.

# drush make openpublish-site.make /tmp/testing 
drupal-7.12 downloaded from http://ftp.drupal.org/files/projects/drupal-7.12.tar.gz                                                   [ok]
openpublish-7.x-1.0-alpha6 downloaded from http://ftp.drupal.org/files/projects/openpublish-7.x-1.0-alpha6-core.tar.gz                [ok]
Found makefile: openpublish.make                                                                                                      [ok]
entity_autocomplete-7.x-1.0-beta1 downloaded from http://ftp.drupal.org/files/projects/entity_autocomplete-7.x-1.0-beta1.tar.gz       [ok]
 >> Project feeds contains 4 modules: feeds_ui, feeds_news, feeds_import, feeds.

At least it now found the profile's makefile and continued the installation as expected. Thanks for looking into this. Oh, and I do see your point about always wanting drush make to download the profile.

'variant' is a good solution for now. The wrong error message is printed by drush_make a line below the call to pm-download:

 47     drush_log(dt('@project downloaded from @url', array('@project' => $full_project_version, '@url' =>                       $download['download_link'])), 'ok');

'download_link' is obtained previously in make and is not needed at all for pm-download to operate. We need pm-download to return this data somehow.

Seems like a good solution to me. Will wait a day for more feedback ... Perhaps we just omit the @url from the log message for now (see #8)

Great, glad you like it.

However, we can't commit this until we fix the test I mention in #6A. And yeah, I guess we could just omit the URL to fix #6C, but it'd be nice to keep that if we had a good way to get that info back from PM download. Maybe that'll have to wait for Drush 6, I dunno.

Also, I'm not going to be able to re-roll this today, so I don't want the fact it's assigned to me to confuse anyone. ;) If anyone can run with this, please do.

Thanks,
-Derek

Actually, I went ahead and committed this with an updated MD5 hash for the recursion.make test. I also removed the wrong url from the log message along with a note to bring back a right one in the future.

Sweet, thanks!