Goal → MVP

  1. As a distribution profile author, I'm easily able to inform the installer that only my distribution profile is to be installed, so that (1) I do not have to fork Drupal and so that (2) downloading my distribution package from drupal.org makes any sense in the first place.

  2. As a distribution profile author, I can control the entire installer experience, including the theme, starting from the very first installer screen, so that I can tailor my product for the target audience.

  3. As a distribution profile developer, I do not have to resemble or depend on any drupal.org specific release packaging script mechanics (that may or may not happen in ~2018), so that I can simply git clone my project repository + rock on.

Proposed solution

Proudly introducing...

Distribution installation profiles

Usage

Beautiful simplicity in $profile.info.yml:

distribution:
  name: Portfolio
  install:
    theme: bartik

Anatomy

  1. The new 'distribution' info file property is optional and not required. → If no installation profile specifies it, then the installer operates as usual: Regular Drupal installer, regular profile selection, etc.

  2. The 'distribution' info file property denotes that an installation profile is a distribution, which takes over the installer. → By definition, if you install with a profile that defines a distribution name then you are no longer installing "Drupal".

    Logical reverse condition: If you no longer install Drupal, then any installation profile, whose distribution name is not "Drupal", is hence (1) a distribution and (2) to be treated exclusively.

  3. Next to the distribution name, a distribution profile can override additional installer environment parameters, such as the theme.

    Note: The goal of this patch is to establish the basic architectural functionality. Only the installer theme can be set at this point. The info file definition format is intentionally prepared to allow for other overrides. Further installer settings (langcode, translation server, etc) can be easily added and implemented in separate follow-up issues at any time (even after 8.0.0). There might also be use-cases for non-installer parameters, hence the nested 'install' key. Any additional settings should not be added prematurely; they should be based on actual distribution author feedback.

Live example

via Portfolio:

$ cd profiles
$ git clone --branch 8.x-1.x http://git.drupal.org/project/portfolio.git

→ Just hit the installer.

API changes

  1. The optional 'exclusive' (Boolean) flag and the optional 'distribution_name' property in installation profile .info.yml files have been replaced with the 'distribution' property:

    Drupal 7

    distribution_name = Commerce Kickstart
    exclusive = 1
    

    Drupal 8

    distribution:
      name: Commerce Kickstart
    

    Note: The install.php?profile= URL query parameter can still be used to manually override the automatically preselected installation profile.

  2. The behavior of the installer when multiple exclusive/distribution profiles are discovered has been changed:

    • In Drupal 7, if the installer discovered multiple 'exclusive' profiles, then no profile was preselected.
    • In Drupal 8, if the installer discovers multiple 'distribution' profiles, then the first discovered is preselected and others are ignored.
  3. Distribution installation profiles are now able to override further aspects of the installer environment:

    distribution:
      name: Commerce Kickstart
      install:
        # Theme to use in the installer.
        theme: commerce_kickstart_admin
    
  4. Distribution installation profiles now fully participate in the entire early installer environment (as if they would be a regular module).

    This means that the installation profile can implement any kind of module hook.

Notes

  1. This work is not to be mistaken with installation profile inheritance à la base themes.
    #1356276: Allow profiles to define a base/parent profile

  2. Support for multiple installation profiles in a distribution is a separate and different feature request.
    #2186949: Allow a distribution to ship with multiple installation profiles (of which one can be selected in the installer)

.

.

.


Original summary

This came up in #1260716: Improve language onboarding user experience with the code suggestion I'm going to post soon (assuming that one gets committed) and in #1337554: Develop and use separate branding for the installer to make it possible for distros to make changes to the installer before they are selected.

1. It was always an issue for distros to pre-select themselves, effectively skipping the install profile selection page. There are various tricks Open Atrium and other distros used to overcome this and skip the profile selector by redirecting the user in the installer.
2. #1260716: Improve language onboarding user experience moved the language selector before the profile selector so it removed the possibility for profiles to affect the language (either pre-select it or alter the language selection screen). This then becomes an equivalent problem of the profile acting before it is selected.
3. There is an existing distro name .info file property that is effective once the profile is selected. The language step move makes the drupal naming/branding appear a bit longer, so it would be ideal to migrate this setting to a distro level setting that is effective right from the first screen.
4. The installer theme issue at #1337554: Develop and use separate branding for the installer suggests we introduce a distinct brand in the installer that would need to be alterable for serious profiles again before the profile is selected.

