When drupal_check_profile() checks installation requirements, it doesn't check the type of the values returned from hook_requirements implementations before sending them to array_merge(). This results in an error if an implementation doesn't return a value for the 'install' phase.

e.g.

function mymodule_requirements($phase) {
  if ($phase !== 'runtime') {
    return;
  }
  
  return [
    // ...runtime requirements...
  ];
}

The current check:

if (function_exists($function)) {
  $requirements = array_merge($requirements, $function('install'));
}

For comparison, ModuleHandler::invokeAll() only merges if isset() passes:

if (isset($result) && is_array($result)) {
  $return = NestedArray::mergeDeep($return, $result);
}
elseif (isset($result)) {
  $return[] = $result;
}
CommentFileSizeAuthor
#2 drupal-2942014-2-drupal-check-profile.patch575 bytesgapple

Issue fork drupal-2942014

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

gapple created an issue. See original summary.

gapple’s picture

Assigned: gapple » Unassigned
Status: Active » Needs review
StatusFileSize
new575 bytes

This patch allows hook_requirements('install') implementations to return null without causing an error.

joelpittet’s picture

IMO it's nice when we know to always expect a single type of value and don't have to put more code to deal with those. The return value from that hook is always an array so I'd rather keeping what we have and the fix you did in the contrib module is the way to go. Others may disagree...

joachim’s picture

Status: Needs review » Needs work

The documented return value of hook_requirements() is an array.

So currently, this is a wontfix, as @joelpittet says.

If we want to allow a return of NULL, then we need to change more than this. We need to change:

- documentation
- everywhere that invokes the hook

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lendude’s picture

Category: Bug report » Feature request
Issue tags: +Bug Smash Initiative

As pointed out in #3 and #4 the current documented return type is an array, so this would not be a bug. Moving to a feature request, but I agree with #3 and #4 that I'm not sure we should support this,

gapple’s picture

My justification is that currently hook_requirements('runtime') is fine with implementations returning NULL, due to the behaviour of ModuleHandler::invokeAll(), while hook_requirements('install') is not because it handles calling implementations slightly differently.

I could settle for improving the documentation to explicitly specify that an empty array must be returned if a phase does not have any requirements. I think ideally a better exception would be thrown so that it's easier to diagnose and resolve the issue if someone attempts the early return pattern like I did (so they know to return []; without looking at core's code).

gapple’s picture

(I think there's potentially an argument for separating hook_requirements('runtime') to a separate hook_status() or such, since 'install' and 'runtime' execute with different access to Drupal APIs, and while 'install' and 'update' can halt execution of their respective operations, 'runtime' is only able to display information on the site status page)

gapple’s picture

Looking at the code for other invocations of hook_requirements, install.inc:drupal_check_module() already has a check against the returned value's type:

  $requirements = \Drupal::moduleHandler()->invoke($module, 'requirements', ['install']);
  if (is_array($requirements) && drupal_requirements_severity($requirements) == REQUIREMENT_ERROR) {
    // ...
  } 

update & runtime use invokeAll which is fine NULL:

update.inc:update_check_requirements():

  $requirements = \Drupal::moduleHandler()->invokeAll('requirements', ['update']);

SystemManager::listRequirements():

    $requirements = $this->moduleHandler->invokeAll('requirements', ['runtime']);

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.