Allow profiles to define a base/parent profile

Changing to a more accurate title

Looks great ! I will give it a try soon.

Anyone know how this compares to profiler's base profile option?
http://drupalcode.org/project/profiler.git/blob/refs/heads/7.x-2.x:/prof...

Relevant:#906106: Run hook_install() for base profile

@patcon
Profiler's base profile allow to inherit some base profile's things like installation functions but it do not add all the base profile directories to the stack when looking for modules' or themes' locations.

I can't say with 100% accuracy until I test, but I was under the impression that Profiler was designed for that same reason. Inherited *.install files were secondary, I believe. Are you certain on that?
http://drupalcode.org/project/profiler.git/blob/refs/heads/7.x-2.x:/READ...

Hm. Ok, I think this clarified for me (although I guess I can still spin up a site myself):
http://drupal.org/node/1275160

You're right. Sorry for the confusion and derailing :)

Same patch as #2, but rerolled against pressflow 7.12 using git (hopefully drush_make supports this now, or i'll be back :) )

Cross referencing: #1530756-8: [meta] Use a proper kernel for the installer

Marked #555212: Install profile inheritance as a duplicate.

@cweagan I'm ashamed to admit how many clicks it took me to realize I was in an infinite loop of duplicate issue links...

EDIT: Guessing this was the one :)
#555212: Install profile inheritance

Heh, whoops. Link fixed.

GOOOO TEAM! *anchorman leap*

I rerolled http://drupal.org/files/1356276-make-D7.patch against 7.14, had to make some minor adjustments. Will test and update this comment.

Been using this on several large production sites and works great!

This needs to go into Drupal 8 first, but it looks like the kind of change that might be acceptable for Drupal 7 backport, if it's the direction we decide to go in.

Some issues on a quick readthrough:

+ * recusive function to find listing paths from profiles and their parents

This has typos ("recusive" => "recursive") and grammar mistakes (sentence should start with a capital letter and end with a period). Also similar issues elsewhere in the documentation.

+  $info  = drupal_parse_info_file("profiles/$profile/$profile.info");

It seems pretty bad that we're going to manually parse the .info file every time this function runs, rather than taking advantage of information cached elsewhere. But it's a low-level function, so I'm not sure how best to solve that.

+    $searchdir = drupal_system_listing_profile($profile, $directory, $searchdir);
   }
-
+  

Extra whitespace.

Here is a re-roll of the patch with the spelling, grammar, and whitespace issues fixed. The drupal_parse_info_file() isnt' really a problem because that function already has it's own cache. But I'd welcome other suggestions or patch improvements.

Bah, missed one more space at the end of a line. Here is the cleaner patch.

Grr. Marking this as a D8 issue is causing the D7 patch to fail testing. Is there a way to mark a patch as "for D7" for the tester so this doesn't happen? Is there any special patch naming trick for that?

If you are using D7, ignore the failed test result of #21. It applies just fine to 7.14.

I had seen a naming convention before but the documentation does no longer mention it.

You can just add a "-do-not-test" suffix to say to the bot to skip your patch.

Rerolled patch for D8.

This looks incorrect to me.

We do not want to re-evaluate the base profile at run-time.

Instead, the only change to the foreach in drupal_system_listing() would be the replacement of $profile with something along the lines of this:

$profile = drupal_get_profile();
$profiles = array();
do ($profile) {
  $profiles[] = $profile;
  $profile_info = system_get_info('profile', $profile);
  $profile = isset($profile_info['base'] ? $profile_info['base'] : FALSE);
} while ($profile);
// Search from top/parent profile first.
$profiles = array_reverse($profiles);

Ideally, of course, we'd store the already prepared base profile information with each profile's info already, so we can avoid the repetitive system_get_info() calls for every profile in the inheritance.

I'm also missing some other essential changes to the Drupal installer and actual profile installation in this patch. Merely taking the base profile information into account for path lookups does not fulfill the promise of making install profiles inheritable.

@sun, it enables this kind of setup: http://www.agileapproach.com/blog-entry/inheriting-your-drupal-profile-e...

IMO, what is here so far could go into Drupal 7. The installer changes and profile installation seem more like D8 things.

Yes, that post is how I learned about this issue. New features go into HEAD first though. I will admit that the existing patch provides some degree of profile inheritance, but it totally was not what I expected. It sorta tries to tack the new feature onto a system that's otherwise unaware of the feature, which obviously won't fly.

If we want this to land, then we need to make the system properly aware of possible install profile inheritance. I support this change, since it was mentioned in a couple of other issues and personal discussions already. I do not see a technical reason for why it couldn't be supported.

