One of the major problems with install profiles, is that they do not have access to the Drupal API, and are _only_ loaded during install.
This results in almost every install profile having to write their own custom contrib module, to do the actual work the install profile aims to do.

I propose
a) always loading the install profile code
b) adding the install profile namespace to the bottom of module_listing

This will give install profiles access to the full Drupal API, and work towards removing the differences between install profiles and modules.

Install profiles thus become a superset of the Drupal API and not a subset, and you have to jump through fewer hoops to write install profiles.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rickvug’s picture

Issue tags: +installation profiles

tagging

moshe weitzman’s picture

Is it too much to make an install profile an actual module that implements distribution=TRUE in its .info file? Not sure what that implies for the UI of the modules page.

joshmiller’s picture

Yes!

robeano’s picture

I agree with moshe. When you consider the adrian's other proposed changes to install profiles (adding .info and .install support, see http://drupal.org/node/509404), it sure seems like install profiles are becoming modules.

adrian’s picture

I've looked at this problem a bit, and so far I can determine a few that need to be done.

1) the install profile needs to be registered in the system table
2) install profiles need to be loaded as part of bootstrap
3) the hook registry needs to look for hooks and includes in the profiles dir too.

moshe weitzman’s picture

3) for that matter, i think themes should be able to implement hooks. a different issue, for sure.

webchick’s picture

Moshe's comment at #2 is intriguing to me. *Is* there any reason not to do away with the confusing concept of "installation profiles" altogether in favour of a simple .info file flag for modules?

I think the only thing install profiles can do that modules can't is ship with themes (since both install profiles and modules can include sub-modules). I wonder if it's less work to allow modules to ship with their own themes (which might provide other additional interesting opportunities) than it is to shoe-horn installation profiles into the module system. And I guess finding the possible install profiles would take longer, since you'd be doing a file_scan_directory() through all of the modules folders, gathering up .info files, and filtering the list. But this is something that happens exactly once in the entire lifetime of your Drupal site, so this performance hit seems negligible compared to what feels like might be a substantiative DX improvement.

I can't really think of other cons. Adrian, what say you?

adrian’s picture

@moshe : theoretically, i can perhaps see the point , except that in no uncertain terms that would completely break the separation between the display and business logic.

@webchick : no. not now, not ever. modules and installation profiles are _completely_ _totally_ and _utterly_ different things.

You should read what dries had to say about install profiles : http://buytaert.net/drupal-distributions

At a very high level, modules are interchangeable and configurable bits of functionality that (should) make no direct assumptions on who is using them. Installation profiles are the mechanism by which assumptions are made about how these modules are going to be used in a cohesive system.

The primary reason install profiles have a bad reputation is because the only real mechanisms for making these assumptions is the Drupal API, but unfortunately the Drupal API is not available to these profiles. We have been using the install_profile_api as a band-aid for this problem, but it has still been very painful.

In the most basic sense install profiles are glue code and this issue is not about shoehorning the profiles into the module api, it is about generalizing the module API to cover more of Drupal, to allow us to finally collaborate in a sensible way to build Drupal for intranets, etc. etc.

On a much lower level, the most important part of an install profile that differentiates it from a module is that it provides it's own search path.
The relevant code is here : http://api.drupal.org/api/function/drupal_system_listing/7

The simplest concrete example that I can give regarding this (that also illustrates why we need versioned dependencies) the following :
Install profile A : provides a set of default panels for the Panels 2.x module
Install profile B : provides a set of default panels for the Panels 3.x module

In your approach, the module would need to be installed in sites/all/modules, but you can not install 2 versions of the same module in the same location. So you would need to know what install profiles are running on which sites and place the relevant version of the module for each site in the correct sites/default/module. Now multiply that by 10 000 sites running various install profiles.

So, if you want to go deeper and want to do nested packaged versions of the right modules for each 'install profile module' to be placed in sites/all/module, you would no longer be able to do the simple system_listing to get the overrides, you would need to find the active install
profile, get all the results for it, do the recursive search on the rest of the tree and replace the results from the last search with the results of the first search. And this would need to be done every time the system table is updated for every 'thing' that drupal manages.

