DIRECTORY_SEPARATOR causes errors on Windows running CygWin/Msys/Git shell

When you say "if you are running a Windows shell", does that mean that you are running under cygwin?

Yes. I'm running the Git shell on Windows. I believe the Git (I'm on v1.7.4.1) shell runs on Msys, which is very similar to Cygwin. Anyhow, if I put

print 'directory separator is ' . DIRECTORY_SEPARATOR . "\n";
exit;

I get "\" from both the browser and the command prompt. (FYI: I'm running WAMP server 2.0 with PHP 5.2.11 on Windows 7.)

Is there a way to set this via some PHP config? My understanding was this constant was baked into PHP based on the native environment. And PHP is running as a native Windows app and not as part of a Cygwin/Msys shell. But I'm well above my pay grade talking about what's built into PHP...

DIRECTORY_SEPARATOR is not supposed to be used to compose paths; it is supposed to be used with rtrim and other operations on paths handed to you by the system.

Agreed. Since PHP on Windows accepts either / or \ as a directory separator, I'll remove the bit from drush_find_tmp().

However the rest of the patch deals with processing return values from drush_shell_exec_output() which, when executed in a Cygwin/Msys shell, return paths with / as the directory separator even on Windows

Perhaps the option should be more general? Something like "running_shell_on_windows"?

I agree that there is some detailed knowledge about PHP and your OS and what shell you're using to set the value correctly. The patch currently sets DRUSH_DIRECTORY_SEPARATOR = DIRECTORY_SEPARATOR if there is no option specified. This is how things currently work without the patch. But the patch lets those that do know enough about their PHP/OS/shell config to specify a different separator if they know what they're doing.

I'm concerned that trying to do the automagical settings will result in edge cases causing problems. The current patch leaves things as-is unless a knowledgeable user specifies something different.

How about settings DRUSH_DIRECTORY_SEPARATOR as such:

  if (drush_is_windows()) {
    drush_shell_exec('pwd');
    $output = drush_shell_exec_output();
    if (FALSE === strstr($output, '/')) {
      define('DRUSH_DIRECTORY_SEPARATOR', '\\');
    }
    else {
      define('DRUSH_DIRECTORY_SEPARATOR', '/');
    }
  }
  else {
    define('DRUSH_DIRECTORY_SEPARATOR', DIRECTORY_SEPARATOR);
  }

I couldn't use DRUSH_COMMAND as that has both forward and backslashes in it ($_SERVER['argv'][0] has them).

Or is there another readily available path we could use instead of another shell_exec? I'm not all that familiar with the Drush bootstrap process.

If this is agreeable, I'll roll another patch. Thanks.

My mistake, most $_SERVER entries are forwardslashed, but realpath() converts those to backslashes on Windows. The above can shrink to

  if (drush_is_windows()) {
    if (FALSE === strstr($_SERVER['argv'][0], '/')) {
      define('DRUSH_DIRECTORY_SEPARATOR', '\\');
    }
    else {
      define('DRUSH_DIRECTORY_SEPARATOR', '/');
    }

OK, re-rolled using the logic in #7 instead. This has the added bonus of letting us set DRUSH_DIRECTORY_SEPARATOR earlier in the bootstrap process so we get the correct path in drush_find_drush.

I also worked on the fact that realpath() on Windows forces the backslash separator by refactoring realpath to drush_realpath and dealing with the changes there.

Feedback is appreciated! Thanks.

Some background which may or may not be needed: realpath conversion of slashes on Windows: http://us3.php.net/manual/en/function.realpath.php -- see example #2. Forward slash is the universally accepted directory separator: http://php.net/manual/en/function.dirname.php, see path parameter description.

True, running a Cygwin (or similar) shell on Windows makes backslashes unusable but I do not see this as a system configuration issue. You are correct that this is not the standard Windows/DOS behavior. But anyone that has installed a *nix shell on Windows will be in the same boat -- which I suspect will be a growing portion of the Windows Drupal community as they start working with Git. Also, this patch protects those that are running Windows without a shell by checking the return of $_SERVER['argv'][0]. In a non-shell config, it returns c:\bin\drush\ while in a shell config it returns /c/bin/drush or /cygwin/c/bin/drush. DRUSH_DIRECTORY_SEPARATOR is set accordingly.

I have to admit that it is a little mysterious to me why the filesystem would ever return paths with / in them if realpath was also correcting path separators back to \. Seems very odd.

I think the confusion is because PHP on Windows has some baked-in assumptions about directory separators (backslash) while a Cywin or similar shell tries to mimic the *nix environment with forward slash directory separators. So PHP assumes backslashes while Cygwin (via exec) returns forward slashes.

Oh, if we could all just speak the same language! ;)

