backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

module_enable() does not always enable all modules.

Open jenlampton opened this issue 6 years ago • 13 comments

Describe your issue or idea

In doing upgrades from Drupal 6 and Drupal 7, I've found that the module_enable() function does not enable all modules during updates. (I also tried using update_module_enable() but that didn't run the necessary intsall hooks.

When I called module_enable() only a single time in my update, no modules were enabled at all:

$module_list = array(
    // Modules that have dependancies.
    'flexslider_fields',
    'rh_node',
    'recaptcha',
    // Core modules.
    'ckeditor',
    // Contrib modules.
    'dropdown_breadcrumbs',
    'sharethis',
    'smart_trim',
    'smtp',
    // Sub-modules.
    'metatag_opengraph',
    'metatag_twitter_cards',
    'metatag_verify',
    // Custom modules.
    'bgp_blocks',
    'bgp_views',
    'iw_blocks',
  );
  module_enable($module_list);

But when I provided two calls to module_enable() only the modules specified in the second call (and their dependancies) were enabled...

function custom_update_1000() {
  // Enable modules/themes that are new on Backdrop.
  $module_list = array(
    'bgp_blocks',
    'bgp_views',
    'captcha',
    'ckeditor',
    'dropdown_breadcrumbs',
    'flexslider',
    'iw_blocks',
    'metatag_opengraph',
    'metatag_twitter_cards',
    'metatag_verify',
    'rabbit_hole',
    'sharethis',
    'smart_trim',
    'smtp',
    'views_ui',
  );
  module_enable($module_list);

  // Enable modules that depend on above.
  $module_list_next = array(
    'flexslider_fields',
    'rh_node',
    'recaptcha',
  );
  module_enable($module_list_next);

  return t('New modules enabled.');
}

I'm not sure why the first call to module_enable() doesn't enable modules, but we should look into this further.

jenlampton avatar Apr 03 '18 19:04 jenlampton

Nothing relevant in the watchdog / logs ?

My guess is that you have to much modules to enable at the same time. Enabling a module is a pretty expensive operation. I quickly look at what core do when installing a full profile (https://api.backdropcms.org/api/backdrop/core%21includes%21install.core.inc/function/install_profile_modules/1), and it seems to use a batch.

Try to split your update hook in multiple hooks (1000, 1001, 1002...). It will use the batch api automatically.

opi avatar Apr 04 '18 06:04 opi

I'm having the same issue I think. My install profile is set to enable a bunch of modules, but I get an error during installation and when I finally access the site, nothing's enabled...

ghost avatar Apr 16 '18 12:04 ghost

I'm not sure it's a batch issue, as I've just added batch processing to my module enabling function, and it works better now, but it's still only enabling about 15 core modules before it stops enabling any more. There's no error messages or anything...

ghost avatar Apr 16 '18 12:04 ghost

BTW, you can see the list of modules I'm working with here: https://github.com/PackWeb/pw_profile/blob/480639293fae636a55f32808538be060545a5610/install/pw_profile.modules.inc (that's a link to an older commit as I made a new commit as a result of this issue).

I've since changed that module_enable() call to an array instead, and added the following to the bottom of that file:

$operations = array();
foreach ($modules as $module) {
  $operations[] = array('_pw_profile_enable_module', array($module));
}
$batch = array(
  'title' => t('Enabling modules'),
  'operations' => $operations,
  'file' => backdrop_get_path('profile', 'pw_profile') . '/install/pw_profile.modules.inc',
);
batch_set($batch);
batch_process();
function _pw_profile_enable_module($module) {
  module_enable(array($module), FALSE);
}

ghost avatar Apr 16 '18 12:04 ghost

If I put everything back to normal (see the link to my install profile code - e.g. no batch processing), and simply add FALSE to the module_enable() call, it seems to work fine...

ghost avatar Apr 16 '18 13:04 ghost

I tried that too, and for me it worked better, but there were still a few modules that wouldn't enable, so I also put things back.

