pm:enable ignoring dependency versions

[FatherShawn]

Describe the bug
Drush 9 installs a module that Drupal blocks for installation on admin/modules.

To Reproduce
Setup a Drupal 8.5.6 install
Create a bare bones module - just the info.yml file is needed.
Specify a dependency with version:

dependencies:
  - drupal:system (>=8.6)

Expected behavior
Drush should decline to install because the dependency is not satisfied.
Drupal displays:
Requires: System (>=8.6) (incompatible with version 8.5.6)

Actual behavior
Drush installed the module.

Workaround
None. I attempted to enforce with a redundant hook_requirements in a .install which also did not prevent drush en from installing but did function properly for UI install. Are all requirements being skipped? Again, UI output was This module requires Drupal 8.6 or higher. Drupal 8.5.6 currently installed and module did not install.

function test_module_requirements($phase) {
  $requirements = [];
  if ($phase != 'install') {
    return $requirements;
  }
  list($major, $minor, $patch) = explode('.', \Drupal::VERSION);
  if ($minor < 6) {
    $requirements['map_widget'] = [
      'title' => t('Map Widget'),
      'description' => t('This module requires Drupal 8.6 or higher. Drupal @core currently installed', ['@core' => \Drupal::VERSION]),
      'severity' => REQUIREMENT_ERROR
    ];
  }
  return $requirements;
}

System Configuration

Q	A
Drush version?	tried both 9.3.0 & dev-master 44f3d5e
Drupal version?	8.5.6
PHP version	7.1.15
OS?	Mac running drush in docker (Alpine Linux)

[weitzman]

I know we check requirements for updatedb. Its possible we don't for enable. Needs research.

[FatherShawn]

Do you want to point me at the relevant class? I'm happy to work on a patch or rather PR 😉 .

[weitzman]

See top of \Drush\Drupal\Commands\pm\PmCommands::enable. The code we need is similar to https://github.com/drush-ops/drush/blob/master/src/Commands/core/UpdateDBCommands.php#L43-L48

[alexpott]

Just ran into this myself. V.happy to see this already here.

[alexpott]

Note the "magic" in core occurs in \Drupal\system\Form\ModulesListForm::buildModuleList(). What is super funky is that it invokes a module's hook_requirements() even though it is yet to be installed. See drupal_check_module()

[mxr576]

Just bumped to the same problem as well.

[mxr576]

I haven't checked nowadays how Drush works exactly, but thanks for this issue and @alexpott's last comment I took a quick look the code behind the pm:enable command. (Also related addInstallDependencies).
It seems odd to me that Drush has to re-invent some wheels or event do custom stuff instead of calling Drupal APIs - even for checking requirements.
Not sure whether it is possible without a huge BC break, but I would rather move the requirements check to the ModuleInstaller service as a new method and it should be automatically called before ModuleInstaller::install() to ensure a module can be only enabled when all its install requirements are fulfilled. Just like uninstall validators, there will be new install validators and the requirements check could be a built-in install validator.

[weitzman]

Its quite possible that the code no longer needs to be as complex as it is. Feel free to write a new implementation. We are planning to release a new major version soonish anyway (drops PHP5, backend invoke, etc.).

[weitzman]

Closed in favor of #4337