This is a followup to the issue #586466: Drush.php is no longer directly executable (can interfere with backend invoke). It's a different problem that affects both 2.1 and HEAD. It is because we assume that $_ is a PHP variable that means "the php executable". That is wrong: it's a bash-specific variable that is quite unreliable that generally means "the last command executed".

I suggest we completely drop the $_ parsing, I have started working on a patch to do just that. We should assume that in the general case, "php" is in the path and will just work (which means that we can invoke $argv0 directly, in most cases).

CommentFileSizeAuthor
#7 637574_dash.patch5.68 KBanarcat
#5 637574_dash.patch5.92 KBanarcat
#4 637574_dash.patch5.3 KBanarcat
#1 637574_dash.patch5.3 KBanarcat

Comments

anarcat’s picture

anarcat commented
Status: Active » Needs review
StatusFileSize
new 637574_dash.patch5.3 KB

Here's a patch that brings back --php to deal with particular cases. In general, we assume php is in the path and executable.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

I am getting an immediate error:

Fatal error: Call to undefined function drush_get_option() in /Users/mw/contributions/modules/drush/includes/environment.inc on line 953

anarcat’s picture

anarcat commented

Hum, you're right, the patch is missing a chunk where the include order is modified in drush.php. I'll send this in on monday, sorry!

anarcat’s picture

anarcat commented
Status: Needs work » Needs review
StatusFileSize
new 637574_dash.patch5.3 KB

Here's a more adequate patch.

anarcat’s picture

anarcat commented
StatusFileSize
new 637574_dash.patch5.92 KB

Geez, I uploaded the exact same patch... Here's a proper one, *again*. Sorry for the trouble.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

define('DRUSH_COMMAND', drush_find_drush()) now appears twice in environment.inc. The one in global scope is not needed, right?

anarcat’s picture

anarcat commented
Status: Needs work » Needs review
StatusFileSize
new 637574_dash.patch5.68 KB

sorry about this again, I think that was the original idea: that the bootstrap code initialise the variable, so we don't need to reorder the includes.

Here's a new patch, again. I can't believe I fail at providing a so simple thing. :)

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

Committed. Thanks much.

Status: Fixed » Closed (fixed)

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

macmladen’s picture

macmladen commented
Version: » All-versions-3.0-alpha1
Status: Closed (fixed) » Active

It is still not working, please look at this thred #662926: --php path argument required on Dreamhost and maybe unite threads as they talk about same thing. (I think).

anarcat’s picture

anarcat commented
Version: All-versions-3.0-alpha1 »
Status: Active » Closed (fixed)

Please do not reopen old issues especially when there's a new one that's been opened, unless the other one is marked as duplicate.

We have enough issues in the queue without reopening fixed ones. :P