Thanks.

My brain is starting to get a little knotted up by this discussion, so I'm going to state some basics just to make sure I'm on the same track as Greg... Perhaps this is how I should've started this issue, but it seemed like such a small change at first! Please feel free to correct anything I'm missing.

I see three scenarios that involve directory separators:

1. Building paths
2. Dealing with paths returned from PHP (eg: getcwd() or realpath())
3. Dealing with paths returned from exec()

For #1: we should use the forward slash everytime as it works for Windows (accepts both) and *nix (accepts only forward slashes). That seems to be how Drupal core does it in looking at includes/file.inc. eg: file_create_filename()

For #2: PHP for Windows returns backslashes while PHP for *nix returns forwardslashes. This seems to be baked into PHP and not configurable via $_ENV or similar. See this PHP doc page.

For #3: exec() returns using the directory separator of the underlying command. Things like tar return forwardslashes --at least for Cygwin and Msys, the only two I've unsed. I have no idea what happens if Drush exec's a DOS command in a Cygwin environment... I assume it would return with backslashes? But I couldn't find any examples where Drush exec'ed anything other than *nix commands so perhaps it's not an problem.

Additional headaches arise when using Windows to remotely control a *nix box or the reverse.

My suggestion is to have Drush internally convert any dir separators to forwardslashes -- that way they execute on any OS, eliminating the Windows remotely controling a Linux box issue. There already exists _drush_convert_path() which does this. (Note: I need to update my earlier patch to use this rather than str_replace). The above patch also changes the one remaining place where Drush had hardcoded backslashes (drush_find_tmp).

That leaves paths returned by PHP and ensuring that Drush only calls exec on *nix-style commands. My hope was that add drush_realpath that we could deal with the baked-into-PHP backslashes there. While drush_path_trim (or similar) would be a useful addition, it won't deal with the backslashes within returned by a call to realpath().

I don't think we need anything as nuclear as drush_path_alter if we always use forwardslashes for outbound paths. (Meaning paths passed to the system rather than return values being parsed by Drush). Internally, Drupal seems to build paths with forwardslashes and I've not seem any backslashes in the DB, even on sites that have run exclusively on my Windows dev machine.

I'll reroll the patch to use _drush_convert_path() and add a drush_path_trim function to replace the use of rtrim. But I think that drush_realpath is required.

Thanks.

Hmmm... I had forgotten about the drush.bat environment. I'll have to try that out.

Drupal doesn't use DIRECTORY_SEPARATOR in core on D6. In D7, it shows up in a few places: includes/database/log.inc and modules/simpletest/filetransfer.test. In log.inc it's parsing the return from debug_backtrace(), which would have backslashes on Windows, and in simpletest it's building paths to pass to PHP which can use either on Windows.

I can't say this with certainty, but it seems that Drupal core is building paths with the forwardslash only (except for the simpletest example). Which is good...

1a. Building paths for use by PHP
1b. Building paths to be passed as arguments to drush_shell_exec or drush_op_system.

re: 1b: Again, I can't speak with certainty here, but on the two shells I've worked with (Cygwin and Msys) shell commands accept forwardslashes and return filepaths with forwardslashes (eg: tar -tf returns "drupal-7.0/..."). And since *nix boxes only take forwardslashes, if we make the forwardslash universal, it will (hopefully) eliminate some of the Windows -> Linux remote issues. I don't believe there's a need to convert exec'ed or system'ed paths to backslash on Windows. (Note: need to verify this w/ drush.bat).

I'll just throw this out as crazy-talk: What about eliminating drush.bat and requiring anyone using Drush on Windows to have a shell? They've already got PHP. They ought to be installing Git anyhow, right? Git comes with a shell that works great with Drush (once you add wget). Having a more limited config on Windows would help us improve Drush support on Windows. Through this process I've been working on instructions on setting up a Drupal dev environment on Windows, a stripped down version of which could be used to setup Drush on Windows.

Just wanted to leave a quick comment that I'm still interested and working on this issue. It's just those pesky paying customers keep getting in the way... More coming (hopefully) in a week or so.