Continue #3166/2. Fix script name when Drupal uses a domain + subdir without scheme.

[rmuit]

This is an addition to #3553 (committed just before Drush 9.3.0, 3f18f81) which continued #3166.

This PR fixed the situation for --uri=http://domain.com/subdir and for --uri=domain.com/subdir/, but not for --uri=domain.com/subdir . (This is a regression from Drush 8 and has caused some emergency work on an existing Drupal-using project. Not sure if this ever worked with Drush9; I only tested 9.2.3, which obviously doesn't work because... above referenced code is missing.)

In the latter case, $_SERVER['SCRIPT_NAME'] will contain 'subdirindex.php' (i.e. missing slash) ... which makes DrupalKernel::findSitePath() return "sites/default" instead of the correct directory in $sites.

I believe the fix, and the fact that this doesn't introduce new regressions, is self evident when comparing it with the above referenced commit.

[rmuit]

Sigh. I managed to mess up even such a small PR.

I touched $this->uri in my first commit. Since I don't know Drush internals, I really shouldn't be doing that, because this meant it was not self-evident I wasn't introducing regressions.

Fixed now. (The one line I am changing now, is equal to the one line that was added in #3553 which was not in #3166 . I think that now the fix is self evident, when comparing it with #3553.)

2. to prevent confusion:

• Using Drush 9.6.0-beta3, a simple 'drush --uri=domain.com/subdir status' would not output the correct results because DrupalKernel::findSitePath() returned "sites/default".

• Drush 9.6.0-beta4 actually does output the correct result for this command. The reason is:

• • beta4 is calling bootstrapDrupalSiteValidate() a lot more than beta3; 4 times vs. 1 time for this 'status' command.
• • bootstrapDrupalSiteValidate() corrects itself when called multiple times. First time, it sets SCRIPT_NAME to 'subdirindex.php'; second time, $uri includes the scheme and it sets SCRIPT_NAME to 'subdir/index.php'. This means that the first call to DrupalKernel::findSitePath() returns "sites/default" but subsequent calls return the correct value.

Still... this PR is a fix; it makes Drush do things correctly on the first call to bootstrapDrupalSiteValidate().

[greg-1-anderson]

We should try to avoid validating so many times.

[greg-1-anderson]

Made some related improvements in #3852 (different but related issues)

[greg-1-anderson]

I don't think my comments need to block this 1-line fix. Could you please rebase with master and ensure that the tests are still passing?

[greg-1-anderson]

A test to guard against regressions would also be grand.

[rmuit]

Pushed. I... agree that a test is nice to have, and there's a more than average chance I'll get to look at that this weekend if no-one else does.

[rmuit]

FWIW: I was looking at adding a test "to guard against regressions" yesterday.

I thought that... probably needed to be a functional test, and I should probably extend CoreTest.php:
CoreCase::testOptionsUri() to test against a list of URIs, including with/without slash, and with/without scheme?

That's a fairly simple change to make and I can push a PR with that...
but... I couldn't get the test (run against yesterday's master) to fail. I just confirmed that the cause is that the 'drush core-status' command calls DrupalBoot8::bootstrapDrupalSiteValidate() 3 times, and it's "self correcting" on the second call (as mentioned earlier), so the drush command will output the correct URI.

How to write a test for that... I have no clue.

So this test isn't super useful right now. Could still be added, in case bootstrapDrupalSiteValidate() is going to get called only once in the future, though... I can't really judge.

[greg-1-anderson]

Confirming that the drush command outputs the correct URI is, I think, still valuable. Maybe you could use drush ev 'return DrupalKernel::findSitePath();' (after adding full namespace) to determine that findSitePath is returning the correct value.

[rmuit]

Followup in #3978. I discovered the reason why bootstrapDrupalSiteValidate() was called so much, and after fixing(?) that, could get the new tests to fail.