Add a module and theme install command

The drush pm:install command provides a nice interaction model where it automatically enables the module(s) unless there are dependent modules also need updating and were not mentioned. It also checks various requirements before the install starts. I think these are nice and ideally will stay when this command ports to core. In order to achieve this, the ModuleInstaller will need refactoring.

Also, we may or may not want to bundle theme and module install into one command. Drush has them separate but thats a historical artifact not a design decision.

Help wanted.

I'm not opposed to some refactoring to make this more resilient.

I would like to get the command and ui install process closer rather than further apart unless there is a strong reason the cli should act differently.

Also, module installer needs the ability to process batch operations, for example for locale translation import. We could in theory get in a first iteration without that, but it would result in different behavior between CLI and UI. That UI helper method would need to be split from the form state to work for CLI as well.

The core logic for install requirements and dependencies is in \Drupal\system\Form\ModulesListForm::buildModuleList/submitForm, There's also a special case for non-stable module confirmation in the UI.

I think the command-line should only enable a module and not to try to pipe in additional tasks, like importing translations.

So we could do something like enable a few modules and after that import the translations separately.

$ dr en module1 module2 module 2
$ dr import:locale

I pushed an MR for this.

1. I focused on good UX, similar to Drush's UX. I settled on one command for both modules and themes (unlike Drush). However, we disallow providing a theme and module in same call. This simplifies the code. In my experience this need is very rare and easily worked around by running twice.
2. Now 11.x compatible. The MR as it stands is targeted to Drupal 12. Once that gets in, someone can work on an 11.5 version IMO. This is chosen in order to allow refactoring of dependency checking, which unlocks the strong validation/user-prompting
3. The command does not run a batch at the end (e.g. locale translation import). I'd prefer that be a followup - It seems complex to get batches working in the CLI (non-progressive, fork to sub-processes, etc). This code is rather old and ugly in Drush.
4. "There's also a special case for non-stable module confirmation in the UI. ". This is incorporated into the command In the interest of keeping this small, perhaps this can also be deferred. I personally think this warning is a bit nanny-ish and not as appropriate for a CLI user.
5. Done.Drush emits clickable links at end of module install with links to help, permissions, and config pages. This is left as a followup, partly because of the difference between modules and themes.

AI-Generated: Code - Yes ... All issue and MR comments are fully authored by me, because that is respectful of the reviewers and shows to myself and others that I understand what I am contributing. This disclosure also appears in the MR description.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

Back to NR

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

As for interactively prompting the user to pick a module/theme to install, I opened #3606645: Add Laravel Prompts as a dependency for CLI app and affiliated it with the meta.

The NR bot is apparently not using the same PHPCS that Gitlab CU uses so the PHP 8.5 pipe operator confuses it

FILE: ...rktrees/slot-0/core/modules/system/src/Command/ExtensionInstallCommand.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 65 | ERROR | [x] Expected 1 space after "|"; 0 found
 65 | ERROR | [x] Expected 1 space before ">"; 0 found
 66 | ERROR | [x] Expected 1 space after "|"; 0 found
 66 | ERROR | [x] Expected 1 space before ">"; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 234ms; Memory: 16MB


PHPCS: failed

Thanks @moshe weitzman, the new dr command works well, and the only thing I have encountered so far trying out the install command is that only a comma-separated list of modules is supported, where Drush also supports a space separated list.

These two (space, or comma+space) fail with the same error message:

$ ddev exec dr ex:install basic_auth serialization
$ ddev exec dr ex:install basic_auth, serialization
  Too many arguments to "ex:install" command, expected arguments "extensions".  
ex:install [-y|--yes] [--dry-run] [--] <extensions>
Failed to execute command `dr ex:install basic_auth, serialization`: exit status 1

This one (only comma) succeeds:

$ ddev exec dr ex:install basic_auth,serialization
The following modules will be installed:
 * HTTP Basic Authentication (basic_auth)
 * Serialization (serialization)
 [OK] Successfully installed 2 modules: basic_auth, serialization.

The drush install command supports multiple modules, either space or comma separated

$ drush install basic_auth serialization
 [OK] Module basic_auth has been installed.
 [OK] Module serialization has been installed.

It would be great if dr install also supported installing multiple space separated modules, if possible?

I deliberately did not support space separated arguments (also called a variadic argument), even though Drush does support that. The issue with them is that you can never add another argument after that. Forever more you have to assume that any argument to the command is an extension name. Thats not the end of the world. Yet in Drush a few times I wished I didn't have that limitation. I can make this change if folks think its important.

