Allow install profiles to declare base profiles for Drupal 7

Updated the issue summary and attaching the patch so it can be reviewed by the testbot for D7.

Sorry, wrong project.

@geerlingguy: The patch that you submitted, is this one that I had by accident attached to the D8 discussion? (https://gist.github.com/amcgowanca/6191652 and/or https://drupal.org/node/1356276#comment-7733271)

I think that was the one; I used the last D7 patch I could find.

Perfect, that is the one @geerlingguy. Thanks. I am going to follow and got a few additions/changes to the above patch that I will post once I have done my own testing on my end.

Initial multi-profile inheritance. Additional changes include changing the .info file's property from "base" to "base profile".

Apologies for the idiocy in previous patch around the $searchdir creation (I was thinking something then went sideways).

Contains the fix for the exception thrown regarding undefined variable profile.

This patch makes some additional modifications to core and usages for the inheritable profiles. First off, I think the usage of parent[] is incorrect and instead should be a single profile inheritance vs. one profile dependent upon many. However, with this patch you able to have a inheritance hierarchy allowing single profile to be inherited but the "derived" profile can also be derived once again.

For example, using the provided default installation profiles (minimal and standard), we could do something like this. The standard profile would inherit the minimal one, and a specific site's profile could also inherit the standard (minimal > standard > "site")

Additionally, the other major change within this patch from my previous one that @gerrlinguy posted here from the D8 discussion is that I have changed the .info file's property 'base' to 'base profile' to align with the 'base theme' usage for themes.

I also made one additional core change and re-ordered the install tasks by moving the install_load_profile task before the install_select_locale. Currently just working on testing a new patch that handles the install_select_locale as well for the multiple profiles, however, it is needed so that any one of the parent profiles of the selected profile can have its hook_profile_details invoked if its defined.

Notes
Tests have passed ... Locally ran the unit tests for those that failed in the previous patch and hopefully these small adjustments will assist in that.

Updated the install_select_locale tasks for handling all profiles, not just the selected one.

Fixed typo in previous patch.

#21, patch tested with simple profile and Commons. Looks like it's not including the base profile.

#2, patch is working. Tested with simple profile and Commons, locally and on Aegir setup.

@umtj: Regarding patch #21, how did you test it? Reason I ask is we are actually using this patch specifically for a few new product developments. Would love to collaborate and get some additional information so that I can improve it where needed or fix any bugs.

@amcgowanca i'v been trying with a local MAMP setup and an Aegir. Looks like it's not including JS, CSS and modules from Commons profile, even the extra install steps is missing.

Could you try making a profile that use Commons profile as base profile and let me know how it goes?

@umtj: I will try using commons as a base and report back today.

@amcgowanca, any news?

@umtj: There are actually a few areas that have problems with using this patch with the Acquia Drupal Commons profile that I have identified through testing. My apologies for not getting back to you sooner.

The problems are as follows:

1. There appears to be a conflict when patching Drupal core when Commons requires this patch: http://drupal.org/node/1074108#comment-6463662. If you comment out the patch that fixes the single profile skip check, then the inheritable profiles patch applies cleanly.

2. The Acquia Commons profile overrides the install finished task and implements their own mechanism for setting the currently selected profile. In the patch above, this process actually is altered and therefore with the Commons profile overriding it, it does not take into account the changes caused by the patch.

However, I have made a fork of Acquia's commons profile on my personal GitHub account and made a few changes for this patch to apply. You can take a look at the commits and see the changes that I proposed to work along with this patch specifically. Link: https://github.com/amcgowanca/commons3/tree/feature/inheritable-profiles...

Added link/description pointing to parent issue.

Here is the patch from #21 re-rolled for the latest 7.x branch (ie. soon after 7.26).

The latest version of this patch appears to have switched from using:

parents[] = PARENTPROFILE

... to ...

base profile = PARENTPROFILE

However, I'm actually unable to get it to work. :-/

Here is a re-rolled patch which includes a deep dependency checking (something that Drupal core doesn't provide but is essential) as well a few bug fixes. It has been re-rolled for 7.26.

@dsnopek:

What issues are you having? Can you try the patch from #30?

Aaron

@amcgowanca: I can't get Drupal to see any of the modules or themes in the parent profile! If I say "drush en modulename" it says it's not present and offers to download it. If I go to the "Modules" page, modules which depend on modules in the parent profile will list the dependency as missing.

I haven't tried #30 yet, although, I'm running out of time allotted by my client to look into this. :-/

@dsnopek: Which patch version are you trying? I know that there are a handful of large in production projects using #30.

Yes, "parent profile" also changed to "base profile".

I was using #21. Unfortunately, I started testing before #30 was posted!

With this patch, how does it handle the dependency issues.

For example: Using open publish and a child profile. I don't necessarily want all the modules to be enabled. So in the install task I would disable them, and uninstall them, but this doesn't currently take effect because of the order the dependencies run (or so it seems)

Have others had this problem?

Thanks for the help!

Hi @timodwhit,

The patch actually enforces the dependencies to both exist and be installed. However, one option, if you are so inclined is to actually perhaps look at some of the work that I have been doing at ImageX and its installkit project (http://github.com/imagex/imagex_installkit). One option would be to actually alter the profile installation tasks that remove the modules within the profile's module install operations for batch processing task.

Aaron

Is there a imagex_installkit_child_example or something like that someone can use to test this?

Hi @kreynen,

I can put one together for yourself if that'd help.

Aaron

It would. I'm trying to evaluate the pros and cons of inheriting vs. the nesting approach we've been using in https://drupal.org/project/cm_starterkit_easy > https://drupal.org/project/cm_starterkit_moderate > https://drupal.org/project/cm_starterkit_difficult where the .make of each profile builds on the other.

Just opened https://github.com/imagex/imagex_common_fields/issues/21

I have lots of thoughts based on assumptions about how this works, but I'd like to actually use it first. I checked the drupal-org-core.make of a few distributions. None of these are including this patch...

http://drupalcode.org/project/commerce_kickstart.git/blob/refs/heads/7.x...
http://drupalcode.org/project/openatrium.git/blob/refs/heads/7.x-2.x:/dr...
http://drupalcode.org/project/commons.git/blob/refs/heads/7.x-3.x:/drupa...
http://drupalcode.org/project/cis.git/blob/refs/heads/7.x-1.x:/drupal-or...

Are there any packaged distributions using this?

I'm finding that the patch in #30 does not work. During installation I get a recoverable fatal error:

Recoverable fatal error: Object of class stdClass could not be converted to string in install_select_locale() (line 1251 of /www/build/includes/install.core.inc).

I traced it down to this piece of code:

$locales = array_unique($locales);

As $locales is an array containing objects, array_unique won't work, it expects an array with string values.

[edit]
To get the function to do what you want you need to add the sort flag 'SORT_REGULAR'. I'll see if I can add a patch.

Here's the adjusted patch. I only added the sort flag, otherwise it is the same as #30. (By the way, @amcgowanca, are u on PHP 5.2.9 by any change? In that version, and only that version, the sort flag defaults to SORT_REGULAR)

@kreynen: Open Atrium *is* using a patch to allow profile inheritence, just not this one - it's an older version from here: https://drupal.org/comment/6117722#comment-6117722

I'm one of the co-maintainer of Panopoly, which aims to be a "base distribution". We'd definitely promote using profile inheritence if it was implemented upstream in Drupal! For now, we recommend that child distributions put all our modules in their .make files. So, I'd love to see this move forward. :-) Also, the packager on Drupal.org will need to be updated too in order for anyone to really start using this for distributions on Drupal.org.

@dagomar: Great catch, my apologies for that one - complete oversight on my part. That said, I am definitely *NOT* running PHP 5.2.9.

@kreynen: To my knowledge, no other distribution is currently using *this* patch and the patches that OpenAtrium use I do not believe are a sufficient means at "inheritable profiles". Also, I will be creating a standard flavour of that work from ImageX InstallKit that will be here: http://github.com/amcgowanca/drupal_installkit which I have also created a very simple example profile here: http://github.com/amcgowanca/drupal_installkit-my_profile. You can also see the Travis-CI build here: https://travis-ci.org/amcgowanca/drupal_installkit-my_profile/jobs/23329768 for example, that the webform module is in fact actually installed.

@dsnopek: What are the particular reasons behind your recommendation for placing all modules into child distribution .makes? Yes, I agree that the packager would need to be updated here on D.o (is there a contrib version of the packager in which updates could be made too?)

@dsnopek: What are the particular reasons behind your recommendation for placing all modules into child distribution .makes?

1. This patch isn't committed yet. :-) I also haven't personally been able to get this patch working, although, I was trying an older version.
2. The Drupal.org packager won't pull in parent distributions, so even if you used this patch, your distribution couldn't be released on Drupal.org

Like I said, I'd love for there to be profile inheritence in Drupal core! But both those things need to be handled before we can really start using it.

@dsnopek: Let me know how I can help you get it working on your end; I'd be more than happy to help as well as have someone else test it.

child distributions put all our modules in their .make files

That's basically what we're doing. Not so much as parent/child, but to limit the number of modules a site admin sees when getting started. The imagex_installkit has > 100 modules. While that could be great for developers or experienced site builders who have some idea what all these modules do, a less experienced site build who spins up a distribution on Pantheon or a similar service can be overwhelmed by that many options. Looking at the list of modules at /admin/modules is like drinking from a firehose. Even for developers, that many modules leads to a lot of updates to code you aren't using in all sites.

We encourage new users of our distributions to start with fewer modules, then use https://drupal.org/project/profile_switcher to move from Easy > Moderate. This is only possible because we refrain from doing anything in the profile's .install.

Switching the active profile has issues as does keeping duplicate copies of modules in different profiles. While Drupal handles multiple profiles in profiles/ well enough for .modules, Drush is not a fan of finding copies of same code in multiple profiles even if it isn't active.

We're also using https://drupal.org/project/profile_status_check to empower site admins to test updates to modules before they are updated in the .make and packaged into the distribution. Again, moving modules between profiles/[profile_name]/modules and sites/all/modules has issues like needing to rebuild the registry, but this seems better than "hacking" the distribution.

The Drupal.org packager won't pull in parent distributions, so even if you used this patch, your distribution couldn't be released on Drupal.org

I agree that this is really key. While a distribution using this patch would work for a client specific or otherwise private install profile, that child distribution can't be shared on Drupal.org in a way that would include the base in the packaging. I can't use this for the Easy > Moderate > Difficult versions of the Community Media Starter Kit without this change, but I don't think the code is really the challenge.

Looking at http://drupalcode.org/project/drupalorg_drush.git/blob/refs/heads/7.x-1...., the validation prevents packaging of $info['projects'] and $info['libraries'] in the drupal-org-core.make. The drupal-org-core.make seems like the logical place to define the base profile since it would be downloaded into /profiles vs. built within /profiles/[profile_name] the way the profiles that is the Drupal.org project is.

It really isn't the Drupal specific packaging that needs to change... though it wouldn't hurt to propose specifically allowing profiles. The key is going to be updating the Drush make function to process profiles https://github.com/drush-ops/drush/blob/master/commands/make/make.drush.... similar to how it is processing projects and libraries and then get Drush updated on Drupal.org.

And that is no small task.

Rerolled #2067229-41: Allow install profiles to declare base profiles for Drupal 7 with additional logic to check module info for version before trying to use it. Custom modules aren't always versioned... :)

I tested #47 with Panoply 1.6 and it works great. Here's the contents of my .info file:

name = MyProfile
distribution_name = MyProfile
description = The base profile for MyProfile sites.
core = 7.x
exclusive = 1
base profile = panopoly

FYI, this can't get in until #1356276: Allow profiles to define a base/parent profile is complete... not sure if RTBC is proper status.

Postponed per #55.

I think there are pluses and minuses to the inheritance approach.

The plus is that you are given a lot out of the box as a freebie because you can inherit the parent. But there seems to be a lot of technical debt that is possible using this approach if your project wants to diverge from the parent install.

For example: Using open publish as a base and then have a child that needs a blog. Most cases the blog will not resemble exactly what is expected from Open Publish's profile and may need to have custom fields, etc. At this point, you can re-roll the feature and place it in the child profile with the changes, but that seems to defeat the purpose and then we run into the issue of is the module being inherited from the correct location.

The other issue is that if the site does not need a blog you now have technical debt to maintain code for just the install that runs a single time, and leaves the possibility of a blog being installed at a later date. This may or may not be an issue depending on namind convention, but could create issues.

I did initially support this strategy but upon hitting road blocks, etc. It becomes more and more difficult to get the "perfect" parent profile. Where nothing needs to be overridden.

One of the biggest issues I've had with this profile inheritance approach, and the main reason I've stopped utilizing it myself, is that the parent profile gets enabled after the child profile. This makes it difficult or near impossible to do things like use a different theme than the parent profile enables and sets as default - or choose to disable a module the parent profile enabled. It also makes it difficult to build upon what the parent profile creates - since your child-profile's install hook runs prior to the parent's install hook.

In the end, it seems that this "inheritance" is actually more of a "bundling" - allowing you to bundle another installation profile alongside your own. I think if inheritance is still the goal here this patch will need to be re-factored with regard to what is installed/fired first.

For anyone interested, here's a contrib approach to solving this problem D7:
https://github.com/bryanhirsch/profile_installer

I've gone through a few different overhauls working on the proof-of-concept and renamed it a few times to accurately reflect what it does. Once the dust settles and the name sticks, I'll give it a proper home on drupal.org. (Changing project names on drupal.org is not trivial. And I don't want to pollute namespaces.)

@bryanhirsch, that looks a very promising project.
I will take a look and try to collaborate with it.

Thanks for sharing

This is a reroll of patch provided in #47, have been using this patch and this approach for years with no real problems, this patch just improve the installation detection of missing modules.

This is a fix to the problem introduced in my patch #62, to improve the detection missing module (e.g: .module with inaccessible or corrupt info file), but since modules are not required to have versions (e.g: modules installed using git, custom modules), I am checking if the info file of a module have been parsed correctly by checking if that the info array is not empty.

@chrisolof I think you can go around the problem of the parent profile getting enabled after the child profile, by using install tasks instead of hook_install(), also you can alter tasks in the parent profile by using hook_install_tasks_alter().

Hi there, to start I should say the following patch does not directly contribute to this issue. However if you have also applied the patch from https://www.drupal.org/node/1253774 the patch in this thread from #64 will conflict.

Attached is a re-rerolled patch that should allow both patches to work together.

As another aside, if anyone else was wondering about drush support for this patch, adding a dependency to an base installation profile and then running drush pm-enable child_profle will not parse the dependencies from the base profile. As such the new dependency will not be enabled.

I have written a drush extension, Profile Enable (https://www.drupal.org/project/profile_enable), that will parse the base profile(s) for new dependencies and enable them. Of course this is only intended to be used in the event a profile dependency is added after the site has been installed. The installation process does handle the dependencies correctly.

FYI, the reason Atrium uses an older, simpler patch and not the one in this thread is that I disagreed with the complexity of where this issue went.

For Atrium "inheriting" Panopoly we didn't need true inheritance. We don't care about inheriting the parent's info file (the child distro should decide which modules it wants enabled in its own info file).

All we cared about is keeping the modules separate between two profiles: profiles/openatrium and profiles/panopoly. We want Drupal to search profiles/panopoly if a module is not found in profiles/openatrium. That's it! Super simple.

In fact, we didn't end up needing this patch at all because drush doesn't support this issue. And even if drush is updated, Open Atrium needs to use the drupal.org packaging system which still uses old Drush 5. Thus, we ultimately needed to include Panopoly directly into Open Atrium.

However, when I'm building a client distro/profile on top of Atrium, we *do* use the simpler patch just to keep the clients modules in profiles/clientdistro and the atrium stuff in profiles/openatrium (and then site specific stuff in /sites/all).

There might be good reason for some of the other stuff in this patch but it got too large and complex for me to really test. A shame the original simple patch to just the module search order didn't get more traction. Sometimes I think we wait for a "perfect solution" instead of making incremental improvements, but let's not start that discussion here.

I just wanted to chime in with the reasons I wasn't using this patch in Open Atrium.

Packaging of distributions on Drupal.org is unlikely get much attention until Composer supports limiting licenses of dependencies to GPLv2 compatible licenses... and then I expect most new features (like being able to package multiple profiles) would come from how we leverage Composer. See the most recent comments on #2551607: [meta] Path forward for Composer support

My point was only that if packaging of a profile with a base profile in a distribution on Drupal.org is someone's goal, that is very unlikely to happen for D7. Might be possible for D8 distributions, but this needs to be part of the Composer discussions or it won't happen.

@mpotter:
Can you give a link to this simpler patch, as it really covers the basic need - separation of code bases.
I know I've sen it somewhere on drupal.org, but finding it now is not the easiest of tasks.

@mpotter: Your last comment was in the parent issue, i decided on using:
https://www.drupal.org/files/1356276-make-D7-21-redux.patch

Please correct me if I'm wrong :)

@ndobromirov I also would like to use the "simple" patch and not the complicated one (even though the complicated one is working for me). But this one looks like it is for version drupal 7.21, is there any patch for Drupal 7.4x? looks like commons.inc changed quite a bit.

another comment, I am not sure if it belongs to this issue or to libraries module, (sorry for the noise).

I think there is the same issue with libraries, they are not found on the child profile. Sometimes the parent profile needs a library on the installation process, but if the library is not present in the child profile, this will brake at installation time and the Drupal installation will be unsuccessful

This happens even with patch #62

There is a bug with the patch in #64 when creating the install_profiles variable after install. For example if you have a profile structure like the following:

profiles
├── minimal
├── profile_one
├── profile_three
├── profile_two
├── standard
└── testing

Where profile_three has a base profile of profile_two, profile_two has a base profile of profile_one and profile_one has a base profile of minimal. After install the install_profiles variable contains duplicates:

Array
(
    [0] => profile_three
    [1] => profile_two
    [2] => profile_one
    [3] => minimal
    [4] => profile_two
    [5] => profile_one
    [6] => minimal
    [7] => profile_one
    [8] => minimal
    [9] => minimal
)

Attached patch resolves this problem and produces, in this case, an install_profiles variable of:

Array
(
    [0] => profile_three
    [1] => profile_two
    [2] => profile_one
    [3] => minimal
)

Just a question... I see it's an ordered list but should we have a second one or just change the format like so:

$extends = array(
 'profile_three' => 'profile_two',
 'profile_two' => 'profile_one',
 'profile_one' => 'minimal',
 'minimal' => NULL,
);

As minimal does not extend anything - it is not used as a key or ('minimal' => NULL).
The storage in this manner will prevent any duplication by design.

Or we are going to trust the ordering of the existing solution to maintain the inheritance chain.

@ndobromirov - Thats an interesting approach and certainly would prevent duplication. However the current solution is adequate in building the inheritance chain; the issue was concerned with the differing state of the $profiles variable when used in the drupal_get_profiles function. When the install finishes $profiles contains an array of the entire inheritance chain, but in other cases it can just contain the top profile.

This results in the inheritance chain being parsed and built again.

As a counter-example:

$extends = array(
 'profile_A' => 'profile_B',
 'profile_B' => 'profile_A',
 'minimal' => NULL,
);

it is always possible to screw things, so I don't see a big win in having $extends array

Convinced. Thanks @danielmrichards and @valthebald.

Updated patch for Drupal 7.64.

#84 supplied PHP 7.2 compliance but switched from using the 'base profile' key to just 'base', and moved the logic for building the profiles array into install.inc rather than common.inc. Attached is a revision to #84 that restores those two items.

(In my case, moving the logic to install.inc created a problem since my codebase adopted profile inheritance for many sites well after their initial installation, so the 'install_profiles' variable was never set)

Setting back to Needs Review & Triggering bots.

It looks like https://www.drupal.org/node/2759197 broke #85.