All these call for distro level configuration which would act before the profile would be selected for distros which serve one clear purpose, such as Open Atrium, COD and many others. Profiles that have a clear idea of how the language selection screen should look or what theme the installer should use, etc. would presumably ship with one custom install profile that could provide these details. So the need for for conflict resolution, when multiple profiles specify custom themes or ask for themselves to be the selected profile is minimal IMHO. We can just use the values from the first profile we found to specified them in the file system.

Proposed Resolution

Patch #29 adds the ability to have a file in profiles/distribution.php with settings on it that will pre-select options for install_drupal().
With this patch, the installation should work as normal by default, no installation tasks should disappear from the task list when going through it manually.

To test the distribution settings, create a file at profiles/distribution.php with the folliwing:

$settings['theme'] = 'stark';

- You should see that the installation theme changes to stark.

$settings['parameters']['profile'] = 'standard';

- With this file, the installation process will use the 'standard' profile and it won't ask or show the profile settings during an interactive install.

$settings['parameters']['langcode'] = 'en';

- With this file, the language will be set to ('en') (try other languages also) and the language selection task will not be part of the list of tasks for an interactive installation.

Remaining tasks

Related Issues

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Status: Active » Needs work
FileSize
4.49 KB

The attached patch is broken out of #1260716: Improve language onboarding user experience, and only deals with (2) from the four item list above of distro settings needed. It also assumes #1260716: Improve language onboarding user experience is committed, its a diff based on the code from there.

Gábor Hojtsy’s picture

Status: Needs work » Needs review

#1260716: Improve language onboarding user experience landed, so marking this for testing and looking for review.

David_Rothstein’s picture

Category: task » feature
Priority: Major » Normal
Status: Needs review » Needs work

I've filed #1386394: Add back the ability for install profiles (or at least distros) to pre-select a language or modify the language-selection screen as a more narrowly-focused issue for (2) above, so as to focus there on being able to add back the feature that was removed between Drupal 7 and Drupal 8.

This issue is a complete new feature proposal entirely, so I'm recategorizing it accordingly. However, I've marked the other issue as postponed on this for now; since if we get traction here, it might be a better way to accomplish a similar goal.

The above patch is more closely related to #1386394: Add back the ability for install profiles (or at least distros) to pre-select a language or modify the language-selection screen and also had some feedback that it couldn't be committed as is, so this is at least "needs work", although for the purpose of the present issue (distro-level settings rather than profile-level settings) I think we'd probably just have to start from scratch.

Gábor Hojtsy’s picture

Well, my working idea would have been to introduce a distribution container in the .info files, such as

name = My great distro
description = The best distro ever.
version = 8.x-3.4
core = 8.x
dependencies[] = views
dependencies[] = cool_feature
files[] = greatest.profile

distribution[select] = TRUE
; or
distribution[preselect] = TRUE
; or
distribution[preselected] = TRUE
; or
distribution[select] = 'greatest'

; plus

distribution[theme] = 'greatinstaller'
; or
distribution[install_theme] = 'greatinstaller'

; plus

distribution[langcode] = 'pl'
; or
distribution[install_langcode] = 'pl'

I think the main part of this issue is debating the format to death :) The actual implementation is probably not hard. The langcode preselection you basically already implemented, but the feedback was that we should have a nicer format for specifying it and a more generalized one useful for other related settings. The profile selection is likely similarly easier. The installer theme override might pose some challenges, I have not looked into that, but we can postpone that part to a separate issue once the format and an example or two is in place IMHO to keep moving / improving.

I've pinged @catch to ask for the commit of http://drupal.org/node/1260716#comment-5403754, so we can roll that into this one and then adjust the info file related part to the format desired. We can keep debating the format in the meantime :) I don't think whether we put this in the distro .info file or somewhere else is a question, the .info file for the profile is the natural place we have for it.

Gábor Hojtsy’s picture

FileSize
6.89 KB

@David: now that your followup in http://drupal.org/node/1260716#comment-5403754 was committed, I rolled those pieces in this patch as well. Now that we have a record of work done so far on this, let's debate a bit on the format and then apply that to the code.

Gábor Hojtsy’s picture

Status: Needs work » Needs review

Status change for testbot and general code review :)

skwashd’s picture

I like this idea a lot. I think it has the potential to be very useful, not only for distros, but for modules too.

I'd prefer to see this implemented via hooks rather than in the info file, so the defaults could be dynamic. If there was a generic hook_defaults() (or similar) an installation profile or module could define a set of default settings/variables. In some ways it would be similar to strongarm+features just without the revert option.