Plus you need to throw in the fact that sites installed with a certain install profile _should_not_ be able to see the modules in the other install profile. This makes finding the right module, the right tpl.php , the right whatever , orders of magnitude more complex.

And the only benefit you would get from this is that you can drop your packaged 'install profile module' into the modules directory instead of the profiles directory.

The end result of this is essentially that multiple install profiles would not be able to easily exist in the same drupal directory and most people would end up having a separate drupal directory for each profile they run. This is just a nice way of saying we have semi-forked the code base by forcing the end user to do the forking themselves whenever they run the code, while needlessly and endlessly complicating their maintenance and their ability to keep their systems up to date.

Keep in mind this is the very simplest example I could have chosen to illustrate this point, and the simple fact is that the main idea behind install profiles are sound, but the current implementation isn't. What install profiles are right at this moment is not what they should actually be, but we can actually fix this.

I feel that there is such a large amount of cognitive dissonance around the nature of install profiles and distributions that many people do not understand fundamental considerations such as why you aren't able to switch install profiles on a site that has already been installed and why you can't have 'multiple' install profiles running on a site.

As part of my holy crusade to make install profiles work I will also be working towards educating people about all these issues, but this issue queue is not the place to do that.

Until I get these concepts communicated in a more adequate venue, please accept that:
install profiles are not modules. To make them useful they need to be more like modules, but we can not do away with install profiles altogether.

ac’s picture

subscribing

chx’s picture

With all that said, install profiles still should be modules in the profiles directory. It makes things a lot and a LOT easier. Instead of a lot of special casing, we have everything granted. The only thing is that we do not add profiles as a normal search path but only deal with one, hardwired module from there. Edit: this last sentence makes no sense, see following comments.

chx’s picture

Moshe points out that we already have the profile as a search path and yeah, $searchdir[] = "profiles/$profile/$directory"; in drupal_system_listing. If this is the case, then why on earth not make 'em a module? #509404-10: Fix some conceptual problems with install profiles and make them actually usable says that

would remove the last profile specific hook and instead use hook_menu to provide post install tasks. Goodbye strange hook_profile_tasks state machine malarky.

aye and if it would be a module then that would be granted immediately.

chx’s picture

To clarify, Adrian's chain of arguments is built on "In your approach, the module would need to be installed in sites/all/modules" which is not true, it sits happily in profiles/foo and all the modules that it requires would ALSO sit in profiles/foo which is already supported.

eaton’s picture

Chiming in to support this as well; although I suspect it won't make ti for D7, it's an important step towards maintainable Drupal distros.

adrian’s picture

Status: Active » Needs review
FileSize
26.75 KB

This patch adds the install profile to the results returned from system_get_module_data, and therefore adds the profiles to the system table.

The existing default_profile_setup was no longer necessary, and is now default_install in the new default.install file.
This patch also resolves:
#80272: Make install profiles updatable

I did not touch the existing wizard code, and it will likely still work but there is no core install profile i can test it with.
The patch I want to write for the following issue will almost completely replace this system, but it should still be working :
#525594: Installation should consist of 2 phases instead of one

There are some small inconsistencies, specifically in the user interface where the install profile is referred to as a module, but the code is incredibly simple, and works during install, update and normal operation.

I'm not sure, but i think this also allows modules to depend on profiles.

The testing system is down, but I think it should work. will know as soon as something changes.

adrian’s picture

Title: Install profiles should be added to the module_listing and always be loaded » Install profiles should be modules with full access to the Drupal API and all it entails(.install files, dependencies, update_x)

better title

adrian’s picture

Title: Install profiles should be modules with full access to the Drupal API and all it entails(.install files, dependencies, update_x) » Install profiles should be added to the module_listing and always be loaded

I really think that this patch will make this even simpler #545452: Store install profile in the generated settings.php file

so we can remove all the weird install_state stuff

adrian’s picture