That said, while this feature addition in core seems feasible and doable, there will still have to be support for it coded into drupal.org's distro/profile packaging scripts, which inherently/actually means to support it in Drush. But that's a separate follow-up issue.

The previous patch wasn't exactly what I was expecting, either. Also, pushing fake inheritance in the parts of Drupal's non-OOP code is a foreign pattern that exists nowhere else in Drupal – and feels a bit hacky imo. Certainly if install profiles were re-written as classes then that would totally make sense, as well as be trivial to implement.

Instead, this patch contains a different approach that follows the pattern of Drupal's theme system, in which there is a concept of defining a single "base". So everywhere in the system where an install profile is requested to participate – instead of invoking the single current profile, it invokes the ancestral chain of profiles starting from the top-parent down to the last sub-profile.

This allows developers to truly extend Drupal distributions with the same ease as creating a sub-theme.

This patch is for D7 – Sorry, I know new features need to go in D8 first, but I just wanted to put this out there. I'll try getting in contact with the right developers to see what direction install profiles are going in D8 (#1530756: [meta] Use a proper kernel for the installer suggests it will be a complete rewrite, so I'm not sure how the back port to D7 will look like.)

Thanks! That looks a lot more like it :)

Btw, some of the changes clash/conflict with #562042: Search for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profiles, which I'd like to see getting in first. But since that is RTBC already and we don't have a patch for D8 here yet, that doesn't seem to be an issue in the first place. ;)

The bot should take a ride with the latest patch.

Slight addition to #29 to check for drupal_get_profiles(), which is defined in install.inc, and so not normally included.

Sorry, we really need a patch for D8 here. The patches for D7 are very confusing, so I'd appreciate if we could stop posting them.

#25 is obsolete by now, drupal_system_listing() does not have to be touched at all, since D8 scans for profiles everywhere already.

This also needs to be taken into account when forward-porting #32 to D8.

My bad. To be honest, I kind of knew that, but since local patches don't work in drush make yet, I posted here. That's an abuse of this issue queue though, so I will go stand in the corner for 20 minutes...

Just a small tweak to #17: it looks like the $bases array generated in drupal_system_listing_profile was never being used. Trying to use it as is caused the base module to not be used at all, and displayed the following error:
Warning: Invalid argument supplied for foreach() in drupal_system_listing_profile() (line 5220 of includes/common.inc).

Same again for #21.

Finally, a Drupal 8 patch :)