How can I coax the Needs Review Bot to ignore this issue? Its vandalizing the Status field.

Add this tag to skip the bot.

Thanks for a fast reply and explanation about the comma-separated argument values, I had a hunch that it was probably deliberate, but wanted to check. I do understand that there are downsides to supporting space-separated modules ... it's complicated to discern between values and arguments.

We could keep it like it is (comma-separated), but possibly either 1. Improve the error message, making it clearer that only a single comma will work as separator, or 2. Add an example string like ex:install basic_auth,serialization in the output of dr ex:install -h? Because some might try comma+space, which works in Drush. If not, it's fine, people will realize the new syntax over time :)

I opted to add a couple usage example. However that exposed a bug in our handling of invokable commands. I submitted a fix at #3606744: Invokable commands dont show their usages in help

The improved "Usage" example looks great, and the added ex:install views,views_ui will work well to inform the users of the correct syntax, thanks!

$ dr ex:install -h
Description:
  Installs one or more modules or themes.

Usage:
  ex:install [options] [--] <extensions>
  exin
  pm:install
  pm:enable
  enable
  en
  ex:install views,views_ui
  ex:install claro
[...]

Why so many aliases for the same set of commands?

pm:install vs ex:install

As I can see, pm and ex are doing the same thing.

We actually dropped one in the MR. pm:install has 6 aliases. ex:install has 5. Unlike Drush Help, Console Help shows Usages and Aliases mixed together.

Thinking more about this, I am not a fan of the ex:install and its exin alias.

And while I understand the desire to use install for installing Drupal, I think it is better used for installing modules and themes, since that command is used much more often, in daily Drupal development.

Can we instead use install-drupal for installing Drupal?

$ dr install-drupal -h
Description:
  Install Drupal using an install profile or recipe.

Usage:
  install-drupal [options] [--] [<install-profile-or-recipe>]
  install-drupal demo_umami --langcode fr
  install-drupal standard --site-name QuickInstall
  install-drupal core/recipes/standard --site-name RecipeBuiltSite

... because then, while we are changing things, we can clean up the command for installing modules, to just be either install or its alias in, getting rid of old cruft, such as pm:install and enable (which is misleading) and mainly there for historical reasons:

$ dr install -h
Description:
  Installs one or more modules or themes.

Usage:
  install [options] [--] <extensions>
  in
  install views,views_ui
  install claro
[...]

@ressa I'm not opposed to this discussion, but installing a module is what happens when you run composer require or install commands.

Enabling is what this command does.

Install is also what happens with what you are proposing become install-drupal.

The aliases are really just for muscle memory, I don't think it's really craft to keep them.

Thanks for weighing in @nicxvan. But actually, the difference between these concepts is something many new users mix up, I have seen it often in the Drupal forum. I left this comment April 2024 to clear it up:

Download ≠ install

It's important to discern between downloading and installing modules, they are two very different things in Drupal.