Title: Install profiles should be added to the module_listing and always be loaded » Install profiles should be modules with full access to the Drupal API and all it entails (.install files, dependencies, update_x

pp[s

moshe weitzman’s picture

Title: Install profiles should be modules with full access to the Drupal API and all it entails (.install files, dependencies, update_x » Install profiles should be modules with full access to the Drupal API and all it entails(.install files, dependencies, update_x)

Wow. Once again, this simplifies life for the profile author. Authoring a profile is looking a *lot* like module authoring which is just great. I did a code review and all looks swell. Lets see what test bot says.

Status: Needs review » Needs work

The last submitted patch failed testing.

adrian’s picture

Status: Needs work » Needs review
FileSize
26.65 KB

Here's an updated patch that passes more of the tests (my vm keeps on crashing during testing, so i haven't finished testing, but no issues so far).

I just updated to head and modified the test case slightly so the profile gets installed last.

Status: Needs review » Needs work

The last submitted patch failed testing.

adrian’s picture

Status: Needs work » Needs review
FileSize
27.4 KB

Fixed the module API test, i needed to include the profile as one of the modules that would be loaded.

eaton’s picture

Status: Needs review » Reviewed & tested by the community

I just took this for a spin, and I can't say enough good about it. Getting install profiles into the mix,. giving them access to the full API, and allowing them to participate after the initial events on install.php allows much, MUCH richer configuration and ongoing maintenance of highly customized Drupal based web-apps.

It means more people will be able to write profiles, because they understand how to use and maintain modules. It means profiles will be better supported, because their customizations can be maintained via update hooks.

This patch has the highest 'awesome-per-line' quotient I've seen in a long, long, LONG time.

It's a very small amount of actual code (mostly just moving existing functions to different files), it's passing tests, and a reading of the code itself makes perfect sense.

RTBC, with great anticipation.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

This patch has the highest 'awesome-per-line' quotient I've seen in a long, long, LONG time.

You know what else it has is the highest "moving WTF to non-WTF" quotient as well.

The only real downside (and it's not even a downside really, because modules work the same way) is that many profiles will just be a gigantic .install file and a totally blank .profile file, just to get the profile to register. But whatever, we can deal with that (or not) in another issue IMO.

Also, it appears this patch is missing an upgrade path for people who've already got Drupal installed. I'm not really sure how to handle this.

+++ includes/install.inc
@@ -522,6 +518,9 @@ function drupal_verify_profile($install_state) {
+  // We have already verified that the install profile is present.
+  $present_modules[] = drupal_get_profile();
+++ install.php
@@ -733,6 +733,11 @@ function install_system_module(&$install_state) {
+    // The install profile is also a module, which needs to be installed
+    // after all the dependencies have been.
+    $modules[] = drupal_get_profile();

Sorry, what? Could we document this line a little better to explain why we're adding a profile to the list of present modules?

+++ includes/install.inc
@@ -1087,6 +1086,10 @@ function install_profile_info($profile, $locale = 'en') {
+    // drupal_required_modules includes the current profile as a dependency.
+    array_shift($info['dependencies']);

drupal_required_modules()

+++ install.php
@@ -733,6 +733,11 @@ function install_system_module(&$install_state) {
+    // The install profile is also a module, which needs to be installed
+    // after all the dependencies have been.
+    $modules[] = drupal_get_profile();

(about the earlier note) Yeah! Like this! :)

+++ install.php
@@ -1424,7 +1429,12 @@ function install_finished(&$install_state) {
+    db_update('system')
+      ->fields(array('weight' => 1000))

Why weight = 1000? Could we get a snippet of documentation here?

+++ modules/simpletest/drupal_web_test_case.php
@@ -1062,7 +1063,7 @@ class DrupalWebTestCase extends DrupalTestCase {
+    variable_set('install_profile', 'default');

Just curious.

One of the traditional complaints about install profiles is that you can only ever install one of them. By only allowing a single value into this variable and not an array, we're continuing on with this tradition. Is this seen as acceptable, or are you planning on fixing it in another patch, or..?

+++ modules/simpletest/drupal_web_test_case.php
@@ -1086,7 +1087,7 @@ class DrupalWebTestCase extends DrupalTestCase {
+    drupal_install_modules(array('default'), TRUE);

Something else that occurs to me is we're going to open ourselves up here to some wonderful naming collisions between install profiles and modules, but it won't be clear anywhere to someone what profile they're using without hacking the DB. We hide required modules from the modules screen.

Could we please expose this data so it's visible on the status page at admin/reports/status?

+++ modules/system/system.module
@@ -1753,6 +1753,15 @@ function _system_get_module_data() {
+  // Install profile hooks are always executed last.
+  $modules[$profile]->weight = 1000;

(about earlier note) Yes, like that. ;)

+++ profiles/default/default.install
@@ -0,0 +1,216 @@
+ * Implementation of hook_install

"Implement hook_install()."

This file is also missing a line for // $Id$ and a /** @file */ definition. Please fix.

+++ profiles/expert/expert.install
@@ -0,0 +1,71 @@
+<?php
+// $Id$
+
+/**
+ * Implementation of hook_install.

Same deal as the default.install hunk. (though the $Id$ tag is here already)

This review is powered by Dreditor.

adrian’s picture

Status: Needs work » Needs review
FileSize
30.9 KB

Also, it appears this patch is missing an upgrade path for people who've already got Drupal installed. I'm not really sure how to handle this.

I have added an update path. The install profiles get registered as schema 0 from this point on. so any update_x hooks will run by update.php.

Which means we are free to evolve default.install too.

One of the traditional complaints about install profiles is that you can only ever install one of them. By only allowing a single value into this variable and not an array, we're continuing on with this tradition. Is this seen as acceptable, or are you planning on fixing it in another patch, or..?

It's not really possible to switch installation profiles on an already running site without completely breaking the site.
I am however working on #555212: Install profile inheritance

That patch will build on this patch and turn the value into an array, but that's beyond the scope of this issue.

Something else that occurs to me is we're going to open ourselves up here to some wonderful naming collisions between install profiles and modules, but it won't be clear anywhere to someone what profile they're using without hacking the DB. We hide required modules from the modules screen.

Could we please expose this data so it's visible on the status page at admin/reports/status?

We already have that problem, but nobody wants to talk about it. Modules and themes should not share the same namespace, and the same was already true for themes and profiles.

This is only a problem for stuff not hosted on drupal.org, as we are currently enforcing that here.

I have added install profile display to the requirements, but i hide it when the default profile is used because it's superfluous (check pic)

adrian’s picture

FileSize
12.25 KB

pic

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC. Thanks adrian.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Guys. ;) I know we're all excited about this functionality, but can you please make sure patches are actually RTBC before you RTBC them? If I can't trust RTBC on these patches, I need to wait until I have brainspace to do full reviews myself, and with everything else grabbing at my attention, that drastically reduces the chances of it happening.

+++ modules/system/system.install
@@ -28,6 +28,19 @@ function system_requirements($phase) {
+        'value' => $info['name'] . ' (' . $profile . '-' . $info['version'] . ')',

That is not translatable. Other strings in that function use $t().

Additionally, the last two hunks of #24 aren't implemented.

And could I get confirmation that someone tested the upgrade path?

This review is powered by Dreditor.

adrian’s picture

Status: Needs work » Needs review
FileSize
31.02 KB

Weird, that last hunk was fixed in my local code.

Fixed now.

adrian’s picture

The upgrade path can be tested the following way :

1) install drupal without the patch
2) apply the patch
3) go to update.php
4) check the system table to ensure that the default profile is loaded as status = 1 , schema_version = 0

You can test upgrades by adding the following to default.install either before or after you go to update.php

function default_update_1() {
  return array();
}

update.php will now let you run the update on the install profile.

Status: Needs review » Needs work

The last submitted patch failed testing.

adrian’s picture

Status: Needs work » Needs review
FileSize
31.02 KB

Typo in last patch

and rebuilt against latest head

webchick’s picture

Status: Needs review » Fixed

Awesome! :)

I went through Adrian's testing instructions myself and things seem to be working properly. The one annoyance I have lingering is that the row in the system table that points to the profile still says 'type' = 'module'. For the purposes of this patch that was nice, because it kept the changes to the minimum and made it easy to review. But in a follow-up patch it'd be nice to resolve this discrepancy, and/or just rename .profile to .module and be done with it (though the .profile extension is kind of nice for knowing at a glance what you're dealing with, and Adrian pointed out a couple of ways that profiles are actually different than modules [tasks, wizard, etc.]).

Committed to HEAD. On to the next improvement!

yched’s picture

Status: Fixed » Closed (fixed)
Issue tags: -installation profiles

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