With the addition of info files and install hooks to installation profiles, they gain a status equivalent to that of modules and themes. However, in order to make an installation profile available, unlike modules or themes, it must apparently be added to the profiles/ folder in the code-base itself.

This may work (more or less) when the profile and code-base are distributed together as a third-party package, but if eg. a profile is supposed to be downloaded by itself or created by the local web admin, we want it to be outside the code base, for the same reason as other addons. That's exactly what sites/all/ or sites/[conf_path] is for.

I propose that install_find_profiles() be extended to use the same search pattern as module and theme discovery, looking in ./profiles/, ./sites/all/profiles/ and ./sites/[conf_path]/profiles/ (in order of increasing priority, to allow overrides).

Files: 
CommentFileSizeAuthor
#61 platform.profiles.61.patch19.84 KBsun
PASSED: [[SimpleTest]]: [MySQL] 39,799 pass(es).
[ View ]
#61 interdiff.txt1.78 KBsun
#44 platform.profiles.44.patch19.57 KBsun
PASSED: [[SimpleTest]]: [MySQL] 37,109 pass(es).
[ View ]
#44 interdiff.txt2.71 KBsun
#42 platform.profiles.42.patch18.57 KBsun
FAILED: [[SimpleTest]]: [MySQL] 18,877 pass(es), 298 fail(s), and 296 exception(s).
[ View ]
#42 interdiff.txt2.11 KBsun
#40 platform.profiles.40.patch18.13 KBsun
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#40 interdiff.txt8.8 KBsun
#38 562042.patch16.18 KBRobLoach
FAILED: [[SimpleTest]]: [MySQL] 10,824 pass(es), 347 fail(s), and 326 exception(s).
[ View ]
#37 562042.patch17.89 KBRobLoach
FAILED: [[SimpleTest]]: [MySQL] 10,825 pass(es), 347 fail(s), and 325 exception(s).
[ View ]
#36 1661468.patch12.78 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 37,007 pass(es).
[ View ]
#26 562042_20_C_M.patch14.19 KBscor
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 562042_20_C_M.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#20 562042.patch53.27 KBRobLoach
FAILED: [[SimpleTest]]: [MySQL] 24,670 pass(es), 7,231 fail(s), and 3,840 exception(es).
[ View ]
#9 screenshot-113.png83.49 KBmgifford
#6 install-profile-system-listing-562042-6.patch6.05 KBcburschka
Failed: Invalid PHP syntax in includes/install.inc.
[ View ]
#5 install-profile-system-listing-562042-5.patch5.9 KBcburschka
Failed: Failed to install HEAD.
[ View ]
#3 install-profile-system-listing-562042-3.patch5.11 KBcburschka
Failed: Failed to install HEAD.
[ View ]
#1 install-profile-system-listing-562042-1.patch2.58 KBcburschka
Failed: Failed to install HEAD.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.58 KB
Failed: Failed to install HEAD.
[ View ]

Surprisingly simple. This patch replaces file_scan_directory() in install_find_profiles() with drupal_system_listing().

A slight modification was required to install_profile_info to remove the hardcoded ./profile/$name path. Specifically, I changed install_profile_info to take the whole file object (name, uri and all) instead of just the name, and used dirname($profile->uri) to get the proper directory.

I think this will also fix an undiscovered bug when a profile is deeper in the ./profile hierarchy (for example, if there is a "profiles/lovelycms/lovelycms_minimal/, profiles/lovelycms/lovelycms_full/" structure), as file_scan_directory() will already recurse, but install_profile_info() will assume a direct sub-directory of profiles/.

This is a preliminary patch that I have only tested as far as the select-profile screen. More fixes might be required to actually install it properly.

Status:Needs review» Needs work

The last submitted patch failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.11 KB
Failed: Failed to install HEAD.
[ View ]

Had to extend several other places that hardcode the path into using the object properties instead. drupal_check_profile() needs a parameter change as well.

The tests will probably fail to pass again, as I haven't updated them yet. Just trying to get the site to install for now.

The last submitted patch failed testing.

