Consistently use either is_file() or file_exists()

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for 8+ years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

I think this is probably still valid, but we only really care about it in the critical path and it's not straightforward when checking for things that might be directories, so we can't enforce it.

So I think we should change DrupalKernal::findSitePath() to use is_file(), since that runs on every single request to Drupal. And not worry too much about anywhere else. Asset aggregation used to use file_exists() on every request 12 years ago but don't any more.

Updated IS a bit and made an MR. Tests are running.

Correct the counts in the issue summary.

I'm willing to bet this is random and unrelated

 Json Api Functional (Drupal\Tests\jsonapi\Functional\JsonApiFunctional)
     ✘ Read
       ┐
       ├ Behat\Mink\Exception\ExpectationException: Current response status code is 200, but 503 expected.
       │

Searched DrupalKernel and all instances have been replaced.

This looks good - changes the usage in the critical path, leaves everything else because it's not a 1-1 swap.

The performance benefit per-request is not really measurable except with strace, but across billions of requests for every Drupal website for the next decade or so it will probably add up to something.

catch → closed merge request !11048

Is it correct that this means settings.php is no longer allowed to be a symlink? If so, would be nice to get a change notice for this. I do find that particular setup to be useful for local development, i.e. I have a generic settings.php that sets up e.g. the database name and things like that based on the site name and then (along with the respective webserver config) I can easily "spin up" an additional multi-site by just running mkdir sites/foo; ln -s '../settings.site.php' sites/foo/settings.php; and I'm off to the races. Although slightly less neat, not a big deal to change that setup to copy those files around, but, yeah, would be good to have an actual change notice for that, if that is in fact the case.

If #49 is correct then I think we should roll this back, there are surely numerous hosting setups where settings.php is a symlink and there's no technical reason for this to be disallowed.

@tstoeckler are you asking because this no longer works or just in case? https://www.php.net/manual/en/function.is-file.php resolves symlinks so I would think there should be no change for that use case.

Ahh OK, sorry for the noise in that case. No just noticed this issue and apparently I misread the (ancient) discussion above about is_file() vs. file_exists().

Automatically closed - issue fixed for 2 weeks with no activity.