Problem/Motivation

Following up on Try to use is_file() instead of file_exists() in DrupalKernel::findSitePath():

The check for finding a site path in a multisite construction, without the requirement for the existence of settings.php, is broken since #1333940 has been merged.

Change on Gitlab: https://git.drupalcode.org/project/drupal/-/commit/b310d19833c0de5dcd487...

Where file_exists($directory) returns TRUE when a directory exists, is_file($directory) does not. It returns FALSE when the given path is not a file.

Returns true if the filename exists and is a regular file, false otherwise.
- https://www.php.net/manual/en/function.is-file.php

Steps to reproduce

  1. Create the directory web/sites/example.org
  2. Add $sites['example.org'] = 'example.org'; to web/sites/sites.php
  3. To test the site path output you can use Drush: drush --uri=https://example.org php:eval 'echo \Drush\Drush::bootstrapManager()->bootstrap()->confPath(FALSE);'
  4. Instead of the expected output sites/example.org, you'll get sites/default

Proposed resolution

The check (!$require_settings && is_file($app_root . '/sites/' . $site_id)) needs to be replaced with (!$require_settings && is_dir($app_root . '/sites/' . $site_id)).

Remaining tasks

  1. Writing a patch
  2. Testing the patch
  3. Update core/tests/Drupal/Tests/Core/DrupalKernel/DrupalKernelTest.php to test changes
  4. Merging the patch

User interface changes

-

Introduced terminology

-

API changes

-

Data model changes

-

Release notes snippet

-

Issue fork drupal-3571452

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

sanderwind created an issue. See original summary.

sanderwind opened merge request !14634

ishani patel changed the visibility of the branch 3571452-drupalkernel-findsitepath-incorrect-check to hidden.

ishani patel’s picture

ishani patel commented
Assigned: Unassigned » ishani patel
ishani patel’s picture

ishani patel commented
Assigned: ishani patel » Unassigned
sanderwind’s picture

sanderwind
Primary language Dutch
commented
Status: Active » Needs review

sanderwind changed the visibility of the branch 3571452-drupalkernel-findsitepath-incorrect-check to active.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Something we probably need is a test case showing the problem.

sanderwind’s picture

sanderwind
Primary language Dutch
commented

Added some tests. Also for other situations, like missing sites.php and requesting a site path for a missing host.

sanderwind’s picture

sanderwind
Primary language Dutch
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests

Updated task list after tests have been added too.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left some comments on the test file.

Thanks.

sanderwind’s picture

sanderwind
Primary language Dutch
commented

Thanks for checking out my MR, smustgrave. I did add some comments too :-)

sanderwind’s picture

sanderwind
Primary language Dutch
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative, +Needs Review Queue Initiative

Test this manually on the main branch following the steps
mkdir sites/example.org
Created a sites.php in sites folder
Added $sites['example.org'] = 'example.org';
Ran ddev drush --uri=https://example.org php:eval 'echo \Drush\Drush::bootstrapManager()->bootstrap()->confPath(FALSE);'
Got sites/default%

Unfortunately applying the MR and running the same command I get the same so is there a step missing from the summary?

sanderwind’s picture

sanderwind
Primary language Dutch
commented

Hey smustgrave,

Thanks for having a look at this issue again.

I retested my patch at an existing project and a clean project (composer create-project drupal/recommended-project). Both return sites/example.org when running drush --uri=https://example.org php:eval 'echo \Drush\Drush::bootstrapManager()->bootstrap()->confPath(FALSE);'.

I am not using ddev, but a plain docker-compose setup, using wodby images. 

./vendor/bin/drush st
Drupal version : 11.4.2
Site URI       : https://my-drupal11-site.local.dev.frl
PHP binary     : /usr/local/bin/php
PHP config     : /usr/local/etc/php/php.ini
PHP OS         : Linux
PHP version    : 8.4.23
Drush script   : /var/www/html/vendor/bin/drush.php
Drush version  : 13.7.6.0
Drush temp     : /tmp
Drush configs  : /var/www/html/vendor/drush/drush/drush.yml
Drupal root    : /var/www/html/web
Site path      : sites/default
Drupal config  :

Are you sure your sites.php contains $sites['example.org'] = 'example.org';? Maybe a typo somewhere?

sanderwind’s picture

sanderwind
Primary language Dutch
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented

Nope I had $sites['example.org'] = 'example.org'; exactly.