StatusFileSize
new5.9 KB
Failed: Failed to install HEAD.
[ View ]

... what?

-

Anyway, yes it did, but only by manual testing, not the unit test. I had to modify _system_get_module_data() to use drupal_get_filename() for the profile path instead of hard-coding it. Here's the new patch.

Glad that at least the existing test case doesn't break anymore.

StatusFileSize
new6.05 KB
Failed: Invalid PHP syntax in includes/install.inc.
[ View ]

Ah. SimpleTest depends on install_profile_info() working the other way. That's why the unit test breaks so strangely.

It's relatively simple to allow both, which this patch does.

I applied the patch and it worked fine. I also set up a new install with this patch. That didn't run into any problems either.

I didn't actually put another profile in the site to test that piece.

Other than whether there are any hidden problems in other parts of the system not yet test-covered, one of the main questions for this patch is now the design (ie. does it do things the way they should be done).

What I'm particularly worried about is that install_profile_info() accepts either a file object of the .profile, or a base name of the profile. In the first case, it uses the file object's URI to locate it, in the second case it calls drupal_get_path().

The justification for not doing it the first way globally is that SimpleTest and drupal_get_profile() work with bare profile names, and reworking them would drastically change the scope. As to the second option, the installer already works with file objects in $install_state['profiles'] which have all the information. Passing one part (the name) of the information to install_profile_info(), only for it to have to scour the system files for another part of the information (the path) doesn't feel right.

Tagging this "review swap" - anyone who takes a good look at this patch (anything from design, rigorous testing, code style) gets a patch review from me in return. :)

StatusFileSize
new83.49 KB

I've applied this patch again and tested it a bit further. I set up 2 different mock install profiles. Placed one in /profiles and the other in /sites/all/profiles.

Both showed up properly. I got this error during the install:

An error occurred.
Path: http://localhost:8888/drupal-cvs/install.php?profile=not_default&locale=en&id=1&op=do
Message:
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'filectime' at row 1

I hate these new D7 errors. However, in this case I'm quite certain it's because the install profile I constructed was faulty. I could have just replicated all of the functionality, but decided to add/remove modules.

Are you working from a fully up-to-date HEAD? This error was occurring earlier today on clean installs due to a missing file....

I just updated the CVS. I also copied the expert profile into /sites/all/profile & just renamed everything to expert2.

After that the install went off seamlessly!

What else would this need to be committed? Bringing profiles up to the level of themes/modules makes sense to me.

a site can't have multiple install profiles, so it doesn't make sense to search for profiles in the site directory.

plus then you have sites/$site/profiles/$profilename/modules

it just doesn't seem worth the extra complication, all the additional nesting.

Also, keep in mind that we're working on packaging scripts to create the entire profile and all dependencies as a single download.

profiles are basically applications built on the drupal framework,
and sites are instances of those applications.

sites/all is not an instance.

profiles/$profile is not an instance either.

@Arancaytar, having /profiles in parallel to /sites is by design. Both the /sites and /profiles folders allow you to make themes and modules available to a site under certain conditions. For example /sites/foo.example.com/modules exposes module to a site that is accessible at foo.example.com, whereas a /profiles/$profile/module exposes a module to a site if it was *installed* using that profile.

Stacking the sites and profiles concepts in such a way that all profile must live in /sites/all/profiles doesn't get us anything flexibility wise, and makes it less clear when a module will be available to a particular site.

I'm with Adrian and I'd vote that we don't change how this works.

Status:Needs review» Needs work

The last submitted patch failed testing.

Assigned:Unassigned» Dave Reid

Subscribing, I still think this should be valid. If I have a site-specific profile I'd like to have it included in sites/mysite.com/profiles so it could be properly put in a revision system per-site.

I agree; this seems to make sense - sites/all/profiles would be for profiles that you intend to make available for any site to install with, whereas sites/example.com/profiles would be for profiles that only that particular site is allowed to choose from. This would bring consistency with modules and themes, plus help with upgrade confusion (i.e., the general mantra that you can safely upgrade core as long as you keep your sites directory intact would actually be closer to the truth). I don't think I see the downside.