@opi I had also tried doing separate calls to module_enable() in separate update hooks, still not all modules were enabled.

I ended up enabling the modules I needed using the UI, since they also wouldn't enable with Drush. I wonder if this is related to the problems we are having with core, custom, and sub-modules not enabling with drush. @serundeputy

jenlampton avatar Apr 16 '18 18:04 jenlampton

could be ... drush is definitely leveraging the module_enable() function form backdrop core ... seems we need some investigation here.

serundeputy avatar Apr 19 '18 03:04 serundeputy

...could #3177 be related?

klonos avatar Jul 12 '18 22:07 klonos

Just experimenting with a recipe and I'm experiencing the same issue.

It's not working... even when I add one module to the list...

`function albany_dfy_website_bertie_install() {

$module_list_single = array ('addanother');

$module_list_full = array (
	'addanother',
	'backup_migrate',

*** more deleted *** );

module_enable($module_list_single, TRUE);

}`

According to the Docs... Enables or installs a given list of modules."

Order of events: Gather and add module dependencies to $module_list (if applicable).

When I Debug the code...

`function module_enable($module_list, $enable_dependencies = TRUE) { if ($enable_dependencies) { // Get all module data so we can find dependencies and sort. $module_data = system_rebuild_module_data(); // Create an associative array with weights as values. $module_list = array_flip(array_values($module_list));

// The array is iterated over manually (instead of using a foreach) because
// modules may be added to the list within the loop and we need to process
// them.
while ($module = key($module_list)) {
  next($module_list);
  if (!isset($module_data[$module])) {
    // This module is not found in the filesystem, abort.
    return FALSE;
  }`

The module "addanother" isn't in the list so it returns FASLE...

The impression the docs gives, is if the modules isn't in the list it should add it to the array and install...

This isn't happening.

albanycomputers avatar Mar 26 '24 14:03 albanycomputers

The looks like if one of the modules in the array is not found, then it will exit out of the function...

So it's an "all or nothing" function.... is this the correct behaviour?

albanycomputers avatar Mar 26 '24 15:03 albanycomputers

It is the behavior documented in the code.

if (!isset($module_data[$module])) {
  // This module is not found in the filesystem, abort.
  return FALSE;
}

avpaderno avatar Mar 26 '24 16:03 avpaderno

It is documented in the return value description too, excepts the code consider dependencies also all the modules passed in $module_list (which is not probably what I would expect).

Returns FALSE if $enable_dependencies is TRUE and one or more dependencies are missing. Otherwise returns TRUE.

avpaderno avatar Mar 26 '24 16:03 avpaderno

To make it clearer, these are the first lines in module_enable().

  if ($enable_dependencies) {
    // Get all module data so we can find dependencies and sort.
    $module_data = system_rebuild_module_data();
    // Create an associative array with weights as values.
    $module_list = array_flip(array_values($module_list));

    // The array is iterated over manually (instead of using a foreach) because
    // modules may be added to the list within the loop and we need to process
    // them.
    while ($module = key($module_list)) {
      next($module_list);
      if (!isset($module_data[$module])) {
        // This module is not found in the filesystem, abort.
        return FALSE;
      }
      if ($module_data[$module]->status) {
        // Skip already enabled modules.
        unset($module_list[$module]);
        continue;
      }
      $module_list[$module] = $module_data[$module]->sort;

      // Add dependencies to the list, with a placeholder weight.
      // The new modules will be processed as the while loop continues.
      foreach (array_keys($module_data[$module]->requires) as $dependency) {
        if (!isset($module_list[$dependency])) {
          $module_list[$dependency] = 0;
        }
      }
    }

    if (!$module_list) {
      // Nothing to do. All modules already enabled.
      return TRUE;
    }

    // Sort the module list by pre-calculated weights.
    arsort($module_list);
    $module_list = array_keys($module_list);
  }

All that code is not executed when the second argument is FALSE.

avpaderno avatar Mar 26 '24 16:03 avpaderno