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.By design, Drupal allows multiple copies of a module in different directory spaces (eg one in /core/modules, one in /modules, one in sites/all/modules).
However, if duplicate copies exist in the same space, this works too, though I assume this is by accident rather than by design. So for example if I have:
- sites/all/modules/foo
- sites/all/modules/contrib/foo
then the first one is taken by Drupal, and the second one is simply ignored.
However, because this behaviour is undocumented, it effectively puts the codebase in an unstable state: a developer doesn't know which module is being used by Drupal without inspecting the system table.
Having two copies like this is therefore a code error, and it would be better if discovery of a second module with the same name within the same directory space caused an exception on module discovery.
In other words, ExtensionDiscovery::scanDirectory() should keep an array indexed by just the extension name to track uniqueness, and throw an exception if it finds a second copy of the same extension name. The exception message should clearly state which module is already registered, if any, to allow a developer to know which one to remove in the case that the two are different.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | warning.JPG | 35.55 KB | Anishnirmal |
| #3 | 2647388-2.drupal.extensions-discovery-duplicates.patch | 991 bytes | Anishnirmal |
| #2 | 2647388-2.drupal.extensions-discovery-duplicates.patch | 1.43 KB | joachim |
Comments
Comment #2
joachim CreditAttribution: joachim commentedQuick attempt at what I mean.
Not sure what class of exception to throw though.
Comment #3
Anishnirmal CreditAttribution: Anishnirmal commentedDisplayed a Warning message on finding duplicate modules. Hope this helps.
Comment #4
swentel CreditAttribution: swentel commentedHmm, I've sometimes used this behavior to override core modules in a different directory and not having to patch core itself. And I think it was handy to override a module specifically for a site having it in sites/sitename/modules. Whether it's a good thing is debatable of course, and I don't really mind removing it. Throwing an exception seems a little harsh though.
However, some kind of warning, or least telling the user, or simply core knowing there are duplicates might become interesting for including new modules in core, e.g. #2296423: Implement layout plugin type in core or #2469431: BigPipe for auth users: first send+render the cheap parts of the page, then the expensive parts during minor releases and having at least an easy mechanism to find two modules.
(also, in the latest patch, we can't put drupal_set_message there, discovery shouldn't throw it there.
Comment #5
joachim CreditAttribution: joachim commentedThanks for looking at this!
However, I'm not sure it's suitable for this component to use drupal_set_message(), as it should be usable as an API, and not necessarily from the Drupal UI.
Also, if we merely set a message here, then we've gone ahead and enabled one of the duplicate modules, and the user doesn't know which. This puts the site in an unclear state, and I don't think that should be allowed.
> Hmm, I've sometimes used this behavior to override core modules in a different directory and not having to patch core itself.
I assume you mean something like having sites/all/modules/node to override core/modules/node?
That's legitimate, and wouldn't be affected here. This issue is about duplicates in the same space, eg both in sites/all/modules, or both in /modules, or both in the same profile.
Comment #7
SKAUGHTIf this is not documented behaviour or is lost documentation, @poor_document_discovery_on_d.org, it is (dare i say) 'common knowledge'. historically, same with module weight. Which i also assume still exists in 8.
Is it not still the case that the higher version number of the module will 'win out'? i've not tested this in D8, i would expect this to be true still.
ie: https://www.drupal.org/project/contact <-- sometimes we just want to completely override a core module
#4 -- yes, multisite/profile projects allow for updates to modules by a /sites/[domain]/modules installations.
Of course, due to possible hook_update()'s done by higher modules; downgrading probably shouldn't/can't be done.
if this issue was to be continued you should be able to use drupal_set_message(). however, as that same person installing may install 'a bunch of things' at once. The notice may be better off with each module, and just a general message at the top of the page. @UX
I would have to say.. In a 'one off' site, if a dev has messed up:
- sites/all/modules/foo
- sites/all/modules/contrib/foo
Certainly they have created their own issue. it was a sloppy installation by that dev (or said person who shouldn't have been installing anything..)
Comment #8
joachim CreditAttribution: joachim commented> Hmm, I've sometimes used this behavior to override core modules in a different directory and not having to patch core itself. And I think it was handy to override a module specifically for a site having it in sites/sitename/modules.
This issue isn't proposing to remove that behaviour.
Just to make it illegal to have modules with the same name in the *same* area.
Comment #22
PasqualleThis should definitely be an exception, not a warning.