Problem/Motivation

#3106531: Notify in Status Report that per-table database prefixes are no longer supported, and will throw errors in Drupal 10.0 introduces a function that includes settings.php again as part of requirements checks.

But that IMHO has two problems:

a) It is missing context, for example $class_loader. To use an alternative cache backend for the container cache, you afaik need to set the class loader explicitly, afaik that is still required. This is a fatal error now.
b) It loads those files a second time. They could do all kinds of things like messing with request data, define functions and what not that would cause problems.

Not quite sure what we can do? At least for the current/default connection, we can access \Drupal::database()->getConnectionOptions()['extra_prefix'], we could reduce the function to just checking that as a quickfix?

Other approaches would be to parse/preg match the settings file, but there can be includes to other files that define the database info. Does not seem viable

Remaining tasks

None

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

TBD

CommentFileSizeAuthor
#4 3251625-4.patch1.83 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

catch’s picture

Not quite sure what we can do? At least for the current/default connection, we can access \Drupal::database()->getConnectionOptions()['extra_prefix'], we could reduce the function to just checking that as a quickfix?

Let's try that. It's only the status report and the likelihood of a site actually using this is quite low these days, so better to do the absolute minimum.

alexpott’s picture

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. It's weird there's no other way to get to the raw settings - with this change we are already checking the processed ones.

  • catch committed 773a213 on 9.4.x
    Issue #3251625 by alexpott, Berdir: Including settings.php a second time...

  • catch committed 81690f8 on 9.3.x
    Issue #3251625 by alexpott, Berdir: Including settings.php a second time...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.4.x and cherry-picked to 9.3.x, thanks!

Status: Fixed » Closed (fixed)

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