Drush was unable to download the Colorbox plugin to                  [error]
sites/all/libraries

This should be working, but it isn't and I don't see why. I tried playing around with the drush script, but it just doesn't seem to execute the download part

  // Download the zip archive
  if (!drush_shell_exec('wget '. COLORBOX_DOWNLOAD_URI)) {
    drush_shell_exec('curl -O '. COLORBOX_DOWNLOAD_URI);
  }

I leave this one for frjo since he created the script.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

frjo’s picture

I have only tested the Drush command on Mac OS X and Debian GNU/Linux.

What OS/Distro are you using when you see this problem?

jdwfly’s picture

Ubuntu 9.04 and Drush 3.0.

I am running it as my user as normal and I have write privileges to the directories. I can't think of any other reasons why it doesn't work.

frjo’s picture

Please run the Drush command with the debug flag.

drush --debug colorbox-plugin

That should give you some good hints why it's not working.

Do you have wget or curl installed? Or both?

jdwfly’s picture

I'll give that a shot and I do have both wget and curl installed. I can manually download it using wget or curl like the script does.

jdwfly’s picture

Title: drush script error » drush script error - add documentation on requirements
Component: Code » Documentation

unzip wasn't installed. It definitely won't work without that being installed. Probably should have some documentation added somewhere that states requirements for the drush script. By default unzip isn't installed in a Ubuntu server.

frjo’s picture

Version: 6.x-1.0-beta3 » 6.x-1.x-dev
Category: bug » feature
FileSize
674 bytes

What do you think of adding a dependency check in the command itself?

Could you check if this works as it should in stock Ubuntu?

hutch’s picture

Patch inserts fine. I'm not sure what 'stock ubuntu' is, I recently did an install of ubuntu 10.4 and as part of the install (expert mode) you get some options, which type of server you want eg ssh server, web server, database server, samba server and so on. I think it is reasonable to expect a sysadmin to ensure that certain packages are there such as wget, curl, rsync, ssh server and client, zip, unzip, bzip2, gzip, make, patch, perl etc etc. Ubuntu 10.4 server installed more by default than Debian 5 netinstall, my usual platform but you should check irrespective of distro.

Having said that I think the patch should go in anyway. I would haved used 'which unzip' but it may well be that 'type -P unzip' (which gives identical results) is more cross-platform. If anyone knows more speak up!

Both work on ubuntu, debian and OSX, that's all I have access to ATM, I will find out how the BSDs handle this from a friend.

EDIT:
Both work on FreeBSD and will likely work on openBSD and netBSD. Both commands appear to be generic bash. Other shells may/will do things differently, you can't cover them all.

frjo’s picture

Thanks for testing this so thoroughly. I have committed it to 6-dev now.

I prefer to use "type" since it's a Bash built in command and not an external process like which.

hutch’s picture

My BSD friend says
The "type" command is built into shells other than BASH (it's definitely in CSH).

So 'type' is probably the most likely to work in different shells.

I've not really looked at drush include scripts before, interesting.

frjo’s picture

Status: Active » Fixed

Drush has a good API making it very easy to add commands like "colorbox-plugin". Got the inspiration from the "solr phpclient" command.

aufumy’s picture

Status: Fixed » Needs work
FileSize
675 bytes

I am not able to use this, eventhough unzip is installed.

I had to reverse the operator. It may be that on success drush_shell_exec returns 0?

frjo’s picture

aufumy, what environment are you running Drush in?

On success drush_shell_exec('type -P unzip') returns 1 on my systems (Debian/Linux and Mac OS X/BSD).

aufumy’s picture

I am using Ubuntu:

Linux FOO 2.6.32-22-generic #36-Ubuntu SMP Thu Jun 3 22:02:19 UTC 2010 i686 GNU/Linux

According to the drush inline docs (in includes/drush.inc):

 *   0 if success.
frjo’s picture

If you execute "type -P unzip" on the command line what result do you get?

The documentation in drush.inc seems to need correction.

    …
    // Exit code 0 means success.
    return ($result == 0);
  }
  else {
    return 0;
  }
hutch’s picture

There is a difference between the way a *nix shell program and PHP usually return their exit code.

Most shell programs return an exit code of 0 to indicate success and 'type' is no exception. Failure would return a non-zero exit code, 1 in the case of 'type', which also prints the path to STDIN on success. In a shell script you can collect the exit code (with $?) and check that the command succeeded.

Here is a snip of shell script:

url=" http://example.com/index.html"
wget -q $url
RET=$?
if [ $RET -ne "0" ]; then
  echo "wget failed"
fi

Most PHP functions are designed to return TRUE or 1 (or some content) on success and FALSE (which is empty or 0) on failure.

I haven't tested it but
drush_shell_exec('type -P unzip')
would return 1 or TRUE or the path to unzip, it's a PHP function

Hope this clarifies things a bit

heavy_engineer’s picture

Hi, I also have this double negative thing going on - ubuntu 10.4, Drupal 6.16, colorbox latest dev. If i change line 19 from

  if (!drush_shell_exec('type -P unzip')) {
  return drush_set_error(dt('Missing dependency: unzip. Install it before using this command.'));
  }

to (notice its now checking if drush_shell_exec returned anything other than a zero)

  if (drush_shell_exec('type -P unzip')) {
    return drush_set_error(dt('Missing dependency: unzip. Install it before using this command.'));
  }

drush_shell_exec returns a 0 according to http://drupalcontrib.org/api/function/drush_shell_exec - although looking at the code for drush_shell_exec, i fail to see how it can return 1 (might be my unfamiliarity with the module). But it looks to me that the drush function cant return anything but zero (line 410 drush/includes/drush.inc)

  // Exit code 0 means success.
    return ($result == 0);
  }
  else {
    return 0;
  }

Personally i'd expect it to work how hutch described it. I've quickly searched on the drush issue queue for drush_shell_exec but cant find anything. I'll dig around and see if im just not reading it right before opening an issue on the drush queue.

hutch’s picture

Looking at the code for function drush_shell_exec()

drush_shell_exec() returns TRUE on success, return ($result == 0); would resolve to TRUE or 1
if $result is 0. $result is the exit code from exec() which in turn gives the exit code of the *nix command

so

if (!drush_shell_exec('type -P unzip'))

is correct

frjo’s picture

steev_initsix and aufumy, if you execute "type -P unzip" on the command line what result do you get?

jdwfly’s picture

I don't seem to have this issue (double negative) but with Ubuntu 9.04 I get this from the command-line...

jdwfly@server:/$ type -P unzip
/usr/bin/unzip
aufumy’s picture

frjo, sorry for the delay, I get:

/usr/bin/unzip

frjo’s picture

There seems to be a difficult to track down problem with the check for unzip.

What do you think about simply removing it? Instead we could add something like this to the final error message for faild downloads.

"Drush was unable to download the Colorbox plugin to @path. Make sure you have wget or curl and unzip installed."

To the README we can add.

You need to have the following installed for this command to work:

* wget or curl
* unzip

aufumy’s picture

replacing

if (!drush_shell_exec('type -P unzip')) {

with

if (!drush_shell_exec('type unzip')) {

works for me

I added some debug statements to drush_shell_exec(), and when using 'type -P unzip', the $result would be 127 instead of 0. Also the foreach ($output as $line) would show
-P: not found
unzip is /usr/bin/unzip

whereas with 'type unzip', the $result would be 0. The foreach ($output as $line) would show just
unzip is /usr/bin/unzip

frjo’s picture

Status: Needs work » Needs review

I have committed aufumys suggestion in #22 to 6-dev, simply removing "-P" from the type command.

frjo’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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