Extension installers allow extensions with duplicate names to be enabled

That is definitely related to this. Should we even mark it as a duplicate for this?

Having a module and a theme with the same name is not supported and will result in a completely broken site.

http://drupal.stackexchange.com/questions/152809/install-a-custom-empty-...

Instead of trying to do something about that here, we should just check for that and prevent that from happening in the first place.

Right, both the theme installer and the module installer could check whether there is some other extensions with the same name available and throw an exception or something else, in case that happens.

Yes, lets change it to that then.

I added a test on the ThemeInstaller. We will need a test of the module installer.

1. +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
   @@ -81,6 +81,13 @@ public function addUninstallValidator(ModuleUninstallValidatorInterface $uninsta
   +    $installed_themes = $extension_config->get('theme') ?: array();
   +    $duplicates = array_intersect(array_keys($installed_themes), $module_list);
   +    if ($duplicates) {
   +      throw new ExtensionDuplicateNameException($duplicates);
   

   And

2. +++ b/core/lib/Drupal/Core/Extension/ThemeInstaller.php
   @@ -104,6 +104,11 @@ public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryI
   +    $installed_modules = $extension_config->get('module') ?: array();
   +    $duplicates = array_intersect(array_keys($installed_modules), $theme_list);
   +    if ($duplicates) {
   +      throw new ExtensionDuplicateNameException($duplicates);
   +    }
   

   ---

Doesn't it look like we are duplicating samething (I know that its one for theme and another for module) ?

We may want to implement a trait or something.

#371375: Do not allow a module and theme to use the same name.
#2375731: update_fix_compatibility() doesn't respect theme/module naming collisions

Just to note, the fix on D7 is being worked on under #2375731: update_fix_compatibility() doesn't respect theme/module naming collisions.

Tests seem to fatal on testClassLoading, re-triggering testbot.

needs reroll & fix

+++ b/core/modules/system/src/Tests/Extension/ThemeInstallerTest.php
@@ -141,6 +142,22 @@ function testInstallNameTooLong() {
+  function testInstallDuplicateName() {
...
+    try {
...
+      $this->themeInstaller()->install(array($name));
+      $this->fail($message);
+    }
+    catch (ExtensionDuplicateNameException $e) {
+      $this->pass(get_class($e) . ': ' . $e->getMessage());

Better to use @expectedException

Rerolled and fix the patch #26 but without test

@adev apparently the patch in #28 is failing with the error core/modules/system/src/Tests/Extension/ThemeInstallerTest.php: No such file or directory... it's possible that test was moved between 8.8.x and 8.9.x. git log --follow -- 'core/modules/system/src/Tests/Extension/ThemeInstallerTest.php' might help track down what happened to it.

Your patch also includes several older patch files from this issue, which aren't necessary to include. If you're interested, I wrote my best practices for generating patches and interdiffs a few years back: I apologize in advance for its verbosity, but I still refer to it when writing patches myself.

Trying to fix #28

Re-roll for 9.1.x-dev

Returned to #10 to re-roll for D9.1.x

Returning to Needs Work to address comments in #26.

Returning to Needs Work to address comments in #26.

* Address comments in #26.
* Fixed coding standards violations highlighted in #33.

The current work on this issue is very much a duplicate of the work on #371375: Do not allow a module and theme to use the same name.. I'm new to both of these issues so I don't feel comfortable closing either of them.

I prefer the exception message on the other issue and I prefer the tests on the other issue.

Another difference in the patches is the location of the code block that checks for duplicates. In this issue the code block is near the beginning of the install function. In the other issue the code block is closer to the end of the install function. Not sure if that matters to anyone.

Neither issue appears to deal with profiles at this point.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Added patch against #35 in 10.1 version

Setting as need review for tests to run.

@Akhil Yadav Once you have uploaded the re-rolled patch, change the status to Need Review so the test-bot runs tests against the patch.

Missing core/lib/Drupal/Core/Extension/ExtensionDuplicateNameException.php

in the patch #43 present in #35 . Hiding patch #43.

#35 still applies to 10.1.x

Closing this as a duplicate of #371375: Do not allow a module and theme to use the same name. there is a (at least at one point) green patch there with test coverage for both cases (module and theme)

I also can't see any code in this issues patch regarding profiles so maybe another issue should be opened just for that.

Triaged as part of Bug smash