Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Reproduce:
$ cd ../your_drupal8_folder/core/
$ drush status
[...]
Drupal root : ../your_drupal8_folder/core/
The reason is a bug in drush_valid_drupal_root. Patch is coming up in a few minutes.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | drush-wrong-drupal-root-1874726-15.patch | 665 bytes | hansfn |
| #9 | drush-wrong-drupal-root-1874726-9.patch | 726 bytes | hansfn |
| #7 | drush-wrong-drupal-root-1874726-7.patch | 1.04 KB | hansfn |
| #6 | drush-1874726.patch | 1.03 KB | jonhattan |
| #3 | drush-wrong-drupal-root-1874726-3.patch | 1.08 KB | hansfn |
Comments
Comment #1
hansfn CreditAttribution: hansfn commentedComment #2
hansfn CreditAttribution: hansfn commentedTabs replaced with spaces ;-)
Comment #3
hansfn CreditAttribution: hansfn commentedRemoved a typo - drupal7 and drupal8 key mixed up in last return statement.
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedCode still looks confused about version. Maybe it is deliberate but it looks weird at quick lance. See these two adjacent lines:
Comment #5
hansfn CreditAttribution: hansfn commentedThx for taking a look. The code isn't confused at all. Maybe an example will explain what I'm trying to handle:
You are located inside /docroot/drupal8/core (or any subdirectory) when running drush. From that location, the Drupal 7 path candidate "includes/common.inc" will match. Without an extra check to see if we are inside a D8 core directory, Drush will accept this as the Drupal root and we get "/docroot/drupal8/core" as root in stead of the correct "/docroot/drupal8".
The patch fixes this problem, without breaking the existing behavior. Maybe my comment could be clearer - it was the best I could come up with. Accept for this, I don't see that the patch needs more work ;-)
Comment #6
jonhattanProposed resolution
Comment #7
hansfn CreditAttribution: hansfn commentedThe patch above is functionally equivalent to mine. I guess the "../sites/sites.php" extra check is to catch the case when someone installs Drupal 7 in a folder named "core"?
Some issues:
1) The comment says "core/sites/sites.php" which doesn't exist AFAIK.
2) In a "standard" installation "../sites/sites.php" also doesn't exist so checking for it doesn't add any value. Maybe check for "../sites/example.sites.php" (or "../sites/default") instead.
Updated patch attached.
Comment #8
jonhattanYou're right. Committed #7
Comment #9
hansfn CreditAttribution: hansfn commentedHm, I just tested and there is a tiny bug in our patch - a missing slash. See attached patch.
Another minor thing: In the D7 case we added an extra check in case a user installed D7 inside a directory named "core". If we want to be really pedantic (and consistent), I guess we should add the same check in the D8 case: If we are running drush from the parent directory of a D7 installation in a directory named "core", the current code will return wrong Drupal root since the D8 candidate matches.
Comment #10
hansfn CreditAttribution: hansfn commentedHm, maybe I should have set the status to active ;-)
Comment #11
tim.plunkettWorks for me, thanks!
Comment #12
tim.plunkettI missed #8, I was RTBCing a committed patch :)
Comment #13
hansfn CreditAttribution: hansfn commentedIt seems my trivial bug fix in comment #9 has been committed (by moshe weitzman on the 29th of January). Apparently your RTBC had some effect ;-)
Comment #14
jonhattanI had a more accurate patch locally to check for the presence of composer.json. Committed: http://drupalcode.org/project/drush.git/commit/a6fedd1
Comment #15
hansfn CreditAttribution: hansfn commentedNice, jonhattan. There is one minor issue with your patch - your replace and with or in the "Drupal 7 root" case. This means that a user installing Drupal 7 in a directory named "core" is in trouble ... I have attached a trivial fix.
Comment #16
jonhattan@hansfn #14 works for me. Did you test it?
(file_exists($path . '/' . $candidate) && ((basename($path) != 'core') || !file_exists($path . '/../core/composer.json')))As I read it: candidate exists AND basename is not core OR (because basename is core)
core/composer.jsondoesn't exist.Comment #17
hansfn CreditAttribution: hansfn commentedSorry, jonhattan, your code works - I was wrong. I misunderstood the logic.
PS! If basename($path) is 'core', you might use
!file_exists($path . '/composer.json')in stead of
!file_exists($path . '/../core/composer.json')Comment #19
BMDan CreditAttribution: BMDan commentedcomposer.json is no longer in the root of core, so being anywhere under "core" results in misidentification of "core/" as a D7 root.
Full list of "composer.json"s under the real docroot:
Based on this, it looks like the test should be ../composer.json instead of ../core/composer.json, but I'm hesitant to actually suggest that without understanding the full logic.
Comment #20
BMDan CreditAttribution: BMDan commentedComment #21
hansfn CreditAttribution: hansfn commentedThat is the correct fix AFAICT too. The logic is simple - there is no composer.json in D7 (or D6) so the existence of such a file indicates very strongly that this is D8 ;-)
Comment #22
moshe weitzman CreditAttribution: moshe weitzman commentedCommitted as suggested. Thanks BMDan and hansfn