I moved drupal_get_profiles() to common.inc since it makes more sense to keep it there (as @ezeedub pointed out in #32 that install.inc isn't always included). Also incorporated all the D8 changes (that I know of) including the update mentioned in #30 and #33.

Also, for those using the D7 patch, here is an equivalent update.

Here is a reroll of #38 to include a fix for base profile dependencies.

FYI an interdiff for the previous 2 patches:

#36: 1356276-make-D7-21-redux.patch queued for re-testing.

I took the patch from #39 and pulled all the logic out of runtime. It was a pretty big performance hit finding sub-profiles on every request. This patch is for D7. Is there another issue open for 7.x core to post patches to, so we don't pollute this space with 7.x issues?

Without looking through the code, is there a reason why there is a variable called 'install_profiles' that everything depends on? The old 'install_profile' variable does still exist.

The main reason I'm asking, is because we couldn't update an existing site with this patch, since that variable is created during the installation. We solved it by simply adding the variable manually, but it would be nice if it could be solved automatically.

Aside from this, the patch at #42 has worked on several sites we've been testing.

I haven't tried the patch yet, but I have a question, will the child profile inherit update hooks in the parent profile? Thanks!

I have tested and can confirm this works beautifully. I had 2 issues with existing sites, the variable "install_profiles' was missing, as previously mentioned. I just installed a blanc site to get the correct variable and imported that entry manually. The other issue I had was that the parent wasn't enabled in the system table, after I enabled it I was good to go.

I have recreated the patch because of whitespace issues. Note, it's for D7.

@dagomar Yes, parent profile's update hooks are picked up as well.

+++ b/includes/common.incundefined
@@ -232,6 +232,22 @@ function drupal_get_profile() {
+ * Returns a list of related install profiles in decending order of their

s/decending/descending

Could we maybe file a helper issue for D7 so those patches don't distract from getting this into D8 (if it isn't already?)? I'm a bit late to the game here, and haven't read through the entire issue+comments.

I don't see why this would be added to Drupal 7 at all. Lets keep this in D8.

@frob - the issue has the 'needs backport to D7' tag... which is used for issues that are possibly going to be backported/included in D7.

I am removing that tag. I might be out of line and I wont be offended if it gets added right back in. But to me this isn't fixing a bug or glaring UX, DX problem that needs a backport. It is a nice to have in Drupal 7, so to me it doesn't need a backport.

I would like inheritable installation profiles in Drupal 7.

Here is my attempt to explain why...

Background

Install profiles are the core of Drupal Distributions — a really quick way to set up a complex sites.

Creating a Drupal Distribution is simple: you create an installation profile; add a set of Drush makefiles to download specific versions of core and contributed modules, themes and libraries; and add your own features, custom modules and themes. If you have any patches to core or contrib, stuff to add to .htaccess or settings.php, or you write any behavioural or functional tests of the system as a whole, you can put them in sub-folders. Any changes to the database that don't belong to a particular module or theme can go in the install profile's .install file. You can build the whole site's filesystem with a single Drush make command. And, because this is an installation profile, you can easily build a fresh database for use behavioural and functional tests or to deploy on a new machine.

The result is a folder which only contains the code you've customized or written from scratch, but not cluttered with environment-specific stuff or content. This folder is ideal for checking in to version-control. And, any older version of the site can be upgraded to a new version by running update.php and reverting all features.

When do I use Distributions?

Currently, we build a Drupal distribution for any client site we work on. It lets any number of developers work on it, developers seldom need to touch staging or live directly (Jenkins does almost everything), and we don't have to constantly share copies of the database.

Why inheritable install profiles would make this better.

Inheritable install profiles would allow us to build a base install profile common to all sites we work on, containing modules, themes, etc. that we use on every project. Also, it would make it easier for us to build sites on other popular distributions. And, if a client wanted a new site similar to their old one, we could inherit from the old one.

Why I want this patch backported to D7.

We're using distributions as the base for our sites right now on D7. Since D7 will be maintained until D9 comes out, at the very least, we will be using or maintaining this methodology for a while yet.

mparker17, you seem pretty passionate about this; why aren't you helping with this issue more?

Sorry :S Somebody already wrote a patch for D7 before I knew this issue existed!

I did actually try on my own time to see if I could write a patch for D8, but I found that I didn't understand enough about D8 at the time to be able to make any progress. I know a lot more about D8 now: if I had the time, maybe I could take another crack at it. But, I don't think I'll have the time before the feature freeze.

Remove the tag or not?

@frob makes a pretty good point — this is a "nice to have" not a "need".

Is there a "would be nice to have a backport to D7" tag instead of a "needs backport to D7" tag?

Let's not argue about the tag. Open Atrium needs it, it is backportable (since it does not change any existing behavior of install profiles unless you mark your install profile as inheriting from another), and people seem to want it. Regardless, it needs to get into D8 before anything else happens with it, so let's focus on that.

mparker17 wrote:
> Also, it would make it easier for us to build sites on other popular distributions.

And for example well-known distributions such as Drupal Commons or Open Atrium have been only recently ported from D6 to D7 (OA for D7 still in alpha). D8 is far away for them. A backport to D7 would be indeed good for Drupal distributions in general. Of course after it's first into D8.

It's not that I don't want it backported; it is that the conversation about backporting never took place. The purpose of backporting is to make sure bugs that get fixed for D8 that that are also in D7 get fixed in both code bases. Personally I want this in D7 too.

So then is the backport patch done? Someone review it please.

We need a patch for 8.x first, and that has to be done in the next couple of days if it's going to be committed before feature freeze.

Rerolled patch from #37 for D8 (some files have moved, also fixed a couple of the typos mentioned earlier in the thread). Let's see how the testbot likes it.

Whoops! Put part of the patch in the wrong section of system.install. Attached patch should fix those errors.

Raising to major per #53.
Not sure though this would be subject to the API freeze, as it is an API addition that shouldn't affect handling of standalone profiles.

In any case this needs extensive test coverage, especially because it is not used by core.
Inheriting requirements basically seems to work but I don't believe we're already taking care of everything.
How about precedence of required modules if multiple instances are included, possibly the one patched by a distro or in a different version? There might be tricky cases to take care of.
How about inheriting config or hook_install or whatever else could possibly be included in a profile? We might omit that but need to make an informed decision.
I believe we can't do this right without a sophisticated inherited profile test case. Another option would be making standard profile inherit from minimal profile, even though that would be a very basic implementation.
Either case this would be an opportunity to vastly expand and improve our profile test cases.

Some definitive input on whether this issue is subject to the July 1st API freeze or not, would be very valuable! Depending on this we need to put much more efforts into this one - or can take our time in order to do it 100% right.

No. This is not major. It has a pressing deadline, but that doesn't make it any more important than the other features that should go into D8 either. This needs to be done before 7/1 if it's going to happen, though there's a possibility that it could go in after (since we're already considering it backportable to 7.x). Safest bet is to plan on having it done by 7/1, and if it turns out that it can be improved after that date, then great.