There would probably need to be another hook defined for setting default selections but allowing the user to change them. hook_form_alter() doesn't seem like the right solution for that, especially when using "drush site-install".

Gábor Hojtsy’s picture

@skwashd: well, we used to have hook telling the installer what does an install profile want. If you look at the Drupal 6 profile API, it is a .profile file that tells you the name of the profile, the modules, etc. with hooks. Not sure we really want to move back there for dynamism's sake. As long as the .info files can specify values that makes callbacks or hooks run, that might be more in the style of Drupal 7. I agree that is kind of duplication where you need to specify in the .info file that you want to do something dynamic and then do it, instead of just implementing a hook. This clearly needs more responses though.

catch’s picture

Could we look at making the installer itself a class, then allow install profiles to extend it? For things like overriding the order of steps in the UI it seems like that's the closest model we have in core. We may still need something in profile.info to specify the override especially for distributions but hooks that only get run once, and aren't implemented by modules even, can't really be called hooks, so 'swappable class' seems the closest model we have in core here.

sun’s picture

The changes of this patch are targeting installation profile "settings"/pre-configuration, but actually want distribution pre-configuration.

A distribution, in its minimal sense, would be Drupal core. But a distribution can contain multiple installation profiles.

As soon as any of the contained installation profiles contains "a" setting to be pre-configured, it applies to all profiles.

I don't think we want to do that - as it implies a lot of possibly unintentional magic, which is a) untestable, and b) insane to debug.

Distribution-level customizations might have to wait for CMI, so (actual) (locked down) distributions, which can be downloaded from d.o, may contain a /config directory in their repo, which automatically applied/moved into the right location, so that those hard/locked distribution-level decisions are applied, before... or actually, when there is actually no Drupal yet.

Gábor Hojtsy’s picture

@sun: ok, are you advocating its fine to even keep the features for language preselection removed altogether if this more advanced support for distros will not land in time for D8? (in other words #1386394: Add back the ability for install profiles (or at least distros) to pre-select a language or modify the language-selection screen would be a wont fix?).

sun’s picture

Aren't those distributions also additionally skipping the profile selection step?

In any case, I'd rather investigate a direction of adjusting drupal.org to

  1. allow such distributions to contain a distribution.info or distribution.php file
  2. copy distribution.php into /profiles/distribution.php in the packaging process (à la sites/sites.php)
  3. expect a $settings array to be defined in there
  4. make core/install.php automatically read /profiles/distribution.php if it exists
  5. pass $settings into install_drupal().

Effectively, install profiles that are (locked down) distributions are allowed to hard-code pre-installation settings, which partially behave like the non-interactive installer. However, this is only in effect and only works when you're dealing with a truly packaged distribution (or when copying that file manually). If you just happen to have an installation profile checked out or downloaded in /profiles, your installer works as usual.

Gábor Hojtsy’s picture

I agree it would be good to come up with a better was for distros on top of (one layer over) install profiles, but my itch here is to either ensure that the lost feature of language pre-selection is not lost in D8 or it is ok to loose. :) Apart from that, I'm just trying to help the larger distro discussions to get to a result with that question :) If improved distro support is a result, that is just great.

BJ___’s picture

This is great stuff. I also would like to see the user experience improved when installing Drupal.
I created a separate issue to suggest the idea of allowing install profiles to select an install theme.
A patch is included that solves the problem and could be used in combination with the other stuff here.

http://drupal.org/node/1442386

Gábor Hojtsy’s picture

Anybody still interested in this or table it for Drupal 9?

rupertj’s picture

I've had a go at this, based on sun's ideas in #12.

To try this out, apply the patch, then copy /core/profiles/example.distribution.php to /profiles/distribution.php

On top of the defaults to the existing $install_state array, I've also added the ability to override the installer's theme from that file, and hidden the steps for language and profile from the list of steps when defaults are provided for them. It looked a bit odd to see the steps marked as complete in the installer, when you never saw them.

Gábor Hojtsy’s picture

Looks like a good concept to me. @David_Rothstein / @sun / @catch?

Jose Reyero’s picture

I like it, great idea, it is simple enough and provides great flexibility.

sun’s picture

Status: Needs review » Needs work

Great work! :)

