Install profile should not be written to settings.php during site-install for Drupal version 8+

Related issues: #827888: Drush not picking up commands from modules located in an install profile in the help message? and #545452: Store install profile in the generated settings.php file

@Greg - I had to look closely to see what is going on. 827888-#9 changed the CONFIGURATION phase, not the SITE phase, which is what you propose to change here. Search for 0c4a04e6 at http://drupalcode.org/project/drush.git/blame/HEAD:/includes/command.inc to see what I mean.

This sounds like a reasonable change. Can you share your use case, or other use cases you have in mind?

From the title it sounds like you want this for help command. Try to add $conf['install_profile'] = PROFILENAME to your settings.php and see if help gets better for you. As noted at #827888: Drush not picking up commands from modules located in an install profile in the help message?, site-install should be expanded so it writes this to bottom of settings.php.

I kind of think we should write $conf['install_profile'] unconditionally. That will help make folks aware that this feature exists and distros will encourage folks to use it. Otherwise, looks RTBC.

Though I don't mind the pile up, we're going to get bug reports from the fastidous masses. i now agree that we should conditionally write is, just like what you had before. we can ignore the pile up problem for custom profiles until a better idea comes along.

I'm worried that someone's settings.php will rely on bootstrap.inc being loaded. Lets not include() settings.php

Maybe we should just do

$contents = file_get_contents('settings.php');
if (strpos('$conf['profile']', $contents) === FALSE) {
  // Append $conf['profile']
}

It looks like were writing install_profile only for D6 sites and only when settings.php did not already exist. I just committed a fix so we write for all Drupal versions and we try to append when settings.php already exists but has no install_profile specified (uses technique from #12 .

Nope, forgot to mark Fixed with #14.

This breaks Drupal 8 testing. I need to remove manually the profile from settings.php.

Where does D8 record the profile that was used? WHats the CMI name?

Could you describe exactly what the failure is when writing this to D8? I am not seeing a problem.

Sure, try to install minimal and run a test.

To elaborate, because you force the install profile to be minimal, testing will give you some notices at least (some tests warnings / fatal) because it will try to operate on minimal profile when that is not actually enabled...

I don't quite buy "because you force install_profile to be minimal". Settings.php has nothing to do with the site-under-test. $databases refers to the parent site, config_directories refers to the parent site, etc. Simpletest should be injecting all that it needs into the site-under-test, including the install_profile. This is yet another leakage from parent to site-under-test. I'll probably fix this in Drush, but it is a Simpletest bug.

Simpletest can't help that $conf overrides are stronger than the variable it sets...

And I bet that there's a problem with Drupal 7 + testing too just noone does that any more :D

I wasn't testing but still ran into an issue with this behavior of Drush on Drupal 7 that gave me a headache.

When doing a reinstall with drush, the settings.php cannot be altered because it has been set to read-only after the previous install, which usually is fine. But when I change the install profile in one of those reinstalls, the $conf[install_profile] line will stay in settings.php there overriding the variables table which has the correct value and completely messing up the site. It is an easy fix if you know what's going on, but a pita if you don't, as that behavior of Drush is totally unexpected.

Drush shouldn't mess with settings.php for purely cosmetic reasons.

Reinstalling a different profile is very much an edge case. Drush needs to know what profile is in use in order to find commanfiles and such, early in the bootstrap. We don't do this for "cosmetic" reasons.

As of today, drupal_get_profile() still does a variable_get(). See http://api.drupal.org/api/drupal/core%21includes%21common.inc/function/d.... Once this is in CMI, we might be able to stop writing to settings.php for D8.

Note: this still breaks testing.

Simpletest can't help that $conf overrides are stronger than the variable it sets...

Simpletest sets up a whole child Drupal. Surely it could inject whatever environment it needs. After all, it injects a db_prefix despite that not existing in settings.php. It could similarly inject an install_profile.

I guess Drush could use its own variable instead of clashing with 'install_profile'

> It could similarly inject an install_profile.

It's only now it can -- but it would require an override settings.php which does nothing but unsets the $conf['install_profile']. Merely doing a variable_set is not enough because $conf wins over variable_get. (And CMI for that matter)

@chx - simpletest could inject the db_prefix injection there too. not sure if that would be better or not.

Here is a patch to change the variable name that we write and read to drush_install_profile so there is no conflict with drupal's install_profile variable.

However, I've just thought of an alternative which is for us to drush_cache_set() the current install profile for a given site. This would be analogous to our commandfile cache for a given site. We would need to figure out where we put the write to cache. I suppose we could do the following during FULL BOOTSTRAP. It would add a cache read for every request though.

if (!drush_cache_get('install_profile') {
  drush_cache_set('install_profile', $install_profile)
}

Here is the patch I mentioned above, though we might not want it.

A drush_cache_set() approach would work for all sites, not just those whose profile happens to get written to settings.php

OK, here is the drush_set_cache() approach that implements what I mentioned. Feedback or questions are welcome.

1. No more writing of $conf['install_profile'] in site-install
2. During full bootstrap, we check the drush cache for this site's install_profile. If not present, we drush_cache_set() it. If present, we do a correctness check and re-cache the value in the unlikely event that the profile has changed (e.g. site got re-installed). This slows us a down a little but I think I'm OK with it.
3. And now the whole point, we read the install_profile during SITE bootstrap phase and load commandfiles from only the right profile. We can now do this for all sites, not just the ones that wrote install_profile in their settings.php

Todo: the cid for the install_profile needs to be made site specific. It is currently the same for all your sites which share a drupal major version. That's not good. Help wanted.

In case it isn't clear, the extra wrk I am proposing would be a fix for some problems reported in #1529968: Don't reload commands that have already been loaded..

I'm not sure how I want to proceed here.

Oops. Here is right patch. It does not yet have Greg's fine suggestion regarding a better cid.

We should decide if we want to go with #35 or #39 or none of the above.

Committed. Thanks.

FYI, the patch in #41 did not apply for me with git appy but I was able to freshen it up per the interdiff.