Improve git executable check

 mode change 100755 => 100644 drush
 mode change 100755 => 100644 drush.php

These two files need to be executable. Right?

Powered by Dreditor.

It is questionable whether drush.php needs to be executable. It does not need to be if you use the drush script, or if you use php drush.php status. It does need to be executable if you ever call it directly via drush.php status. I don't know that we need to support that any more, but I suppose it does not hurt.

I'm testing the patch. I think we need also a fix for update actions.

ps. execute permissions should be a separate issue. I'll not commit those changes here. Note also examples/helloworld.script is executable.

Its probably a good idea to have "filemode = false" in ~/.gitconfig so we don't get patches that change file modes.

Yes, but drush script needs to be executable.

Tested this out and it works great. Considering Drush git_drupalorg handler is currently completely broken even for the dl command I went ahead and committed the latest patch to HEAD - feel free to post further review/refinements here (I didn't test with upc yet). I think we should also backport this to Drush 4.x.

I've also been testing this and it works so far for me. Committed to 4.x.

Cool. I haven't tested it myself yet, but setting to 'fixed' per #10 and #11. Thanks.

Thanks for this patch it works well on my test, but I did found it unneccessary to use 'executable --version' for checking if it's available, the proper way is to use the hash utility.

drush # time git --version
git version 1.7.4.1

real    0m0.002s
user    0m0.001s
sys     0m0.001s
drush # time hash git     

real    0m0.000s
user    0m0.000s
sys     0m0.000s

As you can see the advantage is crystal clear, that .002s can be used to think of that cool new function for your module :)

Hash may be faster, but I'm going to suggest that we not switch to this technique for detecting git (although it may be useful elsewhere in drush).

As I recently noticed on some of my older systems, git 1.6.x says only "Unable to clone project xyzzy from git.drupal.org." Maybe we should spend even a couple more us and actually parse the version, and make sure that it is 1.7 or later.

Here you go, see what can be improved on it. This patch includes what I added before plus the version checking for >=1.7 on git executable, I still think we should leave 'hash' to detect executable but 'git --version' works too but not needed since we just want to check if executable is available not check version, we do that after :D

what about windows compatibility?

Agree; awk '{print$3}' should be done in php.

What is this windows you speak off? :D

Sorry for the noise, had to fix my grammar :D

Code looks good.

* hash is a sh/bash/dash/? subcommand. I see it is not available in csh/tcsh/?. Surely neither in windows.
* there's still a unchanged curl --version in the code.
* in the case of git we don't profit the gain in milliseconds as it also needs a call to --version. I don't see the interest for such a gain when the command is not found.

Maybe we should take just the git version check, and skip hash. It would be possible to make a wrapper drush_executable_exists, but it seems that if we took the trouble to do that, it should also provide a service to require a minimum version.

Perhaps the minimum version could be passed in as an argument with a default value of FALSE or NULL; if not Windows and no minimum version supplied, then use hash, otherwise use fn --version.

I say stick with --version all round for now, 2ms is really not too bad. I am working on a sqlite wrapper for convenient local RW storage for Drush, which could be used to cache these results, which should give us both simple non redundant cross-platform code, and high performance.

@jonhattan the unchanged --version for curl needs to stay there because some older versions of curl return 1 while newer ones don't, hence the dual call to 'curl --version'.

Attached is an updated patch that switches back to 'app --version'. The reason for the dual 'git --version' is that drush_shell_exec returns only the first number for 'git --version' instead of '1.7.4.1' in my case when exploding into an array. Perhaps this needs to be looked at later.

drush_shell_exec returns TRUE to indicate success. Call drush_shell_exec_output() to get the data that was written to STDOUT.

here you go.

Committed to master. Thanks.

I'm confused. Do we know what will/won't work with git clients older than 1.7 with drush?

I'd hate to introduce this dependency in drush 4 unless we have pretty specific reasons. If its just the --branch option, we could always clone it like normal and then just checkout that branch afterwards.

I confirmed that drush dl anymodule fails on 1.6.x with an inscrutable error message without this patch. I did not try to fix it, though; if anyone wants to try to maintain old versions of git, we could re-open...

Since no one else has asked for this in drush 4, and 5 is already out, I'm considering it not worth backporting.