+++ b/core/includes/install.core.inc
@@ -574,16 +579,22 @@ function install_tasks($install_state) {
+  // Determine whether we are hiding some options that can be pre-selected.
...
+  $profile_preselected = isset($install_state['parameters']['profile']) && !empty($install_state['parameters']['profile']);
+  $language_preselected = isset($install_state['parameters']['langcode']) && !empty($install_state['parameters']['langcode']);

Hm. These conditions will actually equate to TRUE for regular installations, too, since the language and profile selection steps are actually populating those parameters. In turn, those steps would suddenly disappear.

The installation task/step controller does not provide a facility for such conditional tasks yet, since the concept of some predefined settings did not exist in the past. Up until now, any kind of predefined settings actually implied that the installer is executed non-interactively.

This patch introduces a new notion of predefined parameters, which may exist in parallel to the installer's runtime parameters.

Figuring out how to implement this "properly" could probably take ages, so for now, I'd recommend to simply check for potentially existing values in global $settings, and if that exists, mark the appropriate/corresponding steps conditionally as 'display' = FALSE.

+++ b/core/install.php
@@ -31,6 +31,16 @@ if (version_compare(PHP_VERSION, '5.3.3') < 0) {
+// Look for a distribution file.
+if (is_file(DRUPAL_ROOT . '/profiles/distribution.php')) {
+  include(DRUPAL_ROOT . '/profiles/distribution.php');
+}
+
+// Make sure a $settings array exists. There may not have been one in the file.
+if (!isset($settings)) {
+  $settings = array();
+}

1) Let's use is_readable() here.

2) include is a statement, not a function. We use include DRUPAL_ROOT ...; in Drupal.

3) Let's always initialize an empty $settings = array(); before including the distribution file.

4) Lastly, let's change the condition to !empty($settings) and automatically set 'interactive' = TRUE in that case, so this does not have to be specified in the distribution file.

