#2909: Relative root paths no longer seem to work

[JTubex]

Describe the bug
#2909
When executing any drush command in a relative root path, the command times out and gives a segmentation fault.

To Reproduce
execute drush cr.

Expected behavior
A cache rebuild.

Actual behavior
The command timed out and gave a segmentation fault.

When executing drush cr -vvv the result was the following:

[preflight] Redispatch to site-local Drush: /var/www/..../htdocs/vendor/drush/drush/drush.
[preflight] Redispatch to site-local Drush: /var/www/..../htdocs/vendor/drush/drush/drush.
[preflight] Redispatch to site-local Drush: /var/www/..../htdocs/vendor/drush/drush/drush.
....

System Configuration

Q	A
Drush version?	9.6.x. 9.5 still works
Drupal version?	8.6
PHP version	7.2
OS?	Linux

[weitzman]

Greg suggest Path::canonicalize() instead of realpath().

[JTubex]

@weitzman, Path::canonicalize() did not resolve the issue.

The problem is that in Drush\Preflight\RedispatchToSiteLocal::redispatchIfSiteLocalDrush() $vendor is the absolute path and $root is the symlinked absolute path. So the check on rule 38 in Drush\Preflight\RedispatchToSiteLocal will fail because the symlinked path to drush is not the same as the real path to drush.

[weitzman]

I just approved webflo/drupal-finder#40. Hopefully @webflo will merge it soon. Does that fix it for you?

[JTubex]

@weitzman, The problem still occurs with the fix in webflo/drupal-finder#40.

[JTubex]

@weitzman, I was able to find where the bug is located.

1. e.g. Site is located in /var/www/release/1.0.0/htdocs
2. Create a symlink /var/www/current -> /var/www/release/1.0.0
3. The website is now accessible in /var/www/current/htdocs.
4. Execute drush in /var/www/current/htdocs.

The problem occurs in repo Webflo/Drupal-finder.

In file DrupalFinder\DrupalFinder::locateRoot line 43 the check is_link will return FALSE because the webroot /var/www/current/htdocs is not a symlink, but /var/www/current is.

[JTubex]

BTW: I created webflo/drupal-finder#41

[greg-1-anderson]

FsUtils::realpath is a lot safer than a raw realpath call, so perhaps this is okay. If we add this, it means that we'll have to work harder in the AppVeyor tests, because this will convert Windows paths from using / to \.

This only introduces a handful of failures in the current tests, so I suppose it is tractable.

[JTubex]

@greg-1-anderson, I'm not sure what the right way is to fix the tests. Can you point me in the right direction? Thanks!

[greg-1-anderson]

Our typical strategy here is to normalize the output before asserting. For example, if you str_replace each \ with a /, then you can compare the path invariantly again.

[JTubex]

Makes sense. Let's see if this fixes the tests.

[greg-1-anderson]

I'm 👍 on this; however, symlinks and realpath have caused us problems before, so let's get a second +1 from @weitzman before merging here before we decide whether we'll support it.

[weitzman]

Canonicalized paths always have same slashes even on windows. So I don't immediately see why we need str replace.

[greg-1-anderson]

This PR uses realpath rather than canonicalize. That is necessary, because this use-case involves symlinks. Symlinks and realpath have been problematic to support in the past, but perhaps might work out here.

[weitzman]

That's fine. I'm arguing against the replace calls in tests. Instead, whatever is writing cwd to config should canonicalize first.

[JTubex]

What about this change?

$this->originalCwd = Path::canonicalize(FsUtils::realpath($cwd));

[weitzman]

Thanks for your persistence, @JTubex.

[JTubex]

Great!