InstallerController::activateModule() assumes project id and extension ID will match

Created on 19 December 2022, over 2 years ago
Updated 6 June 2023, about 2 years ago

Problem/Motivation

In \Drupal\project_browser\Controller\InstallerController::activateModule()

There is $this->moduleInstaller->install([$project_id]);

But there is no guarantee that a drupal project will have an extension with the same name. This is usually the case but that is just by convention.

for instance if project name is my_module the extension could be mymodule or giraffe(though the first would be more commonπŸ˜‰)

Steps to reproduce

Proposed resolution

After installing a new drupal project foo into the active directory

Some pseudo code

$extension = ModuleLIst::get('foo');
// We don't know what to install
$install_extension = NULL;
$package = getPackageforProject('foo')
$package_path = getPathForPackage($package)
if ($extension) {
   $extension_info_path = $extension->path
   if ($extension_info_path  under $package_path) {
       $install_extension = $extension
  }
}
if (! $install_extension ) {
   // We got here because either
   // 1. there is no module named 'foo' (maybe it is 'foos' or other some other totally unrelated string.
   // 2. There is a module named 'foo' but it is not in the package we just installed. This is very unlikely but possible. 
   
   // At this point we can hope there is just 1 info file in the base directory of the package. If so we can install that 1.
 

   // Probably don't do a recursive search, just search base directory
   $all_info_files = getAllInfofilesinPath($package_path)
   if (count($all_info_files) === 1) {
       $info_file = $all_info_files[0]
        $install_extension = getExtensionForInfoFile($info_file)
   }
   else {
      // 😭 There either
      // 1. multiple info file files in the base directory
      // 2. 0 info files in the base directory, but maybe all in sub-folders.(at this point we could search and see if there is just 1 info.yml in the whole directory and use that🀷)
      // We don't know which to install. This situation is probably unlikely but possible
   }
}

Remaining tasks

  • βœ… File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?
πŸ› Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    This probably needs either a significant refactor or a complete rewrite, due to changes in Package Manager's API.

    Discussion with @tedbow reveals this:

    - It's probably an edge case that the project name and the extension name don't match. So this issue is already not top priority.

    After applying the install stage:

    - If there is no extension matching the project name, then we need to use Package Manager to scan for which package contains the given project name.
    - Within that package, if found, we scan for info files.
    - If only one info file is found, that's the extension name.
    - Otherwise, we've found multiple extensions in that package and we have no idea which to enable. Error? Redirect? Exception? You decide...

  • First commit to issue fork.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 2 years ago
    Custom Commands Failed
  • @fjgarlin opened merge request.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 2 years ago
    Custom Commands Failed
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 2 years ago
    Custom Commands Failed
  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    ComposerUtility was removed here: https://git.drupalcode.org/project/automatic_updates/-/commit/ec8949c926...

    I'm trying to refactor this part.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 2 years ago
    Custom Commands Failed
  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine
  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine

    The work here is maybe woefully outdated and may need to be "ported" to 2.0.x and the Activator system. That said, there is still the possibility that a project_machine_name might not match the underlying module_machine_name.

    For complex scenarios, we really should just lean on recipes, but a small improvement here would increase the reliability of Project Browser.

    Updating IS.

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    If this gets changed at all, it will be done in ModuleActivator.

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    Has anybody actually reported this bug in practice?

    I feel like what would be best here is to close this issue and wait for someone to run into it in the real world. Maybe it's just a non-issue now.

  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine

    Yeah, that's a good point. The only example I recall was the old "googleanalytics" module (note, no underscore) and this has been repaired (and the whole module deprecated, anyway). I know there are others, but I think recipes probably obviates the need for this a little bit anyway.

    I'll leave the issue open with history, but PP it.

    Maybe @fjgarlin could run a query to tell us how widespread a problem this is... something like "find all module projects who don't have a [their_project_name].info.yml" and we can find that as a percentage of projects.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    We have no way of querying this as this would be code inside projects.

Production build 0.71.5 2024