Not sure if this should be for Drupal 7 at this point though. And it does seem like it would require changes to the d.o. profile packaging scripts.

Version:7.x-dev» 8.x-dev
Status:Needs work» Needs review
Issue tags:+Needs issue summary update, +Platform Initiative
StatusFileSize
new53.27 KB
FAILED: [[SimpleTest]]: [MySQL] 24,670 pass(es), 7,231 fail(s), and 3,840 exception(es).
[ View ]
Move /profiles to /core/profiles
This reflects the structure introduced in #22336: Move all core Drupal files under a /core folder to improve usability and upgrades, and means that people will not mess around with Drupal core when they want to add additional install profiles.
Allow use of sites/all/profiles
This gives users the ability to add more install profiles for any domain they want.
Allow use of sites/example.com/profiles
This gives the user the ability to expose install profiles to specific domains they want.

Adding the Platform Initiative tag since different platforms could essentially be installed and maintained via different install profiles. Also, this probably needs a nicer Issue Summary.

Status:Needs review» Needs work

Good stuff Rob! Could you use git diff -C -M so the patch is easier to review?

Status:Needs work» Needs review

Good stuff Rob! Could you use git diff -C -M so the patch is easier to review?

Unfortunately it wasn't working with git diff because we're moving files. I used git format-patch instead.... If you just scroll down, you'll see it's pretty much the same format.

try git format-patch -C -M origin/8.x --stdout > patch.patch

You have my support!

Status:Needs review» Needs work

Also just thought it might be a good idea to add a sites/all/profiles/README.txt and core/profiles/README.txt.

StatusFileSize
new14.19 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 562042_20_C_M.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

same as #20 with the file move hunks hidden.

Also just thought it might be a good idea to add a sites/all/profiles/README.txt and core/profiles/README.txt.

I don't think either of these are necessary. If you're working on making an install profile, you're already in a <1% of Drupal developers. Any company or user that is providing a distribution should be able to figure out how profiles work. I personally would prefer to avoid adding a "sites/all/profiles" directory by default for the same reason (you're already working with <1% of sites that use custom profiles). And since you can't add an empty directory to Git version control, avoiding a sites/all/profiles/README.txt is the same thing as not including the directory by default.

Other than that comment. I'm not sure this passes the smell-test:

  * @param $profile
- *   Name of profile.
+ *   Either the profile name, or the profile object itself.

I'd prefer a way to easily load a "profile object" if we're going to introduce a new object. Unfortunately install_load_profile() is already in use. On the flip side, neither modules nor themes have "module objects" or "theme objects". As the goal of this patch is consistency with other Drupal er, "things" (modules/themes/theme engines), I'd like to reuse our existing patterns or consider if other things need this new pattern applied to them.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 562042_20_C_M.patch, failed testing.

Since distributions are shipped as install profiles, we need install profile discovery on a per-site basis. Therefore, I'm adding this to the distribution blocker queue.

indeed.