1. You download the module and its additional necessary libraries (defined in the module's composer.json file) with Composer.
2. Then you install the module either via the GUI on the Extend page, or with Drush -- the module gets enabled, changes are made to the database (the configuration), necessary folders are created, etc.

[...]

From https://www.drupal.org/forum/support/installing-drupal/2024-04-26/need-c...

... and to add, we delete a module/theme's code base with composer remove and uninstall with Drush or GUI. Important note: when you uninstall a module, all configuration gets deleted. In Drupal 7 it was kept, which is why enable/disable were correct terms for Drupal 7, but not for Drupal 11.

Yeah the services are called moduleInstaller and themeInstaller too.

Personally, I would keep the 'install' lingo only for "composer install" when a Composer lock file exists..

For modules/themes, the instlallation is a two-step process :

1. Download (composer based)
2. Enable (UI or CR based)

So I prefer to keep enable:

cr enable module1,module2
cr en module1,module2

and get rid of cr install.

Edit: I hope that in the future D13, we will be able to enable/disable a module like we did in Drupal up to 7. Or provide a way to export the directly related configuration (not the dependency tree) of a module when we uninstall a module. But this is another story.

It's unfortunate that Composer is using "install" as well ...

But the Drupal User Guide strictly module page uses install, and it is a conscious choice, see #2891294: [Meta] Use Install/Uninstall consistently for turning modules/themes on/off (not Enable) and also Installing Modules.

Because if we use enable, we should also use disable for uninstalling a module. But that's not the only thing that happens, all the configuration is removed.

Thanks for finding that. Now fixed. The fix was to flip that $allow_dumping DrupalKernel param that we discussed in the original issue. That necessitated a consolidating duplicate applicationTester() methods used in 2 test classes. $allow_dumping now defaults to TRUE like Drush has done for years. This fixes quick-start as well.

The CI warning is unrelated to this PR, and is happening on other pipelines.

This seems related #3611084: DrupalApplication should use the compiled container

Oh wow. That issue contains an identical fix. I guess we should mark this one as Postponed on that. I'll leave the fix here too until that one gets in.

The dependant issue was merged, so I removed my commit and pulled changes from main.

Back to NR. Again, the CI warning is unrelated.

I was able to do some manual testing.

I installed a module by itself
I called two modules, one already installed
I installed a theme
I installed a module with dependencies
I installed and already installed module
I installed a theme with dependencies
I installed a module and theme at the same time (gave appropriate error)

Do we need a confirmation for experimental modules and themes?

I have to still go through the code flow in detail again.

Do we need a confirmation for experimental modules and themes?

We have never had that for Drush, and it never got any pushback. I suggest that be proposed in a new issue by anyone who feels strongly for it.

Anyone up for an RTBC?

I think the experimental module confirmation can be a follow up.

If someone rtbcs before I have a chance to review again I'd like confirmation they tested the ui install process too for modules and themes.

I didn't do that on this branch yet and we are changing that code as well.

Ok I tested the scenarios that made sense in the UI:

I installed a module by itself - Works
I called two modules, one already installed - N/A
I installed a theme - Works
I installed a module with dependencies - Works
I installed and already installed module - N/A
I installed a theme with dependencies - Works
I installed a module and theme at the same time - N/A

I also checked the following items in the UI and CLI
Experimental themes, Experimental modules.
Both worked as expected, no confirmation prompt on the CLI, but that is fine as mentioned in 39 I think.

Got a question on some of the changes in the ModuleConfirmForm, I'm not sure how many of those changes are necessary in general.

Other than that I did a lot of manual testing including config import.

We need to figure out where the CR will go, if there is a new one for 11.5 already for CLI commands.

Also we need the follow up for the batch processing for translations.

We need to figure out where the CR will go, if there is a new one for 11.5 already for CLI commands.

I have no pref. A new one for 11.5 sounds good. Do we usually do a CR for new features? Seems more like a release highlight than a change.

I have created #3613834: Install translations after installing a module/theme via CLI.

I think this is ready now!

It's a little bit bigger than I'd like, maybe we could have split the module and theme install into two issues, but this helps make sure the refactoring is consistent across the two.

I provided a couple of suggestion that were applied to the ModuleConfirmForm to streamline the changes a bit.

There is a follow up for translations here: #3613834: Install translations after installing a module/theme via CLI

And we still need to figure out which CR it goes in, I added a remaining task for both.

... and pm:install should probably also be let go, since the pm: prefix is no longer used. In fact all pm:-prefixed aliases should go, but that's probably for other issue(s).

Letting go of the four old aliases would result in short and precise documentation.

Command help

$ dr ex:install -h
Description:
  Installs one or more modules or themes.

Usage:
  ex:install [options] [--] <extensions>
  exin

General help would be much more readable

$ dr
Drupal 12.0-dev
[...]
 ex
  ex:install      [exin] Installs one or more modules or themes.
[...]

Currently, it starts with a long list of aliases, pushing the actual documentation text far to the right:

$ dr
Drupal 12.0-dev
[...]
 ex
  ex:install      [exin|pm:install|pm:enable|enable|en] Installs one or more modules or themes.
[...]

Do we need a separate issue to look into support for deprecated aliases - e.g. we don't list them in the UI, they work, you get an annoying message that they're going to go away etc?

That's a great idea @catch, and I created an issue.

About this install command, @mradcliffe commented:

I'm wondering, if in a follow-up, we could deprecate the drush aliases pm:install, pm:enable, enable and en.
I guess that could be done by doing $input->getFirstArgument(), and issuing a deprecation notice.
I don't want to hold this mr up for that though.

That sounds great, could that be handled in the issue I just created, or perhaps needs a new issue?

Yes, let's handle this in the issue you created, it's worth it's own discussion and we can pivot after.

I think getting some of these commands in first is far more important.

Thanks for fast feedback, let's keep this issue moving.

It's not clear to me why we're not adding the new methods to the interfaces. This is allowed in minor versions where there's a 1-1 relationship between the interface and the class per https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...

It's not clear to me why we're not adding the new methods to the interfaces

I think I was trying to avoid the disruption, but even though I felt strongly about it before I'm struggling to justify it now again.

If it's just a matter of adding the interface docs and method then I think we should do it, sorry @moshe.