Skip to content

Commit

Permalink
fixes lisantra-technologies#3 for ubuntu 12.10 with PHP 5.4
Browse files Browse the repository at this point in the history
  • Loading branch information
@lornajane
lornajane committed Mar 3, 2013
1 parent afdd508 commit 387c787
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions Trunk/VersionControl/Hg/Executable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ public function setExecutable($path = null)
if ( ( empty($executables) ) || ( null === $path ) ) {
/* iterate through the system's path to automagically find an
* executable */
$paths = explode(PATH_SEPARATOR, $_SERVER['Path']);
$paths = explode(PATH_SEPARATOR, $_SERVER['PATH']);

foreach ( $paths as $path ) {
if (is_executable($path . $binary)) { //DIRECTORY_SEPARATOR .
$executables[] = $path . $binary;
if (is_executable($path . DIRECTORY_SEPARATOR . $binary)) { //DIRECTORY_SEPARATOR .
$executables[] = $path . DIRECTORY_SEPARATOR . $binary;
}
}

Expand Down

3 comments on commit 387c787

@mgatto
Copy link

@mgatto mgatto commented on 387c787 Mar 4, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, I commented out DIRECTORY_SEPARATOR because on Windows, Path might have already terminated in a slash. I'll re-run the tests with DIRECTORY_SEPARATOR back in on Windows and confirm or deny. I'll definitely merge the the first chunk: that's a real egregious oversight on my part. If all is well with the second chunk, I'll merge the who pull request as is.

@mgatto
Copy link

@mgatto mgatto commented on 387c787 Mar 15, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the casing is different on different OS's:

//Windows
$_SERVER['Path'])

//*nix
$_SERVER['PATH']);

Uppercasing it to "PATH" on Windows emits a PHP warning, that the index does not exist, and returns an empty value. Bummer. Now, I'm looking for an OS-independent way of grabbing the path, or even not using $_SERVER, but so far, no joy. I can just add a constant for it at the head of the file and test for OS as we do for the binary (and abstract that test out to a private function or statement in the constructor to set a class property as a flag). Suggestions?

@lornajane
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens because $_SERVER['PATH'] is actually an operating system environment variable, not a PHP thing. I'm making a pull request which grabs this a few lines above when we're doing windows/*nix stuff separately anyway. We may need to revisit but I think it's good enough to be going on with!

Please sign in to comment.