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.
Problem/Motivation
Following #1497230: Use Dependency Injection to handle object definitions , we need a common place to bootstrap objects in the drupal_container()
. Currently, it registers the objects in drupal_container() itself, but we need a nicer place to register these bootstrap objects.
Proposed resolution
Have the Container build itself via one of the many Loaders.
Remaining tasks
The usual.... Write a patch, review a patch, patch a patch, wear eye patch.
Comment | File | Size | Author |
---|---|---|---|
#19 | 1511482-19.patch | 2.36 KB | RobLoach |
#17 | 1511482.patch | 2.38 KB | RobLoach |
#10 | 1511482_9_bootstrap.patch | 3.03 KB | cosmicdreams |
#9 | no_language_bootstrap.patch | 13.36 KB | RobLoach |
#4 | 1511482.patch | 2.37 KB | RobLoach |
Comments
Comment #1
RobLoachFollowing #1497230: Use Dependency Injection to handle object definitions , we need a common place to register the very basic early configuration for the Drupal Container. These mappings arn't meant to perform any expensive operations, just meant as the initial mappings so that we can use the Container throughout Drupal's bootstrap sequence.
Symfony uses a configuration loader to build the Container:
We probably don't want XML (there are a number of other options), but you get the idea. Again, this is just the base bootstrap objects that we need. Once the Kernel is in, we will likely extend it to use the event dispatcher so that these mappings can be built upon and modified.
Other option is to just stick all of it in something like drupal_container_bootstrap() or something, but it would be nice to have a configuration file for all this.
Comment #2
sunIntroducing new DI tag.
Comment #3
cosmicdreams CreditAttribution: cosmicdreams commentedis _drupal_bootstrap_configuration a good place to include this logic?
Comment #4
RobLoachMaybe something like the attached. We could move lots of the bootstrap in here.
Using one of the Dependency Injection Loaders would be great, but I don't think we need one of them at this stage. Would be nice to dump some of the dependency injection container configuration to a file during the site installation, but we're not quite there yet.
Comment #5
cosmicdreams CreditAttribution: cosmicdreams commentedThere's a bit of code that runs that looks at every language type and registers it to the DI. I can't remember exactly where that code is but it's there.
Does this approach require that we manually register everything?
Comment #6
RobLoachdrupal_language_initialize() is what you're looking for?
Anything that we want as early as possible that does not require any expensive processing, yes. I'm thinking this is just for the defaults of stuff that could be overriden later on.
What are your thoughts? It would be nice to dump some of this configuration somewhere on initial site set up, like Symfony does, but I'm not sure we're at that stage in Drupal yet.
Comment #7
RobLoachAh, I understand what you mean. Move some/all of drupal_language_initialize() into the Language class so that the Container can lazy-load the Language system?
Comment #8
cosmicdreams CreditAttribution: cosmicdreams commentedin response to #7: Yes, exactly
Comment #9
RobLoachWe probably couldn't get rid of the language bootstrap all together, could we?
Comment #10
cosmicdreams CreditAttribution: cosmicdreams commentedHere's an attempt to do that, that is probably wrong:
Comment #11
Crell CreditAttribution: Crell commentedRegistering certain container objects in the DIC's constructor means that it is tightly coupled to the Drupal\Core\Language\Language class, and will fail without it. This strikes me as defeating the whole purpose of using a DIC, which is looser coupling.
Comment #12
cosmicdreams CreditAttribution: cosmicdreams commentedHere we use 'language.bootstrap' where in other places we have language_url. It would be great to be consistent about how we name these things. The dot notation seems more robust. We should use that.
Comment #13
sunI somewhat shared @Crell's concern at second sight, but since this merely registers the service but doesn't actually do something unless the service is accessed, I do not actually see a problem with the proposal.
It basically says: "Oh, you want to bootstrap Drupal? Well then, you need at least this. Feel free to mock/replace it, but Drupal requires it to be there."
Comment #14
sunAlso, I'm not sure why it failed, and even more so, I'm not sure whether it is technically correct, but I like the direction that @Rob Loach investigated with entirely replacing drupal_language_initialize() in #9. I'd imagine a DI-enabled bootstrap to work like that, or along the lines of that. However, I'd recommend to make that a follow-up issue.
Comment #15
cosmicdreams CreditAttribution: cosmicdreams commentedfrom the automated patch review: most of the exceptions are from a the fact that the code is looking for language_content and can't find it.
Perhaps the it isn't being set in the DI code. Previously the list of all types was retrieved like this:
In #9 the set of types was manually defined.
Comment #16
RobLoach#1550866: Remove obsolete drupal_language_initialize()
Comment #17
RobLoachHaving the Container built from a PhpFileLoader accomplishes exactly what we have here. This just cleans up where the initial Drupal bootstrap configuration is set up. As sun pointed out, this is just the basic object mappings and no logic is actually taken place. It's just the mappings of what's required for a basic Drupal bootstrap.
If you have any other suggestions, I'm all ears. Having a dependency injection container that we can use throughout the Drupal bootstrap process is essential.
Comment #18
cosmicdreams CreditAttribution: cosmicdreams commentedsmall quibble, but I think it's important.
If we're going to extend a class in another library named ContainerBuilder then our class should be named ContainerBuilder. At first glace I thought our Container was extending Symfony\Component\DependencyInjection\Container.php instead of Symfony\Component\DependencyInjection\ContainerBuilder.php
Comment #19
RobLoachComment #20
effulgentsia CreditAttribution: effulgentsia commentedMoved #19 to #1552744-22: Bootstrap for the Dependency Injection Container and make sure SimpleTest abides to it. Resetting this to active to address the stuff that's in the issue summary and early comments.
Comment #21
RobLoachComment #21.0
RobLoachgfd