Problem/Motivation

1. D7 => D8 WTF - Using e.g. sites/all/modules/memcache/memcache.inc did just work, but the same with classes does not - until the module is enabled, which can be very problematic to deploy production changes. (How do you sync the settings.php change with the enabling of the module without the site WSOD'ing in between?)

2. In a composer enabled world the expectation is that modules have a composer.json, which specifies the folders to autoload, which is then automatically loaded. Once a module starts using autoload: psr-4: { 'Drupal\\my_module\\': 'src/'} we get a huge behavior change for a disabled vs. an enabled module when that module is managed by composer or not.

3. Small DX problems, e.g. to override the container class with one from a module, explicitly the autoloader has to be used to addPsr4() for the module.

Proposed resolution

- Always load PSR-4 for all available modules.

This is a step in transitioning us to a static composer world - as the rest of the PHP world already uses.

Remaining tasks

- Discuss
- Create patch

User interface changes

- None

API changes

- Behavior change: Modules src/ directory classes are always available in the autoloader.
- Composer managed modules can add autoload: psr-4 safely as there is no change in behavior.

Data model changes

- None

Comments

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented

I hadn't thought of this very good point.

Taking the contrib module "monolog" as an example, all of the /modules/monolog/src and /core/vendor/monolog files will get loaded by Drupal even if the monolog module is not enabled or being used.

Crell’s picture

Crell CreditAttribution: Crell at Palantir.net commented

Didn't we just recently go to a lot of work to ensure this DIDN'T happen, as it did crazy things to plugins?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@Crell: Plugin discovery used to use the actual namespaces registered to the classloader.

This is no longer the case however, plugin/annotation discovery gets a list of namespaces from the container/DrupalKernel, which in turn gets the module list from config.extension - at least that's what core does, nothing stops you putting something different than container.namespaces in a services.yml

So plugin discovery itself should be OK.

However...

@Fabianx:

D7 => D8 WTF - Using e.g. sites/all/modules/memcache/memcache.inc did just work

This didn't just work.

At least you had to do:

$conf['cache_backends'][] = 'sites/all/modules/memcache/memcache.inc';
$conf['cache_default_class'] = 'MemCacheDrupal';
Once a module starts using autoload: psr-4: { 'Drupal\\my_module\\': 'src/'}

Shouldn't they not do this?

This is a step in transitioning us to a static composer world - as the rest of the PHP world already uses.

There's no chance that 8.x will get there since we can't drop the module interface from core or the requirements that comes with.

 public function getFormId($form_arg, FormStateInterface &$form_state) {
    // If the $form_arg is the name of a class, instantiate it. Don't allow
    // arbitrary strings to be passed to the class resolver.
    if (is_string($form_arg) && class_exists($form_arg)) {
      $form_arg = $this->classResolver->getInstanceFromDefinition($form_arg);
    }

This opens up modules being able to specify classes from other modules for forms (quite a common use case), but then forget to add a dependency because it works as long as that module is in the file system. This seems like more of a gotcha than not adding autoload: psr-4 to contrib modules with a composer.json

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

#3: Some great feedback! :)

Yes, agree 'container.namespaces' is how plugin discovery works and also not a problem for other things like *.yml, which is all based on the module list.

--

1. Yes, but that was a completely different world with almost no dependencies on e.g. cache backends and almost no autoloading.

2. Nothing is preventing them from doing that. Also modules will be re-used in different contexts and then someone will add PSR-4 loading.

e.g. currently strictly speaking all modules break the unit testing contract, I can't require module/X and use its Interface, because it is not auto-loaded.

Therefore modules unit test with the whole of core installed and enabled and using core's phpunit runner, which is okay, but a questionable practice.

Therefore rather sooner or later modules will start adding PSR-4, I can guarantee that.

(Edit: or in this case use PSR-0 http://cgit.drupalcode.org/monolog/tree/composer.json?id=84cdb36, thanks #1 - it has already started then ...)

And strictly speaking modules using composer _should_ be adding autoload: psr-4 to them.

3. We will definitely and 100% keep the module isolation and separation, but we should also ensure that our 'module world' does not conflict with the 'composer world'.

And this is one area of conflict, where composer has different assumptions, but we support that.

4. And then we have that problem anyway once modules start to specify PSR-4, so I would rather have a module_class_exists() function or service, which maps the namespace to the module if existing and denies it when its not loaded.

Rather than to wait for contrib to "break" this.

e.g. start with the right assumption when 8.0.x ships.

And especially not land in a situation where a module behaves different when installed with composer or not.

e.g. a contrib developer uses a composer based workflow for their local development, the class_exists() is true. Hence forgets to add the dependency.

Someone else downloads the module from drupal.org and wonders why the form cannot be found ...

etc.

Lets discuss some more in IRC today :).

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

Okay, I did some research on other content management systems.

- CMS Made Simple extensions just push composer libraries with hard-coded vendor directories, the core does not support it at all. You FTP modules and the code base.

- Typo 3 is where it gets interesting and I am pretty sure we will end up with a similar solution as that is the most sane way and exactly what I thought about for the long-run (though I disagree a little with the specific implementation):

https://git.typo3.org/Packages/TYPO3.CMS.git/blob/refs/heads/master:/typ...

They parse the autoloading information of the composer.json for those extensions not covered by composer and register them in the same way.

So far as I can see they also have essentially a 'all-extensions-are-autoloaded' way in that.

Also adding the psr-4 information in here, makes a lot of sense:

- https://git.typo3.org/Packages/TYPO3.CMS.git/blob/refs/heads/master:/com...

and then again in each module: https://git.typo3.org/Packages/TYPO3.CMS.git/blob/refs/heads/master:/typ...

From their press announcement:

This has now been streamlined and the “old” class loading functionality was migrated and unified to be using composer, which is only triggered on installation of an extension and is cached separately.

(http://typo3.org/news/article/announcing-typo3-cms-73-more-stability-mor...)

Need to check some more other CMS, to see how they solved it.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Title: Ensure all modules are autoloaded with PSR-4 regardless if enabled or not » Ensure all modules are autoloaded with PSR-4 only if enabled
Issue tags: +Needs issue summary update

Discussed in IRC:

Outcome was:

- Automatically loading modules is a bad idea - whether that happens by composer or by core. The contract of Drupal modules is that they are available once enabled, and not before that.

We should ensure that contract is always fulfilled - else chaos via class_exists() can happen.

Plan B is:

- Add a composer hook that removes all autoloading information from modules (type = drupal-module as per composers/installers)
- Add normal autoload: psr-4 to the modules composer.json itself, also do that for all core modules (follow-up), so core sets a good example
- Add 'library' type to drupal.org so that e.g. memcache can provide a library instead of a module and libraries are auto-loaded per definition and cannot be enabled. :)
- Remove the hard-coded addPsr4() from DrupalKernel and instead parse composer.json files and store the autoloading information in the system information / container. / fallback for modules that don't have composer.json to use PSR-4: Drupal\\module_name\\ => src/.

That way:

- We blacklist all autoloading information, but then take care of it while loading the module.
- We ensure a module is never available unless enabled; use a library if you need something auto-loaded.

That should cater to all use-cases and preserves the strong contract that modules have with core, while enabling them to be unit-tested, etc.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Related issues: +#2474007: Add a “General” project type to Drupal.org

Related issue: #2474007: Add a “General” project type to Drupal.org

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Category: Bug report » Task

Given #6 I think this is a task now.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented

Time to resurrect this issue? #2796953: [regression] Plugins extending from classes of uninstalled modules lead to fatal error during discovery would have been caught much sooner if phpunit only autoloaded classes from enabled modules (see #2797269: Improve MigrationPluginListTest).

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Version: 8.1.x-dev » 8.2.x-dev

A little late for 8.1.x...

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Version: 8.2.x-dev » 8.3.x-dev

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.