Drush pm-download exit with 0 even when no release is found

Related bug report on entityreference: #1459540: People seems to be able to disable a Field module which still has fields, and core seems to call field hooks on disabled modules.

I don't think that #1 is related to #0, is it? I agree that it would be a good idea to fix #0.

#1 seems to be a source of the specific error reported in #0, so I added the reference in case people arrive here searching for that error. But, yes, the Drush return value is a distinct question, so this isn't a duplicate of #1459540.

Here's a patch that could be committed to master + backported to 5.x. The mid-stream 'exit' is inelegant, but calling drush_set_error here causes problems with the Drush shutdown process, whereas this runs cleanly. Unit tests pass.

Perhaps we could commit this for now to get things working, and work on cleaning up bootstrap + error recovery in a follow-on issue later.

This looks pretty inelegant. I'm OK with committing it, but lets please do that larger refactor soon.

Committed to master and 7.x-5.x. Will work out a better resolution for Drush-6, and leave Drush-5 as it is.

c.f. Duplicate issue #1679920: drush user-add-role invalid_role gives exit code 0 when run remotely

It doesn't look like this was ever fixed for 6. Using latest bleeding-edge 8.x-6.x branch:

#!/bin/bash
drush dl token
echo $?

drush dl akhsdjkahjskdhakjsdhkjashkdad
echo $?

Yields:

Project token (7.x-1.4) downloaded to /Users/abyron/token.           [success]
0
No release history was found for the requested project               [warning]
(akhsdjkahjskdhakjsdhkjashkdad).
0

Also closed #1828744: Drush fails to return unique exit code to shell in event of [warning] as a duplicate of this issue.

#9 is different than the closed issue, but can be tracked here. The new issue is that drush dl calls drush_log instead of drush_set_error when no release is found for a requested project.

In updatexml_get_release_history_xml():

  if ($error = $xml->xpath('/error')) {
    // Don't set an error here since it stops processing during site-upgrade.
    drush_log($error[0], 'warning'); // 'DRUSH_PM_COULD_NOT_LOAD_UPDATE_FILE',
    return FALSE;
  }

So, we could fix this issue, but break pm-updatecode just by changing that drush_log to a drush_error. A more clever solution is needed. Perhaps updatexml_get_release_history_xml() could be changed to return a string in case of error, or an array of xml in case of no error. All of its clients would then need to call drush_log or drush_set_error as needed.

Assigning to myself so its not forgotten, but patches welcome if anyone else wants to pick this up.

I seem to be having this problem on all commands that produce a drush error.... I still get 0 exit code when running an unknown command, for example:

drush wtf; echo $?
The drush command 'wtf' could not be found.  Run `drush cache-clear drush` to clear the commandfile cache [error]
if you have installed new extensions.
A Drupal installation directory could not be found                                                        [error]
0

#13 did not seem to be a problem for me on the latest master.

I fixed #12 not by adjusting the API call, but by calling drush_set_error explicitly in pm-download if fetching a release failed (and the failure is not due to a user abort).

Also pushed to 6.x.

This issue was marked closed (won't fix) because Drush has moved to Github.

If desired, you may copy this bug to our Github project and then post a link here to the new issue. Please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.

s/

/