+++ b/core/profiles/example.distribution.php
@@ -0,0 +1,26 @@
+$settings = array(
...
+  'parameters' => array(
...
+  'theme' => 'stark',

With aforementioned adjustments to install.php, we can change the declaration syntax of $settings here and follow the style of settings.php.

This also allows to document specific settings properly, and also to comment out rarely used examples; i.e., essentially:

/**
 * Theme to use in the installer.
 *
 * The theme must exist in the distribution.
 * By default, the installer uses the Seven theme.
 */
# $settings['theme'] = 'stark';

With that, I'd actually recommend to split out all the settings, including the subkeys in parameters.

Yes, this will duplicate some documentation of parameters that exist in install_state_defaults() already, but apparently, the amount of possible predefined settings in the distribution file is limited. (for example, there's no need to document how a distribution can predefine database settings)

Lastly:

- All comments should wrap at 80 chars.

- There should be a newline at the end of files.

:)

rupertj’s picture

Assigned: Unassigned » rupertj

Thanks sun. I'll have a go at those changes later on.

Gábor Hojtsy’s picture

@rupertj: have you been able to take a look to make those changes? :)

Gábor Hojtsy’s picture

It is funny isn't it that profile preselection landed in #1727430: Add 'exclusive' flag to install profiles to auto-select them during installation. as part of the profile info file. Ha! Haha! What about the rest? :)

sun’s picture

I wasn't aware of that issue. We additionally want to revert the changes from #1727430-49: Add 'exclusive' flag to install profiles to auto-select them during installation. here. :-/

YesCT’s picture

YesCT’s picture

Issue tags: +Needs reroll

here are instructions http://drupal.org/patch/reroll

[edit:]
do a straight reroll first. upload that new patch.

then in a separate comment... (if you are fixing anything)
upload a new patch with the changes.
upload a patch and an interdiff then. (this is the way for the interdiff to make sense)
For instructions to make an interdiff see http://drupal.org/node/1488712 or http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...

lucascaro’s picture

Assigned: rupertj » lucascaro

rerolling!

lucascaro’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.8 KB

rerolled.

[edit:]
Note that DRUPAL_ROOT was removed from install.php at some point after the original patch. this reroll is still using DRUPAL_ROOT but that needs to be fixed to use _DIR_.

lucascaro’s picture

Here's a new patch with the changes @sun suggested in #19, note that:

* if $settings is empty, install_drupal automatically sets interactive to TRUE.
* Changed DRUPAL_ROOT to dirname(__DIR_ _) since that constant is not defined in the installer anymore.
* Added all default settings settings that looked appropriate to example.distribution.php. Copied from install_state_defaults() There might be some extra [or missing] settings in there.

lucascaro’s picture

I forgot to break the comments into lines < 80 so here's a reroll of #28 with those changes:

lucascaro’s picture

added testing instructions to issue summary.

lucascaro’s picture

Issue summary: View changes

Added testing directions.

YesCT’s picture

Issue summary: View changes

added related issues section

YesCT’s picture

Issue summary: View changes

added some related from the comments that I missed previously. -y

YesCT’s picture

Issue tags: +Needs tests

updated the issue summary for:

AFAIK, we can't create automated tests for the install process, so we need to test all this manually.

Update: #1961938: Test the interactive installer got in, and an example is in #2022549: Regression: No interface translation in the first installer steps
needs tests for that.

the issue summary maybe could use a read through to see if there are other things to update there.

YesCT’s picture

Issue summary: View changes

Updated issue summary regarding testability

sun’s picture

Assigned: lucascaro » sun
Status: Needs review » Needs work

Reviving and currently working on this. Spoiler alert: Some awesomeness ahead.

sun’s picture

Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: +distributions, +drupal.org distribution blockers
FileSize
114.92 KB
13.3 KB

Proudly introducing...

Distribution installation profiles

Usage

Beautiful simplicity in $profile.info.yml:

distribution:
  name: Portfolio
  install:
    theme: bartik

Live example

via Portfolio:

$ cd profiles
$ git clone --branch 8.x-1.x http://git.drupal.org/project/portfolio.git

Notes

This patch adds some additional file scans and info file parsing operations in the installer environment, which presents a massive slowdown in performance. While working on this patch, I dived very deep into the responsible code and discovered some major performance optimization possibilities that will most likely speed up the installer performance and core test suite in significant ways. I'll file a separate issue for that.

→ In case you test this patch, please ignore the performance slowdown in the installer. That will be fixed separately and to an extent that will make the performance difference caused by the additional parsing in this patch negligible.

Status: Needs review » Needs work

The last submitted patch, 33: distro.33.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
15.71 KB
3.68 KB

Fixed install profile selection step is not a form → 'profile' is not taken over for regular profiles.

sun’s picture

Briefly discussed the proposed change with @webchick in IRC. She raised two concerns:

The 'exclusive' flag is replaced with the 'distribution' property, and the behavior when multiple exclusive/distro-profiles are discovered is changed.

Before patch, if the installer discovered multiple 'exclusive' profiles, then no distro profile was selected.
After patch, if the installer discovers multiple 'distribution' profiles, then the first discovered wins.

  1. On the new/renamed property name + behavior:

    The 'distribution' property/info is optional and not required. If not specified, then an installation profile behaves as in HEAD: Regular Drupal installer, regular profile selection, etc.

    Given that we already have a 'distribution_name' .info file property in HEAD, and given that the whole point of this change proposal here is to enable "distribution profiles", the following diff simply appeared most logical to me:

    -distribution_name: Portfolio
    -exclusive: true
    +distribution:
    +  name: Portfolio
    

    → If you want to install a profile that defines a distribution.name then, by design, you are no longer installing "Drupal".

    Logical reverse condition: If you no longer install Drupal, then any installation profile, whose distribution name is not "Drupal", is hence (1) a distribution and (2) to be treated exclusively.

    That appears to be in line with @Gábor Hojtsy's original conclusion in the issue summary:

    So the need for for conflict resolution, when multiple profiles specify custom themes or ask for themselves to be the selected profile is minimal IMHO. We can just use the values from the first profile we found to specified them in the file system.

    Indeed, supporting the case of multiple distro profiles also sounds like a very rare edge-case to me. Most likely, only distro developers will face that situation. However, they can still use the ?profile= HTTP query parameter to override the automated distro profile selection.

    That said, I'm not married to this detail and reverting to the behavior in HEAD would be trivial. So if that turns out to be the only blocker, then we can retain the current "do nothing on multiple" behavior in HEAD.

  2. There might be distributions that ship with multiple profiles?

    That's an interesting question and yeah, I already questioned this myself while working this patch. My thoughts are three-fold:

    1. First of all, multiple profiles per distro is not to be mistaken with installation profile inheritance à la base themes.

      The vast majority of use-cases I've heard about are asking for "Distributions!" while actually asking for install profile inheritance in reality. Inheritance is not to be solved here and does not conflict with this issue; we have a separate issue for that:

      #1356276: Allow profiles to define a base/parent profile

    2. While I can personally understand the desire, I've the impression that this "requirement" is not remotely an actual problem of distro developers today, but instead, a distant pipe-dream of perfection.

      In light of status quo ("You have to fork Drupal core, good luck!") it feels weird to me to try to cater for a hypothetical "distro of distros" meta-level use-case, given that the simple singular distro profile use-case is not remotely supported.

      Since my goal is to make Portfolio a real thing for D8, my primary interest here is to get the following MVP stories/expectations in place:

      1. As a distro profile author, I'm easily able to inform the installer that only my distro profile is to be installed, so that downloading the Portfolio distro from drupal.org makes any sense in the first place.

      2. As a distro profile author, I can control the entire installer experience, including the theme, starting from the very first installer screen, so that I can tailor my product for the target audience.

      3. As a distro profile developer, I do not have to resemble or depend on any drupal.org-specific packaging mechanics (that may or may not happen in ~2018), so that I can simply git clone my project repo + rock on.

      In terms of these simple but yet powerful stories, this patch here gets us from 0/3 to 3/3 → zero to hero → 100%.

    3. That said, I do not actually see a reason for why we couldn't additionally support the meta-level "distro with multiple profiles" use-case later on.

      That use-case/edge-case would essentially mean to resurrect earlier approaches here and introduce a distribution.info.yml outside of any installation profile directory.

      It would be stupidly simple to adjust the installer to (1) check whether that file exists, and if it does (2) skip the distro-profile pre-selection logic entirely, and instead, (3) consult that info file instead.

      But yeah, IMHO that's putting the cart before the horse, at this point in time. The approach has lots of infrastructure/packaging-script dependencies. All of those dependencies are specific to drupal.org, and whether that is a good idea is yet another interesting question and debate.

    4. Mandatory off by one error:

      The goal of a meta-level distro shipping with multiple profiles is to (1) take over the installer and (2) still allow to choose a profile, but (3) only out of profiles that belong to the distro.

      Case in point: (2) + (3) are trivial to implement through a simple taxonomy-alike tag mechanism:

      Two or more profiles that have the same tag belong together. KISS:

      /core/profiles/minimal:       drupal
      /core/profiles/standard:      drupal
      /core/profiles/zero:          drupal
      /profiles/portfolio:          portfolio
      /profiles/commerce:           commerce
      /profiles/commerce_kickstart: commerce
      

      The value of this tag is actually provided by the "project" property that is automatically added by drupal.org packaging scripts already. And you can happily specify it manually in your git repo, too.

      That still leaves us with the question of (1), figuring out which of these distro profiles ought to take over the installer. — A very pragmatic solution to that would be to only specify the 'distribution' property/info in the profile of your project/package that you want to use during the installer.

      In other words, e.g., "commerce" would specify it + take over the installer. The user can choose between the commerce and commerce_kickstart profiles. commerce_kickstart does not take over the installer, because it does not specify the 'distribution' property and thus is only a "secondary" profile option in the package.

      The more I think about this use-case, the more I realize that the notion of packages ("projects" in d.o lingo) should entirely resolve this use-case already. All we need to do (but not in this issue) is to implement this simple algorithm:

      // If we have a profile that defines 'distribution' to take over the installer...
      if ($preselected_distro_profile) {
        $package = $preselected_distro_profile->info['project'];
        $profile_choices = array_filter($profiles, function ($profile) use ($package) {
          // Remove all profiles that do not belong to the same package/project.
          return $profile->info['project'] == $package;
        });
        // If $package (distro) only ships with one: no selection (covered by HEAD).
      }
      else {
        $profile_choices = $profiles; // Any.
      }
      

      Because I already fear someone is going to ask "What about multiple profiles originating from different d.o projects?"... my answer is:

      Edge-case of an edge-case of an edge-case → not supported. What's the point of maintaining a single distro in separate repos/projects to begin with? ;)

      In any case, as mentioned above, the meta-level use-case should be tackled in a separate issue.

RTBC? :-)

