Drush pm and make commands don't properly handle project status url

This patch doesn't make much sense to me. So, Drush will be able to correctly find the source for non-drupal.org projects that you are downloading -- but only if they have already been downloaded into the current site? drush_get_extensions() is really slow; it doesn't seem to be worth it.

This code is supposed to run at pm-update time? The concept seems good, but I'm not sure that the submitted patch does what it is intended to do.

Do you have any test data to run this with?

I don't think the scenario in #6 is very useful, because we don't have the status url available unless the project already exists in the site. The pm-update use case would be very helpful, though; since you have updatexml for your releases, it should be possible for this to work.

You said this is supported in Drupal core, though; if the change in #6 is made, will Drupal's module update status page show new releases published at the custom project status url? I haven't tested, but it seems that if Drupal supports this, a lot of the operations required for Drush updates should already be handled.

I think the patch presented here can be committed if pm-update also works. #2044487: drush_get_projects() does not properly load project status url in rare cases also looks good, so I am in favor of committing both of these, once they have been tested. I'll try to squeeze in time to do that, but maybe another maintainer will get to it first.

While I like the idea and direction here, I'm still reluctant to add the overhead of drush_get_extensions(), which loads information about all extensions, just to get some info on just one project. Also, note that this technique won't work if the project you are downloading is not enabled. Maybe that's not an issue for the use case, but I think that the performance hit is still too high.

pm-download already finds the destination where it is going to save the project. Perhaps you can use this directory to deterine if there is already an info file present, and if so, parse it with _drush_drupal_parse_info_file(). That would be much faster.

I didn't look at the code to see if this variable is available early enough, or if it would be easy enough to refactor the code so that it was, if necessary. I'd guess this should be possible, though.

I think we need to commit #2044487: drush_get_projects() does not properly load project status url in rare cases first; then we can return here and see if there is a better way to find the .info file than drush_get_extensions().

I didn't notice that you merged these issues together. Re-opening.

From https://drupal.org/node/2044487#comment-7658845:

I tried modifying ctools' .info file as instructed..., but could not see a 1.4 version, either on the Drupal modules page, or via pm-updatestatus with #1 applied. Clearing the cache and checking for updates manually (in the modules page in the web ui) did not help. Using Drupal 7.22. What else needs to be done?

I still need an answer to this before I can do any further testing here.

I tried again with #13. pm-download and pm-releases work fine, but I still cannot get pm-updatestatus or pm-updatecode to work.

I tried again on another Drupal site, and pm-updatestatus / pm-updatecode worked fine.

This is all very excellent, except I still don't like the fact that pm-download will end up calling drush_get_extensions() via request_info_url(), and I don't really like that you stored this large structure into a static variable.

The later issue is minor, but is also easy to fix. Note that pm-enable does the following, at the end of its validate function:

  drush_set_context('PM_ENABLE_EXTENSION_INFO', $extension_info);
  drush_set_context('PM_ENABLE_MODULES', $modules);
  drush_set_context('PM_ENABLE_THEMES', $themes);

You could do something similar (with just the extensions) in request_info_url(), or perhaps better yet, make drush_get_extensions() cache in a context, so any client may repeatedly call drush_get_extensions(), and only take a performance hit on the first call. Note that by caching in a context instead of a static, clients can clear the cache by setting the context to an empty array. This is also sometimes done via a boolean parameter $refresh (e.g. in the case of an implementation that uses a static variable, to clear it).

I'm going to leave it up to another maintainer to decide whether any of this warrants further work, or if it should just be committed as-is, with additional performance enhancements to be handled in a future issue.

Good work on this.

Assign to jonhattan for review

pm-download and other pm- commands accepts a non-drupal.org status url (or update xml) via --source option. So drush dl ctools --source=http://www.quietcompanymusic.com/drupal/release-history/ctools/7.x works, after applying the patch in #2044487: drush_get_projects() does not properly load project status url in rare cases.

Also, if a project has a custom "status url", pm-update uses it through drupal's update.module.

So the request here is to allow pm-download to use the status url of an already downloaded module.
I'm planning to introduce caching to drush_get_extensions() since I've found some problems while debugging #1989202: Infinite loop when trying to enable not available extensions and it doesn't work consistently across Drupal versions. #13 should wait till then.

An alternative to #13 would be to hook in pm-download and pass in source option if argument matches. It could be even a handy documentation example.

Just to through one more step in that list, I'm working on #2004058: Allow default update URL to be configured in drush profile settings to match Drupal core update options and thought I'd get some feedback here (and some direction).

• Allow drush users to override anything with the --source option (already working),
• If no explicit --source is provided, and we're in a context where project info is available, check for a status URL (what this issue is about),
• If no status URL is set (or that info isn't available in the current context), assume the default updates.drupal.org location (already working).

I'd propose to have a new third step where drush checks if an override for the default location has been set. Similarly to how you can change the update_fetch_url variable in drupal.

So we would have

• Allow drush users to override anything with the --source option (already working),
• If no explicit --source is provided, and we're in a context where project info is available, check for a status URL (what this issue is about),
• If no status URL is set (or that info isn't available in the current context), check for an override to the default url (not yet working)
• If no status URL is set (or that info isn't available in the current context), assume the default updates.drupal.org location (already working).

You can see in the referenced issue where I'm at, but I don't really know where it would be ideal to check for the override.

Could #23 be done with a command-specific --source option?

$command_specific['pm-updatecode'] = array(
  'source' => 'http://my.custom.location/path',
);

Specifying --source on the commandline will override the command-specific option. You'd have to set both 'pm-updatecode' and 'pm-update'.

Well, yes, but if I am understanding correctly, what we have (modified version from #13) is:

$request['status url'] = drush_get_option('source', release_info_url($name));

I think that in #24, you are suggesting that we add logic to release_info_url() to replace RELEASE_INFO_DEFAULT_URL with drush_get_option('update_fetch_url') if it is specified. However, I think that the result of this is that --source and --update_fetch_url (should be --update-fetch-url, I think) do exactly the same thing -- so I don't think that the later is actually necessary.

Fixed issue in drush upc/up.

StatusInfoDrush.php:93

  $request = pm_parse_request($project_name, NULL, $project_name);

should be:

StatusInfoDrush.php:93

  $request = pm_parse_request($project_name, $project['status url'], $project_name);

Please review