I agree with this proposal, but would like to see the top-level /profiles to be re-purposed, so that it has the effect of the proposed sites/all/profiles here (i.e., don't do sites/all/profiles).

IIRC, @quicksketch or someone else proposed the same for /modules (vs. /sites/all/modules), but I'm not able to find the issue that proposes it.

In the end, we should not throw the multi-site feature into everyone's face. Let's have simple top-level /modules, /themes, /profiles directories instead (all of them as replacement for current /sites/all/ equivalents).

Status:Needs work» Postponed

Let's move profiles to core/profiles first, so that the scope of this issue is a bit smaller: #1661468: Move install profiles to core/profiles

Assigned:Dave Reid» Unassigned
Status:Postponed» Needs work

#1661468: Move install profiles to core/profiles has been marked as duplicate, because the proposed strategy would have suboptimal consequences:

[...] if we do it this way, then aforementioned issue has to be bumped to a critical task and release blocker. I guess I'd rather prefer to do both in one to prevent that critical release blocker scenario.

So let's move the patch from over there in here and perform the full change in this issue. Also unassigning @Dave Reid, since he wasn't involved lately anymore.

@sun Big +1 to #32, though it seems like that could be done all at once in that issue without changing this one.

Do you have a link to that issue or proposal now? I couldn't find it but would like to help.

StatusFileSize
new12.78 KB
PASSED: [[SimpleTest]]: [MySQL] 37,007 pass(es).
[ View ]

@glenn The current patch is attached. Support for sites/all/profiles and sites/example.com/profiles is missing from it, as outlined in #20.

Status:Needs work» Needs review
StatusFileSize
new17.89 KB
FAILED: [[SimpleTest]]: [MySQL] 10,825 pass(es), 347 fail(s), and 325 exception(s).
[ View ]

Hmmmm.

StatusFileSize
new16.18 KB
FAILED: [[SimpleTest]]: [MySQL] 10,824 pass(es), 347 fail(s), and 326 exception(s).
[ View ]

Forgot to remove the standard2 testing profile.

Status:Needs review» Needs work

The last submitted patch, 562042.patch, failed testing.

Status:Needs work» Needs review
Issue tags:-Needs design review, -Review swap
StatusFileSize
new8.8 KB
new18.13 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Jesus. That install system code must date back to Drupal 5 or even 4 times... Anyone with a reasonable grasp of current coding patterns in HEAD should be able to roll a monster patch to bring it on par with the rest of core... But of course, that's off-topic for this issue. ;)

Status:Needs review» Needs work

The last submitted patch, platform.profiles.40.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.11 KB
new18.57 KB
FAILED: [[SimpleTest]]: [MySQL] 18,877 pass(es), 298 fail(s), and 296 exception(s).
[ View ]

acd7228 Fixed install profile URI is lost when rebuilding module info.

Status:Needs review» Needs work

The last submitted patch, platform.profiles.42.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.71 KB
new19.57 KB
PASSED: [[SimpleTest]]: [MySQL] 37,109 pass(es).
[ View ]

d135a0f Fixed install_profile_info() / drupal_get_filename() fails to locate any profiles during installation.

This is a major improvement to Drupal as it means that people will no longer have to "hack core" to add new install profiles. Contributed Drupal distributions will finally be able to be packaged correctly with non-core install profiles in sites/all/profiles rather than mixed in with the core ones.

Tested by doing the following:

  1. Copied core/profiles to sites/all/profiles/standardtwo
  2. Replaced all instances of "standard" with "standardtwo" in the new standardtwo profile
  3. Visited core/install.php and saw Standard Two in the profile listing
  4. Installed the new site with the standard two profile

I'd consider this RTBC, but one question....

+++ b/core/includes/bootstrap.incundefined
@@ -888,6 +889,12 @@ function drupal_get_filename($type, $name, $filename = NULL) {
       }
+      // Profiles are converted into modules in system_rebuild_module_data().
+      // @todo Remove false-exposure of profiles as modules.
+      elseif ($original_type == 'profile') {
+        $dir = 'profiles';

Do we have a follow up set up for this? When does this case happen?

I searched the queue, but wasn't able to find an existing issue for that. (Admittedly, I ran out of steam quickly, since the search keyword "profile" yields thousands of totally irrelevant results ;))

So I've created a new one: #1676196: Install profiles are registered as modules

Status:Needs review» Reviewed & tested by the community

Done!

Title:Search for profiles in site foldersSearch for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profiles

Speaking of ambiguity, let's clarify this issue title... ;)

Assigned:Unassigned» sun

Issue tags:+API change, +API clean-up

Tagging.

Status:Reviewed & tested by the community» Needs work

Sorry to come into this late in the game, but after reading this issue a bit more carefully now, I have a concern.

This issue addresses (correctly) the idea that more and more people want to create an install profile that can be downloaded from D.O. (or wherever) as a simple profile folder and dropped into the profiles folder (wherever it lives). As it was noted in the original post regarding the current state of the profiles folder:

This may work (more or less) when the profile and code-base are distributed together as a third-party package

Unless I'm missing something, the solution proposed in this issue can make it harder for organizations like mine that are developing one distribution of Drupal with multiple custom install profiles. I dont want to maintain "views" in 3 different folders in sites/all/profiles.

The way the folders are currently structured, there is a simple solution to our problem (add profiles/all - see #1617860: /profiles/all folder for distributions that include multiple install profiles) but if the proposal in this issue is committed than there is no longer an easy solution for those people who DO distribute drupal as a third-party package with multiple profiles.

Thoughts?

Status:Needs work» Reviewed & tested by the community

Instructing users to mix their own install profiles with Drupal core's install profiles is, more or less, instructing them to "hack core". This is why we have sites/all/modules instead of asking them to place their modules in core/modules. Having sites/*/profiles stays consistent with the community standard to use sites/*/[modules,themes,etc].

We could still set up sites/*/profiles/all no problem, but that should happen over at #1617860: /profiles/all folder for distributions that include multiple install profiles.

I don't see how this changes anything with regard to your issue of sharing modules between profiles. The current solution provides not better way to handle this, you simply place views in sites/all/modules. Putting your modules in profiles/[name]/modules will not be a requirement to use profiles. Instead, it will simply add a new place to look and allow 3rd party profiles to package everything together.

That said the whole concept of sharing modules between profiles online work when you have complete control over all the profiles....otherwise you cannot count on a specific version (which you may need to and should do in quality profiles).

That said the whole concept of sharing modules between profiles online work when you have complete control over all the profiles....otherwise you cannot count on a specific version (which you may need to and should do in quality profiles).

This is precisely the distinction we use to decide which modules to put in a profile vs. which to put in sites/all ... if we need to control the version it should be in profiles. So in the case where we have a distro with multiple install profiles and they all rely on views-7.x-3.1 where should we put views? Sites/all is not quite right in that case ...

Anyway I fear I'm getting off topic and I dont want to side track this issue any more than I have .... I just want to make sure it wont get any harder to solve the issue we have been having with profiles sharing modules were this patch to be committed.

@bleen18: In fact, this issue + patch here is a fundamentally required cornerstone to make #1617860: /profiles/all folder for distributions that include multiple install profiles possible in the first place. The suggested change over there is incomplete; it doesn't account for the all the plumbing that is additionally required to make the installer and install profile discovery aware of the additional profile folders. That's what this patch here is doing. :)

#44: platform.profiles.44.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

+++ b/core/includes/install.core.incundefined
@@ -1098,7 +1100,12 @@ function install_settings_form_submit($form, &$form_state) {
function install_find_profiles() {
-  return file_scan_directory('./profiles', '/\.profile$/', array('key' => 'name'));
+  $profiles = &drupal_static(__FUNCTION__);
+
+  if (!isset($profiles)) {
+    $profiles = drupal_system_listing('/^' . DRUPAL_PHP_FUNCTION_PATTERN . '\.profile$/', 'profiles');
+  }
+  return $profiles;

Is install_find_profiles() really run more than once during the same request? I didn't see any discussion of this in the issue.

+++ b/sites/all/README.txtundefined
@@ -1,7 +1,7 @@
+files. Place contributed and custom modules and themes in the sites/all/modules,

The order of modules, themes and profiles is inconsistent. Also there is "module and themes" twice without the associated "profiles" - we should add that consistently.

Status:Needs work» Needs review
StatusFileSize
new1.78 KB
new19.84 KB
PASSED: [[SimpleTest]]: [MySQL] 39,799 pass(es).
[ View ]

d783e7d Fixed install_find_profiles() is only called once and can be removed.
691916f Fixed inconsistent order of modules/themes/profiles.

Status:Needs review» Reviewed & tested by the community

Looks good.

Title:Search for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profilesChange notification Search for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profiles
Category:feature» task
Priority:Normal» Critical
Status:Reviewed & tested by the community» Active

Still looks good and unblocks some other issues. Committed/pushed to 8.x.

Title:Change notification Search for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profilesSearch for install profiles in sites/[all|site]/profiles folders, and move core profiles into /core/profiles
Category:task» feature
Priority:Critical» Normal
Status:Active» Fixed

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