It has a pressing deadline, but that doesn't make it any more important than the other features that should go into D8 either.

A pressing deadline certainly wasn't my argumentation, especially as I don't expect the deadline to be pressing for this issue.
Rather I refered to your own comment #53, also recognizing that "Major priority is also used for tasks or features which consensus has agreed are important" (Priority levels of issues)

Safest bet is to plan on having it done by 7/1, and if it turns out that it can be improved after that date, then great.

IMHO it's too late for playing safe. This issue still needs a considerable amount of manpower, which not many are able/ready to invest right now that other important tasks really are endangered by July 1st API freeze.

This needs to be done before 7/1 if it's going to happen, though there's a possibility that it could go in after (since we're already considering it backportable to 7.x).

That's a contradictio in adiecto, but with the second half-sentence I do agree. Some authoritative input by a core maintainer would be helpful.

Hi guys,

how can I help? I'm planning to test the patch with a simple custom install profile that extends minimal. Then I could do some tests with upgrades and stuff.

Note, I have been using the patch for D7 and the only thing that does not work is inheriting libraries, which is an issue with the contrib libraries module (my workaround is to use a libraries folder in the root drupal folder).

That being said, I'll devote some hours today to test the D8 patch. If someone could tell me what I can do to do n accurate test, that would be helpful! Let's get this in (and backported ;))

dagomar, I think that the best thing to work on is to make sure unit tests are in place. It will be easier on everyone if they get in now.

Thanks for the reply frob. I have 0 experience with unit testing, just barely know what it is tbh. I'll read up on it and see if I can do anything. In the meantime I'll test the patch manually to see if anything obvious comes up...

I'm glad to see this issue is gaining interest again!
@dagomar This is a corresponding patch I've been using with Libraries API so it can support libraries found in your base profiles. #1811486: Libraries API cannot find libraries in "base" profiles.

I'm guessing that groups.Drupal.org will want this so that it can inherit commons as part of the D7 upgrade. This patch is a pretty big deal from a functionality perspective so I would love to see it go in.

What's the best way to help with testing?

The best way to help, it seems, would be to write tests for this functionality... it may be a bit complicated, since testing the installer/profiles is already more complex than testing systems that only run in a fully-bootstrapped Drupal environment :-/

I would consider making standard a sub-theme of minimal. Then verifying that tests for both themes are being executed.

Re-rolled with minor updates the D7 patches.

Please leave the status at D8, since the patch has to go in there first.

Hey,

What patch is everyone using above for D7? Reason I ask is I have had countless issues with many if not all of them. I have included another patch completely fresh.

Aaron

#59: 1356276-base-profile-59.patch queued for re-testing.

Right now, we need to focus on getting this into D8. Once that's done, it can go into D7. If you want to move this forward, focus on D8 first.

#59: 1356276-base-profile-59.patch queued for re-testing.

#59: 1356276-base-profile-59.patch queued for re-testing.

I'm going to try to update the issue summary and possibly get some tests written for the patch in #59 for Drupal 8.

To those wanting this for Drupal 7: Please, please, please consider opening a separate issue to track the D7 patch(es) alongside this issue, instead of continuing to add them to this issue directly for now—it's already a slim-to-none chance of this getting into D8 (though I think it would be awesome!), but the signal-to-noise in this issue is getting pretty bad.

I created a D7 issue here: https://drupal.org/node/2067229

