The way drush messes around with the working directory is really confusing. I found out in #1158490: drush dl --destination should assume a relative path that we can't really rely on the current directory being what it was when the process was started because a bunch of functions mess around with the current working directory. The boostrap process and package managers are the main culprits here.
Now, there's a handy drush_cwd() command to work around that, but you'll notice it doesn't work on windows, because windows doesn't set $PWD when the process start. $PWD is nothing magic - it's inherited by the shell, so we shouldn't expect it to be available.
During the bootstrap of Drupal, we do set the original directory:
../../includes/environment.inc:727: drush_set_context('DRUSH_OLDCWD', getcwd());
Interestingly enough, drush_cwd() says:
* TODO: Could cache result, but it isn't really expensive.
This sure looks like a cache to me!
So to summarize here, the problem is as follows:
* getcwd() is not reliable within Drush
* to work around this, we have drush_cwd(), which isn't reliable either (because it relies on PWD)
My suggestion is that we move drush_cwd() to a cache-only operation and configure the cache earlier in the bootstrap. Then we have a more uniform and reliable interface, and we actually make OLDCWD useful.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 1158504_cwd_wtf.patch | 2.42 KB | anarcat |
Comments
Comment #1
anarcat commentedi think this will clarify things a bit.
Comment #2
jonhattanIMO this patch is fine for 4.x (in case someone is using DRUSH_OLDCWD) and in 5.x we can use a static variable.
Comment #3
moshe weitzman commentedSeems fine to me. Any objections? greg?
Comment #4
greg.1.anderson commentedWorks great on Windows, and is a big improvement overall; committed to master. I don't have any objection to jonhattan's suggestion in #2 about a static variable; that would be another fine improvement, but I did not find it pressing.
Comment #5
msonnabaum commentedI think this is fine for 4 as well. Waiting for tests to pass and I'll commit.
Although a static variable seems redundant since the whole context system IS a static variable.
Comment #6
msonnabaum commented