sun’s picture

Created a separate issue/feature request for #36.2, so we can focus on the MVP here:
#2186949: Allow a distribution to ship with multiple installation profiles (of which one can be selected in the installer)

The current patch here contains a change to SystemListingInfo to fix an infinite recursion, which is obsolete as soon as the patch of the following issue lands:
#2185559: Extension System, Part I: SystemListing clean-up + performance; remove SystemListingInfo + drupal_system_listing()

That issue also fixes the performance of the installer, which is a prerequisite for this patch here.

Any feedback/reviews, anyone? :)

sun’s picture

Updated issue summary for the current proposal.

Since my proposal slightly diverges from the original, I've retained the full original issue summary (for now). — @Gábor Hojtsy et al, let me know whether it is OK to remove the old one (quite lengthy).

Gábor Hojtsy’s picture

I think the proposal is beautiful and solves the issues raised earlier without needing extra drupal.org packaging support, which is huge :) The only missing piece I see is an unpublished change notice (see https://groups.drupal.org/node/402688). Once we have that, this should be ready for commit AFAIS.

cosmicdreams’s picture

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Updated the title which was very misleading. The content itself looks good :)

sun’s picture

As noted in #33, this patch has a negative impact on the installer performance, which is caused by deficiencies in the extension discovery in current HEAD.