@frob - thanks! I've updated that issue's summary, and just a note—when linking to other issues, you can use syntax like [#NUMBER], where NUMBER is the issue number, or [#NUMBER-COMMENT] to point to a specific comment in the issue. For example:

• #2067229: Allow install profiles to declare base profiles for Drupal 7
• #2067229-1: Allow install profiles to declare base profiles for Drupal 7

Updated issue summary. added link to D7 issue.

Updated the issue summary with the issue summary template.

Re-rolled the patch from #59. No tests added yet, but some things have moved as a result of #mwds's WSCCI sprint.

Missed a couple lines in system.install again. Testbot, go!

Zero-byte file in last comment. This one may work.

Wait what? It shows up as having passed now. Perhaps Testbot was feeling a bit under-the-weather?

That testbot had a hiccup, and so the test was manually re-run. It passed, so back to needs review!

Changed 'parent' to 'base' to reflect latest patches.

Updated remaining tasks.

Attached patch:

• Parses .info.yml files instead of .info files (which no longer exist in D8).
• Makes the standard profile depend on the minimal profile, and remove bits of minimal that don't belong in standard.

When installing standard profile manually (via UI) with latest HEAD, I'm getting the following error (which can be ignored, and the rest of the install still completes): The plugin (search_form_block) did not specify an instance class.. I'm guessing this is a regression in core—see #2067881: Search is missing from block admin UI after installation.

I might be able to introduce a test or two to make sure that standard inherits from minimal properly—or maybe I'll use the test profile to do a little extra inheritance testing. I'm guessing this patch will fail due to the above issue.

Attaching an actual patch this time...

We also need to make sure that the tests from all profiles get fired

So... something is funky with the search form block—if I simply remove core/profiles/standard/config/block.block.bartik.search.yml and try the install again, the install completes successfully, with everything enabled and configured correctly. With that file, I get the error The plugin (search_form_block) did not specify an instance class..

I'm looking into why this might be happening (maybe a different bug?).

[Update: I can't figure out what's causing the search_form_block to make the installation fail. Other than that, this patch is working well...]

Updated old .info file syntax to YAML.

I updated the issue summary's remaining tasks; primarily, someone needs to figure out why enabling the search form block breaks the standard install profile.

Worth noting that in the original patches and the Agile Approach blog, it was considered a feature that a sub-profile did not actually depend on the parent. In that way you could pick and choose which aspects of the parent you actually need to use.

I am agnostic about this--but if anyone is playing with patches since #36, dependency is presumed.

@Grayside: Can you share the link to original patches and Agile Approach blog? Would love to have a good read about that approach.

#36 has the latest patch, http://www.phase2technology.com/blog/inheriting-your-drupal-profile-from... for the blog post.

...since I don't have time to keep pushing this right now.

Grr... Cross post?

@Grayside: Ah yes, I have read that as well. You previously mentioned that you don't believe the dependency between the base and derived should be enforced, what are the benefits of this in your opinion?

@geerlingguy: I would be happy to take over and assist in pushing this if you would like?

@amcgowanca - go for it!

@amcgowanca, I haven't actually formed a definite opinion on that. My point was really just to identify for issue visitors that there was a significant functional change in what's being done.

Happy to go into it further for the sake of clarity.

The original version of this patch limited the concept of profile inheritance to module & theme discovery. In this way you could build a codebase in which one profile could specify "get all the stuff for my parent profile and stick it in that corner, I'll add my own stuff as needed then decide what to do with it all."

The evolved version says "also, enable all that parent stuff as a default assumption, I'll turn on my additions."

The latter approach makes sense if a parent profile is treated as a black box to which you want to add more stuff. I'd say that takes more planning. The Standard profile ships with Overlay, OA ships with a Blog feature... if those are enforced on, you can't easily replace them with different solutions in your child profile without retroactively disabling those modules.

One example of how this might work is the need to separate Drupal products into minimal and standard profiles like Core. Developers can build child profiles that sit on top of the access control, plugin mechanisms, theme assumptions, and so forth of the minimal profile. Site builders can grab the complete product, make tweaks, and deploy their solution.

In a current project that forced me to go back to #36, I have a child profile that complete swaps out the media handling of the base profile. By not enabling those modules in my child profile, I am free to swap in my own media handling modules. Since those were Features modules tossing content types and fields around, it would not be pleasant to dig in and undo all that work by hand instead of skipping the module enable.

#92: 1356276-base-profile-91.patch queued for re-testing.

Re-rolling the patch to match changes over the last 3 months.

Still needs to be tested, though.

Re-re-re-re-...-re rolled the patch.

If I could get a hand with the failure of the patch that would be awesome. I basically tried to reroll the original patch, and it looks like i could have missed something somewhere a long the line.

@geerlingguy or @amcgowanca have you had any issues with the patch failing tests.

Thanks for the help and thanks for the original patch!

Yes, I had trouble with the search form block (see #95). Couldn't figure it out after a few hours of debugging at the Midwest dev summit.

Just an update, for those using D7 and interested in this issue; @bryanhirsch has a module (in development) that may offer an alternative solution for now. See his comment in #2067229-60: Allow install profiles to declare base profiles for Drupal 7.

The patch in #112 no longer applies and would need more than a simple reroll. Plus, even if I'd love to see this shipped in 8.0.x, the feature freeze won't allow it so I'm postponing to 8.1.x.

postponed isn't the correct status for this issue, it still needs work even if it is being pushed to 8.1