Problem/Motivation

#3605597: Install profiles can opt into being automatically uninstalled allows install profiles to be automatically uninstalled at the end of the installer. When an install profile has no hooks, we can probably skip install altogether.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3614401

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

catch created an issue. See original summary.

catch changed the visibility of the branch 3614401-allow-install-profiles to hidden.

nicxvan’s picture

Oh this is fun.

catch’s picture

Status: Active » Needs review

MR is green. It includes the changes from #3614153: Install system module alongside other modules in the installer for now to try to gauge the impact of both.

The testing profile isn't quite as minimal as it claims - it's got optional default config for the body field which we put there when we moved it out of node module. It also has a locale config override to make locale testing easier. Because these optional config only get installed when the module they depend on gets installed, they need the testing profile to be installed throughout the lifecycle of the test (e.g. that config doesn't actually get created in the installer at all because the testing profile doesn't depend on field or locale modules).

Moved the body field to a testing module. About 10-15 tests in core actually need that.

Created a new testing_locale profile with the locale config. Also using this in a couple of tests that check that the profile is installed after the installer. Bit of a mis-use of the new profile but really those tests don't care which profile they use as long as there is one.

We probably need an explicit test for the opt out too, can probably copy and paste one of the installer tests we had to change and just alter a couple of assertions for that.

catch’s picture

Assuming we can do this or something like this, there's a further step we can take I think.

In HEAD there are up to four distinct module installs per functional test before we reach ::setUp().

1. System module in drupal_install_system

2. Profile modules in install_profile_modules, along with any required modules.

3. The install profile itself installed as a module in install_install_profile

4. Modules specified in the $modules static property.

#3614153: Install system module alongside other modules in the installer moves user and path_alias to the drupal_install_system() step so that if a profile doesn't specify any modules at all, step #2 doesn't need to happen.

This issue allows an install profile to skip itself in step #3.

There are two next steps I think:

1.If we can do #3428372: [PP-1] Stop installing system module before everything else, that will allow the system/user/path_alias install to move to step #2. So that all required modules are installed there along with the install profile modules. That gets rid of step #1 so that when an install profile specifies modules, it's a single install for all of them.

2. We could potentially add a support for an $extra_modules install state parameter to the installer, and populate with modules from the $module static property, and merge that with the profile module list in step #2.

That would get us down to a single ModuleInstaller::install() call for the majority of functional tests.

catch’s picture

catch’s picture

catch’s picture

Rebased and added a couple of CRs.

nicxvan’s picture

This looks great, I tweaked the CRs.

I almost hate to ask, but it's there a way to test this profile is never installed?

I thought through a few times and I don't see a clean way to test that but it does seem like am easy feature to lose without a test.

Adding #3576425: [meta] Profile extension improvement plan so we can track the new parameter.

catch’s picture

We can test that it's not installed at the end of the installer, which would probably be a copy and paste of the coverage added in #3605597: Install profiles can opt into being automatically uninstalled, however that wouldn't test that it's never installed.

I guess another possibility would be a special testing profile that throws an exception in hook_install(), and then if it's never installed, that exception won't get thrown, maybe something like that?

nicxvan’s picture

I guess another possibility would be a special testing profile that throws an exception in hook_install(), and then if it's never installed, that exception won't get thrown, maybe something like that?

Yes let's do that as I was reading your reply I actually had the same idea then I saw your second paragraph and saw you beat me to it.

catch’s picture

Added that test coverage.

nicxvan’s picture

Test looks great, I have two minor comment suggestions.

I've added it to the meta for documenting better too.

I think this is good to go once the comments have been applied.

nicxvan’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Looks great now!