Using ExtensionDiscovery() instead of moduleHandler to discover module composer files would allow us to crawl all modules (installed and not installed) for their composer.json files and avoiding

See #2352737: [composer_manager] Make composer.json generation work without a Drupal install and #2350639: Rewrite 8.x-1.x

CommentFileSizeAuthor
#5 2376785-5-use-extensiondiscovery.patch4.34 KBbojanz
#2 2376785-use_extensiondiscovery-2.patch4.32 KBpcambra

Comments

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Issue summary: View changes
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new 2376785-use_extensiondiscovery-2.patch4.32 KB
bojanz’s picture

bojanz commented

This fixes a huge problem in the current implementation, if you want to use a library interface for an entity type, you cannot, because the composer.json can't be rebuilt before the module is installed, and the module can't be installed before the library is present.

bojanz’s picture

bojanz commented
Category: Feature request » Task
Priority: Normal » Major
bojanz’s picture

bojanz commented
StatusFileSize
new 2376785-5-use-extensiondiscovery.patch4.34 KB

The ExtensionDiscovery initialization was missing the $root parameter and crashing, changed 

$listing = new ExtensionDiscovery();

to

$listing = new ExtensionDiscovery(\Drupal::root());

The patch works fine for me now.

bojanz’s picture

bojanz commented
Status: Needs review » Reviewed & tested by the community

You have the honors, Pedro.

bojanz’s picture

bojanz commented
Status: Reviewed & tested by the community » Needs review

I'm seeing notices on admin/modules saying "Notice: Undefined index: name in system_sort_modules_by_info_name()".
Will investigate.

bojanz’s picture

bojanz commented
Status: Needs review » Reviewed & tested by the community

Confirmed to be unrelated to the patch, all fine now.

  • pcambra committed 2bea74b on 8.x-1.x 
    Issue #2376785 by bojanz, pcambra: Avoid using moduleHandler for...
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Reviewed & tested by the community » Fixed

And we're in! Thanks @bojanz :)

Status: Fixed » Closed (fixed)

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