However, I worked incredibly hard on that topic in the past days and there's now a ready patch over in #2188661: Extension System, Part II: ExtensionDiscovery, which makes especially the interactive installer respond like a breeze. Reviews are very welcome over there.

This patch here can be happily committed before the extension discovery + info parser fix, since the performance will be fixed either way.

xjm’s picture

Thanks for the change record draft! :)

sun’s picture

FileSize
15.61 KB
2.16 KB

Merged 8.x.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 44: distro.44.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
17.03 KB
4.16 KB
  1. Use SystemListing instead of drupal_system_listing() to prevent infinite recursion.
  2. Added distribution installation profile test coverage. :-)

Status: Needs review » Needs work

The last submitted patch, 46: distro.46.patch, failed testing.

sun’s picture

The test failures are weird, I don't know why there are popping up here. They will be gone as soon as #2176621: Remove global $databases has landed, in which case a re-test of this patch should pass again.

sun’s picture

Status: Needs work » Needs review
FileSize
17.31 KB
1.18 KB

Updated for new InstallerTestBase.

This should come back green again.

Status: Needs review » Needs work

The last submitted patch, 49: distro.49.patch, failed testing.

The last submitted patch, 49: distro.49.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
17.51 KB
1.75 KB

Oh, sorry. The test failures were actually caused by silly me.

→ Fixed faulty module list in case a 'profile' parameter is passed.

Status: Needs review » Needs work

The last submitted patch, 52: distro.52.patch, failed testing.

The last submitted patch, 52: distro.52.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
17.53 KB
564 bytes

Fixed larger module list must not be replaced.

That should hopefully be it.

sun’s picture

Finally green again :-) (what a painful ride — sorry for the noise!)

The only difference between the patch that was RTBC and the latest patch is the additional test coverage.

The test coverage is extremely unusual. The only way to fully test that a distribution profile works as expected and is able to take over the installer in the very first installer screen already - is to actually write out a new installation profile extension.

Because all we need is an .info.yml file in the right spot, that is actually easily doable and not as nasty as it sounds.

Yet another benefit of #2171683: Remove all Simpletest overrides and rely on native multi-site functionality instead :-)

To clarify, our usual approach of test modules does not work, because there is no list of enabled modules in the early installer. Thus, we are not able to implant an enabled module into the module list. — Likewise, a testing profile that is marked as hidden: true does not work either, because this distribution profile would actually be discovered for all tests that are executed, which in turn would cause all tests to use it. — Any attempt like these would defeat the entire purpose of the test coverage in the first place, because we would no longer test the actual behavior of the installer.

The approach taken certainly should not be used for anything else. This is an edge-case.

But I'm glad it works so smoothly. :)

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Yay, thanks! Let's get this in then. Still looks good to me.

sun’s picture

FileSize
17.56 KB

Merged 8.x — #2188661: Extension System, Part II: ExtensionDiscovery has landed :-)

No changes, only adapted the SystemListing in this patch to the new ExtensionDiscovery.

effulgentsia’s picture

This is categorized as a feature request. That means it likely won't land before 8.0. But it's also prioritized as major. Does that mean there's a problem if it doesn't land before 8.0?

sun’s picture

Does that mean there's a problem if it doesn't land before 8.0?

Yes, problem. This is a so called "killer feature enabler". It's not just a feature, this change presents an entire category of new features and solutions. The category we've been shooting for since ~6 years: Proper Drupal distributions.

I'm working on this and many related issues for exactly one reason: Make the Portfolio distro finally a reality for D8 and deliver an actual product on drupal.org. Portfolio represents the I in the three meta categories I, Us (company/party), We (e.g. local Drupal community).

