drush_make 6.x-2.0-beta5 depends on php curl libraries, should seek alternatives

In DrushMakeDownload_Get::downloadFile we are already setting up wget or curl as shell requirements to download files. The attached patch uses wget or curl in place of libcurl in php.

$ wget -qO- /Users/dmitri/basic.make
$ curl /Users/dmitri/basic.make 
curl: (3) <url> malformed

This won't work for us ATM.

The current implementation will fail the test in #2 as well. If you inject curl_error to see the error in the current drush_make_get_remote_file() you would see the error on a local file to be "<url> malformed".

The current setup sets 3 ops we should set as well so I don't think this is ready to be committed. Conceptually, what's wrong with this?

If PHP supports curl, that's faster and safer than invoking curl or wget via the shell. We should really try all of the below, in order, and only give up if none of them worked:

1. PHP natively supports cURL (e.g. if (function_exists('curl_init')))
2. PHP is configured to allow URLs in fopen -- it's insecure, but if someone's configured that way, we might as well just use file_get_contents() with the URL as the filename.
3. drush_shell_exec('wget')
4. drush_shell_exec('curl')
5. drush_shell_exec('lynx') ;) (ok, just kidding)

But seriously, we should test for #1 and #2 and use one of those before we fall back to fork/exec for #3 and #4.

amen, it should be as dww said.

also, dww.. keep in mind that the makefile parameter can also be a url

ie: drush make mydomain/profiles/drupalprofile.make

/me raises hand to code this.

Thanks, mfer! ;)

/me goes back to dealing with #649254: version_extra can't just be string sorted -- ugh

Uses the order listed in #5 and sets an error if none of them are available.

Sounds like this "needs work" then. ;)

I'd like to be able to specify local sources. However, I can't wget a local file.

I must say that to its defense, the patch actually works for systems with wget and no php5-curl.

The problem that needs fixing is then twofold:

* make sure we cleanly abort when everything is missing
* fix php5-curl only environment (which should be fairly rare, to be honest, but still need to work if we want to benefit from it)

Thanks for the patch mfer!

thinking more about this: the other thing is this may be a better situation than *without* the patch so I would suggest going ahead and committing the patch, because the two issues noticed by mig5 above are present in the current code too. It's just that there are still other occurences in the code that do not use the common download stub:

anarcat@marvin$ grep wget *
drush_make.download.inc:      // Download the project -- try wget, fall back to curl.
drush_make.download.inc:      if (drush_get_context('DRUSH_SIMULATE') || drush_shell_exec('wget %s -O %s', $url, $filename) || drush_shell_exec('curl -o %s %s', $filename, $url)) {
drush_make.project.inc:        if (!drush_shell_exec('wget %s', $url)) {
drush_make.utilities.inc:  elseif (drush_shell_exec('wget -qO- %s', $url) || drush_shell_exec('curl --silent %s', $url)) {

So in a sense, the patch is complete and fixes the problem (that we depend on curl libraries). The reverse problem exists though: we don't use them everywhere (but I consider that a separate issue).

The issue about the remote failures are another thing altogether: failure handling is non-existent in drush_make_get_remote_file(), with or without the patch, and we have a similar situation elsewhere in the code that uses wget and friends.

Finally, I wonder if that function is properly named. get_remote_file() is really get_remote_file_contents() and a new function get_remote_file() should be written to factor out the wget/curl calls used above.

Again, that should be treated separately and the patch should go in, marking RTBC.

There's a report of this not working with github, because it doesn't use the content-disposition header: #654798: Curl is stupid

Hm, we already have a pure-php http library: http://api.drupal.org/api/function/drupal_http_request/6
Maybe this should be incorporated into drush core? See also #581348: have drush make use the pm primitives

ack, didn't mean to reset status.

@grendzy - the redirect issue is separate from this one.

i agree with anarcat that this patch fixes the existing issue.

committed