Problem/Motivation
DrupalKernel contains bootstrap logic which doesn't belong there because while console applications need a bootstrapped Drupal they do not need a kernel. This logic also contains hardwired assumptions about locations of settings.php and Drupal itself. These problems have spawned multiple several hundred reply issues over the last decade plus. These are the problematic methods:
- DrupalKernel::guessApplicationRoot
- DrupalKernel::findSitePath
- DrupalKernel::initializeSettings
- DrupalKernel::loadLegacyIncludes
- DrupalKernel::bootEnvironment
- DrupalKernel::initializeRequestGlobals
Proposed resolution
In the past it was not possible to provide these overrides simply because there was nothing before bootstrap. But now there is: the composer install process. Composer has a lot of advantages:
- per package config to provide defaults
- root composer.json to provide overrides
- knows all the install paths
- can write into the code directories
Using these advantages the proposal is quite simple:
- Move the bootstrap logic into separate classes. This issue only converts two of them as a demonstration.
- Create a composer plugin which allows changing these classes.
Here's the new DrupalKernel::guessApplicationRoot():
protected static function guessApplicationRoot() {
return PreBootstrap::$container->get('ApplicationRootGuesser')();
}
Here's the relevant part of the new extra section of core/composer.json:
"drupal-pre-bootstrap": {
"services": {
"ApplicationRootGuesser": {
"class": "Drupal\\Core\\Boot\\ApplicationRootGuesser"
}
}
}
If the root composer.json defines the same service it will override this definition. The plugin collects these definitions and writes them into a PHP file and also adds contrib classes to the autoloader on a best effort basis: Drupal\foo\... classes are picked up if they are in the src directory of the target directory of the drupal/foo project. Otherwise, you need to manually add them to composer.json autoload as a file in the root composer.json.
Remaining tasks
Answer concerns raised in #3576593: Create pre-bootstrap extension mechanism
Followups:
- Add a way to override the container definition with environment variables in
PreBootstrap. Probably an env var could store a JSON encoded service definition. - Convert identified bootstrap methods.
- Finally remove DrupalKernel from BootableCommandTrait.
- Potentially: add parameters to the pre bootstrap container with the relative paths from the app root to the web root, drupal core, vendor directories and a method so a script in either of these or the app root can get a relative path to any other and then generate an autoload.php in app root and drupal core as well to tackle #1792310: Wrong DRUPAL_ROOT with non-standard code structure.
Any actual business logic change is not in scope for this issue or these followups. Practically all of the related issues listed are enabled by this issue and followups and actually implementing them can commence once this family is finished.
User interface changes
N/A
Introduced terminology
PreBootstrap class, container and definition.
API changes
Pre-bootstrap functionality can be changed via composer.json
Data model changes
N/A
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|
Issue fork drupal-3576593
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:
Comments
Comment #2
nicxvan commentedComment #3
ghost of drupal pastComment #5
ghost of drupal pastComment #6
ghost of drupal pastComment #7
ghost of drupal pastDoes symfony/runtime provide such a mechanism? This issue came to be because I answered this question with a "no". symfony/runtime in my opinion should be used to interact with the ecosystem and I can't wait for the day when we can just add a line or two into composer.json overriding the runtime and bam, Drupal runs as a frankenphp worker. But that's a different issue than the problem at hand, it's not really usable as a pre-bootstrap extension mechanism.
In detail. The runtime itself is overrideable thanks to a composer generated template file. This generated file is then included from your front controller. Once you are inside the runtime, there's no override functionality provided. It gets about half way but it's just not there. If your front controller looks like this:
then GenericRuntime will reflect this closure, see the type, look for a class named
Symfony\Runtime\\'.$type.'Runtimein this caseSymfony\Runtime\Drupal\Core\ApplicationGuesserRuntimewhich then can decide at its leisure to pick an appropriate application guesser. However, this API is currently undocumented so the stability is unknown. More importantly, while this is an extension point, which is commendable this early, there is no functionality to provide a default application guesser and no functionality to override the default.DrupalRuntime::getArgumentcould, of course, provide this functionality -- but then what is symfony/runtime actually useful for if you need the logic from the composer plugin in this issue to collect the default/override information and you need the minimal logic inBootstrapMapfolded intoDrupalRuntimeas well? Worse, if you want to keep the "canonical" way you will need to pass all six of the overrideable types listed in the issue summary which would look quite ugly IMO, something likeI think this is enough to demonstrate my concerns. Or you could go override the reflection code path in
GenericRuntimeand just pull all these default/override information from a composer generated file but then again what is the point of using symfony/runtime if you hardwire these six anyways?Comment #8
andypostComment #9
andypostComment #10
longwaveThese feel a lot like services. Should they be services, configured by default in
DrupalKernel::$defaultBootstrapContainerDefinition, with perhaps a better override mechanism than the one we currently provide?This might not be feasible, as we might not be able to set up the bootstrap container early enough due to some chicken-and-egg problem, but putting the idea out there.
Comment #11
ghost of drupal pastUsing a container definition sounds like a good idea. Adding new things to DrupalKernel definitely doesn't sound like a good idea. Using the bootstrap container itself is highly problematic anyways because some services depend on the database and the database is not functional before settings is included and so that'd be a bad dx: here's a container but take care what order you retrieve services from it 'cos they might not work.
So perhaps we could create a new include -- much like both approaches on the "less bespoke boot" symfony/runtime did -- which, upon inclusion, gives you a container. The composer plugin would change the definition of this container. That's easy, that's just another array. Makes the DX very nice and familiar. Probably the environment overrides could be a single JSON encoded array which gets deep merged into this definition. This would go a long way towards replacing DrupalKernel for boot because currently the state of bootstrap for eg static::$isEnvironmentInitialized is held by DrupalKernel and we need to put that somewhere, in this case it could go on an EnvironmentBooterFactory or some such.
Comment #12
joachim commentedGuessing the application root is going to go away in the wrong Drupal root issue anyway. Maybe we could get that in first?
Comment #13
ghost of drupal pastComment #14
joachim commentedCould we put the new classes into a new Boot namespace, ie. put them in core/lib/Drupal/Core/Boot/* ?
Comment #15
ghost of drupal pastComment #16
andypostThank you looks nice and solves the Gordian knot of boostrap/autoloader, which kind of tests makes sense here?
I'm confused by naming
core/lib/Drupal/Core/Boot/EnvironmentBooter.phpit just configure php settings it does not boot anythingComment #17
ghost of drupal pastComment #18
ghost of drupal pastComment #19
ghost of drupal pastComment #20
ghost of drupal pastComment #21
ghost of drupal pastComment #22
ghost of drupal pastI am much enlightened. Obviously symfony is the answer to everything. Please disregard this issue. I will not continue working on it.
Comment #24
nicxvan commentedI still think this is worth doing, even if I or someone else needs to take it further.
Comment #25
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #26
nicxvan commented