The original issue summary by @Gábor outlines problems that did not exist in D7 and which were introduced by technical and architectural installer changes in D8. Those present regressions and make this issue borderline a major bug, but I did not feel bold enough to turn a feature into a bug.

If we let this sit until 8.0, then (1) we won't advance and (2) distributions won't happen for D8. Again.

This is of epic significance. This issue is the one and only reason for why I'm working on core currently. We've been shooting for an insane amount of perfection in the past, and absolutely nothing happened. We need this MVP now.

I'm ready to do this. I'm ready to rewrite, modernize, and kernelize the installer. I'm ready to ensure the best possible solution to enable distributions in D8. Because Portfolio is going to be a/the driving API consumer. We can have it now. Or we can talk again in ~3 years.

cweagans’s picture

I'm ready to do this. I'm ready to rewrite, modernize, and kernelize the installer.

On one hand, I'm wondering why this wasn't a priority in code thaw, because technically, that was the time for this to happen. On the other hand, I strongly feel that this absolutely needs to happen. The installer is the single biggest source of frustration to me as an install profile developer (for internal use by clients - not public install profiles on d.o).

sun, if you get any traction on this, reach out to me via my contact form (or guess what my gmail address is) and I'll happily assist.

Gábor Hojtsy’s picture

@cweagans: wasn't it a priority? I worked on this over 2 years ago, got others on it, tweeted the hell out of it, etc. There is only so much a few people can do without more attention.

sun’s picture

wondering why [modernizing the installer] wasn't a priority in code thaw

To clarify: It was a priority right from the start. In fact, it was one of the very first OOP issues that got opened for D8.

However, (1) We did not have architectural concepts and the collective knowledge required in order to execute. (2) The current installer code is one of the most horrible legacy code constructs in D8. (3) I'm counting approx. ~2-3 active contributors who truly understand the spaghetti and who are able to salt it with pasta. (4) Modernizing the installer is not bound to code thaw, because it affects all-internal code only; the "public API" (if there is any reasonable definition of that) will not break.

To clarify further, I only mentioned the closely related tasks, because this issue presents an all-or-nothing deal for my goals — if I'm not able to create the Portfolio distro for D8, then I have no incentive at all to work on the installer - and also no incentive to work on e.g. a root /settings.php for that matter. — Either core enables the MVP for real products, or it does not.

But yeah, even the question of whether this patch is eligible to land right now feels nonsensical to me. This patch is fairly small and trivial. And out of the handful consumers that actually care, everyone will be more than happy to minimally adjust their .info.yml files accordingly. Especially because it involves an API change, it cannot happen after 8.0. Perfectionism kills progress. Progress on this topic is very long overdue.

cosmicdreams’s picture

58: distro.58.patch queued for re-testing.

Seeing if it still applies after menu_router removal.

sun’s picture

Title: Add distribution level settings to install profiles » Distribution installation profiles are no longer able to override the early installer screens
Category: Feature request » Bug report

The original issue summary by @Gábor outlines problems that did not exist in D7 and which were introduced by technical and architectural installer changes in D8. Those present regressions and make this issue borderline a major bug, but I did not feel bold enough to turn a feature into a bug.

After sleeping over this once more, I feel bold enough now.

catch’s picture

58: distro.58.patch queued for re-testing.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
    // Add a default distribution name if the profile did not provide one. This
    // matches the default value used in install_profile_info().
    if (!isset($modules[$profile]->info['distribution']['name'])) {
      $modules[$profile]->info['distribution']['name'] = 'Drupal';
    }

This comment this out of date now since install_profile_info() does not add a default in anymore.

sun’s picture

sun’s picture

Status: Postponed » Needs review
FileSize
15.72 KB
4.4 KB

Fixed merge conflicts, improved test.

sun’s picture

FileSize
16.35 KB
4.28 KB
  1. Fixed docs in _system_rebuild_module_data().
  2. Simplified drupal_install_profile_distribution_name().
  3. Simplified _install_select_profile().
  4. Added docs to install_profile_info().

The longer I need to look at the spaghetti code of retrieving installation profile info, the more I'm tempted to fix that mess in this patch... Please prevent me from doing so. :-)

This patch resolves the docs concern raised in #67. Another sensible concern privately raised by @alexpott was addressed in the last patch already.

Back to RTBC/commit?

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Still looks good to me. The docs improvements look good :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Nice work!

Committed e42ac27 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

YesCT’s picture

Kristen Pol’s picture

In response to #74, I'm going to try to figure this out.