This is a child of #1775842: [meta] Convert all variables to state and/or config systems

In Drupal 7 install_profile was a variable - the magic of variable get working without a database connection protected us during the early install.

For Drupal 8 we need to do something different.

The patch attached in #61 converts the variable to a setting in settings.php. This is because:

  • this setting changes what modules are available to be installed
  • the only UI to select it is in the installer before settings.php has been made read only
  • there is no UI to change it
  • have different values in different environments makes no sense and it never changes at runtime
Files: 
CommentFileSizeAuthor
#65 61-65-interdiff.txt4.77 KBalexpott
#65 drupal-1827112-65.patch10.3 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 58,398 pass(es).
[ View ]
#61 59-61-interdiff.txt680 bytesalexpott
#61 drupal-1827112-61.patch10.32 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#59 57-59-interdiff.txt553 bytesalexpott
#59 drupal-1827112-59.patch10.01 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 58,118 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#57 54-57-interdiff.txt2.7 KBalexpott
#57 drupal-1827112-57.patch9.27 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 58,128 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#54 52-54-interdiff.txt3.26 KBalexpott
#54 drupal-1827112-54.patch6.81 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 58,083 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#52 drupal-1827112-52.patch6.36 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 57,840 pass(es), 68 fail(s), and 1,509 exception(s).
[ View ]
#50 drupal-1827112-50.patch6.16 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#46 drupal-1827112-46.patch4.52 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 57,914 pass(es).
[ View ]
#44 42-44-interdiff.txt490 bytesalexpott
#44 drupal-1827112-44.patch4.5 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 57,030 pass(es).
[ View ]
#42 40-42-interdiff.txt1.45 KBalexpott
#42 drupal-1827112-42.patch4.53 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 13,340 pass(es), 597 fail(s), and 1 exception(s).
[ View ]
#40 drupal-1827112-40.patch3.15 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 56,991 pass(es), 8 fail(s), and 2 exception(s).
[ View ]
#36 drupal-1827112-36.patch4.73 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 52,236 pass(es), 0 fail(s), and 17 exception(s).
[ View ]
#36 interdiff.txt4.73 KBdawehner
#33 1827112-convert-install_profile-variable-CMI-33.patch4.59 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 52,175 pass(es), 1 fail(s), and 2,262 exception(s).
[ View ]
#31 interdiff.txt1.96 KBdawehner
#31 drupal-1827112-31.patch4.59 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 50,141 pass(es), 1 fail(s), and 1,707 exception(s).
[ View ]
#30 drupal-1827112-30.patch3.04 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 50,087 pass(es), 3 fail(s), and 1,674 exception(s).
[ View ]
#28 1827112-convert-install_profile-variable-CMI-28.patch1.93 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 49,616 pass(es), 1 fail(s), and 1,504 exception(s).
[ View ]
#25 1827112-convert-install_profile-variable-CMI-25.patch1.92 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 49,355 pass(es), 0 fail(s), and 1,504 exception(s).
[ View ]
#20 1827112-convert-install_profile-variable-CMI-20.patch1.53 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 45,633 pass(es), 25 fail(s), and 67,890 exception(s).
[ View ]
#17 1827112-convert-install_profile-variable-CMI-17.patch1.84 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install.
[ View ]
#15 1827112-convert-install_profile-variable-CMI-15.patch1.84 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to clear checkout directory.
[ View ]
#10 1827112-convert-install_profile-variable-CMI-10.patch1.9 KBcspitzlay
FAILED: [[SimpleTest]]: [MySQL] 48,182 pass(es), 0 fail(s), and 1,417 exception(s).
[ View ]
#3 1827112-convert-install_profile-variable-CMI-3.patch1.9 KBcspitzlay
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install.
[ View ]
#1 1827112-convert-install_profile-variable-CMI.patch747 bytescam8001
FAILED: [[SimpleTest]]: [MySQL] 44,605 pass(es), 31 fail(s), and 12,805 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new747 bytes
FAILED: [[SimpleTest]]: [MySQL] 44,605 pass(es), 31 fail(s), and 12,805 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI.patch, failed testing.

StatusFileSize
new1.9 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install.
[ View ]

I replaced a remaining variable_set and added an update function.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-3.patch, failed testing.

The test failure is triggered by the update function included in the patch.
But, IMHO, the bug is in update_variables_to_config.

I opened #1830424: update_variables_to_config loses data with a test.

Status:Postponed» Needs review
Issue tags:-Configuration system

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1827112-convert-install_profile-variable-CMI-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.9 KB
FAILED: [[SimpleTest]]: [MySQL] 48,182 pass(es), 0 fail(s), and 1,417 exception(s).
[ View ]

updated the "N" in hook_update_N()

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-10.patch, failed testing.

Hu? Failed but 0 fails? What's the matter here?
Let's see if this is reproducible ...

Status:Needs work» Needs review
Issue tags:-Configuration system

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1827112-convert-install_profile-variable-CMI-10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to clear checkout directory.
[ View ]

Re-rolling without config save.

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install.
[ View ]

Fixing function name error.

Assigned:cam8001» Unassigned

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.53 KB
FAILED: [[SimpleTest]]: [MySQL] 45,633 pass(es), 25 fail(s), and 67,890 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-20.patch, failed testing.

Hi vijaycs85,

why is saving the config value no longer needed?
How will the profile be stored if a fresh installation with a profile != standard is made?

Thought of variable_set doesn't need for config system as it is in yml.

Well, the value in this yml file is just a default.

But that's not the only possible value (that's why this variable exists in the first place :) ).
The install_finished function stores away the actually used install profile.

Status:Needs work» Needs review
StatusFileSize
new1.92 KB
FAILED: [[SimpleTest]]: [MySQL] 49,355 pass(es), 0 fail(s), and 1,504 exception(s).
[ View ]

Re-rolling with saving profile and name change from install_profile to profile.install as simpletest look green locally.

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-25.patch, failed testing.

Getting below exception:

Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "config.factory" does not exist. in Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition()
(line 690 of /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).
">Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition('config.factory')
Symfony\Component\DependencyInjection\ContainerBuilder->get('config.factory')
config('system.site')
drupal_get_profile()
Drupal\Core\SystemListingInfo->profiles('scripts')
Drupal\Core\SystemListing->scan('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\.script$/', 'scripts', 'name', 1)
drupal_system_listing('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\.script$/', 'scripts')
drupal_get_filename('script', 'test')
Drupal\system\Tests\Bootstrap\GetFilenameUnitTest->testDrupalGetFilename()
Drupal\simpletest\TestBase->run()
simpletest_script_run_one_test('39', 'Drupal\system\Tests\Bootstrap\GetFilenameUnitTest')

Might be related to http://drupal.org/node/1821312.

Status:Needs work» Needs review
StatusFileSize
new1.93 KB
FAILED: [[SimpleTest]]: [MySQL] 49,616 pass(es), 1 fail(s), and 1,504 exception(s).
[ View ]

Re-rolling with update_N change to check #27 problem still persist.

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.04 KB
FAILED: [[SimpleTest]]: [MySQL] 50,087 pass(es), 3 fail(s), and 1,674 exception(s).
[ View ]

Added an upgrade path, sadly this patch still breaks.

The general problem is that drupal_system_listing() is called and needed to construct a container,
so the call to drupal_get_profile() in SystemListingInfo::profiles() fail.

StatusFileSize
new4.59 KB
FAILED: [[SimpleTest]]: [MySQL] 50,141 pass(es), 1 fail(s), and 1,707 exception(s).
[ View ]
new1.96 KB

Let's add an empty config factory into the container.

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-31.patch, failed testing.

StatusFileSize
new4.59 KB
FAILED: [[SimpleTest]]: [MySQL] 52,175 pass(es), 1 fail(s), and 2,262 exception(s).
[ View ]

Re-rolling...

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1827112-convert-install_profile-variable-CMI-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.73 KB
new4.73 KB
FAILED: [[SimpleTest]]: [MySQL] 52,236 pass(es), 0 fail(s), and 17 exception(s).
[ View ]

The problem is that this upgrade function jumps in way too late, drupal_get_profile() is used really early during the upgrade path, so let's fix that in update_prepare_d8_bootstrap().

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-36.patch, failed testing.

+++ b/core/modules/system/config/system.site.ymlundefined
@@ -7,3 +7,5 @@ page:
+profile:
+  install: standard

I'm wondering whether we should use minimal here instead?

Is this really configuration? It tracks the originally installed install profile, and can never be updated (unless you hack it). Seems more like state.

Status:Needs work» Needs review
StatusFileSize
new3.15 KB
FAILED: [[SimpleTest]]: [MySQL] 56,991 pass(es), 8 fail(s), and 2 exception(s).
[ View ]

If you look at other similar stuff this really makes sense. There are examples like the state,
which you will never change.

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-40.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.53 KB
FAILED: [[SimpleTest]]: [MySQL] 13,340 pass(es), 597 fail(s), and 1 exception(s).
[ View ]
new1.45 KB

So by changing variable_get (which has no dependencies other than a global $conf) to state we introduce some problems for the installer...

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-42.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.5 KB
PASSED: [[SimpleTest]]: [MySQL] 57,030 pass(es).
[ View ]
new490 bytes

Ok I should know this already...

Oh gosh, all these globals. Your recent interdiffs are looking great.

StatusFileSize
new4.52 KB
PASSED: [[SimpleTest]]: [MySQL] 57,914 pass(es).
[ View ]

Rerolled

catch rightly points out that install_profile never changes. I would think then that it belongs in config, and not state. Not a big deal though.

Is there a reason why this can't be a setting?

@@ -26,6 +26,11 @@ public static function getInfo() {
+    // drupal_get_profile() is using obtaining the profile from state if the

Maybe just "is obtaining"?

Status:Needs review» Needs work

I agree a setting makes a great deal of sense actually as this is used to change what code is included in you site. Having it in settings.php means that it has very few dependencies - which is a good thing.

Status:Needs work» Needs review
StatusFileSize
new6.16 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Had a little bit of fun with Simpletest but here's the install_profile variable moved in to Settings.

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-50.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.36 KB
FAILED: [[SimpleTest]]: [MySQL] 57,840 pass(es), 68 fail(s), and 1,509 exception(s).
[ View ]

Lets try writing to settings.php earlier in the installer.

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-52.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.81 KB
FAILED: [[SimpleTest]]: [MySQL] 58,083 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
new3.26 KB

How about this?

Of all the options, $settings feels like the best. It's weird that we install a module then base the existence of other modules on that module, but that's a holdover from when install profiles became modules and the introduction of install profiles in general, and not something we can sort out here.

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-54.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.27 KB
FAILED: [[SimpleTest]]: [MySQL] 58,128 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new2.7 KB

I love simpletest

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-57.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.01 KB
FAILED: [[SimpleTest]]: [MySQL] 58,118 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new553 bytes

Drupal\system\Tests\Upgrade\BareMinimalNoConfigUpgradePathTest passes locally :(

No idea why it's failing. Still loving simpletest and pift/pifr :)

Status:Needs review» Needs work

The last submitted patch, drupal-1827112-59.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.32 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new680 bytes

Maybe this will work?

Title:Convert 'install_profile' variable to Configuration systemConvert 'install_profile' variable to Settings

Re-titling per the direction in the latest patches.

#61: drupal-1827112-61.patch queued for re-testing.

+++ b/core/includes/install.core.inc
@@ -1234,6 +1234,14 @@ function install_settings_form_submit($form, &$form_state) {
+      'value'    => $install_state['parameters']['profile'],
+      'required' => TRUE,
+++ b/core/includes/update.inc
@@ -439,6 +439,23 @@ function update_prepare_d8_bootstrap() {
+            'value'    => $old_variable,
+            'required' => TRUE,
+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -779,6 +779,22 @@ protected function setUp() {
+          'value'    => $this->profile,
+          'required' => TRUE,
+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/BareMinimalAnonymousUpgradePathTest.php
@@ -28,9 +28,15 @@ public function setUp() {
+        'value'    => $this->profile,
+        'required' => TRUE,
+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/BareMinimalNoConfigUpgradePathTest.php
@@ -35,6 +40,12 @@ public function setUp() {
+        'value'    => $this->profile,
+        'required' => TRUE,
+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php
@@ -146,6 +146,21 @@ protected function setUp() {
+          'value'    => $install_profile,
+          'required' => TRUE,
+        ),

I have never seen an alignment of the arrow in proper Drupal core code, or in other words, that just looks odd.

+++ b/core/includes/common.inc
@@ -269,11 +270,17 @@ function drupal_get_region_content($region = NULL, $delimiter = ' ') {
+    $profile = Settings()->get('install_profile') ?: 'standard';

Let's keep it a lowercase settings() to be safe if PHP just decides to not be case-insensitive.

StatusFileSize
new10.3 KB
PASSED: [[SimpleTest]]: [MySQL] 58,398 pass(es).
[ View ]
new4.77 KB

Thanks dawehner - changes made

Status:Needs review» Reviewed & tested by the community

Thank you

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks.

+++ b/core/includes/common.inc
@@ -269,11 +270,17 @@ function drupal_get_region_content($region = NULL, $delimiter = ' ') {
function drupal_get_profile() {
...
-  if (isset($install_state['parameters']['profile'])) {
-    $profile = $install_state['parameters']['profile'];
+  if (drupal_installation_attempted()) {
+    // If the profile has been selected return it.
+    if (isset($install_state['parameters']['profile'])) {
+      $profile = $install_state['parameters']['profile'];
+    }
+    else {
+      $profile = '';
+    }
   }

I think this change to drupal_get_profile() is great in principal but I think returning NULL instead of an empty string would make more sense, as at this point there simply is no profile.

This wasn't discussed anywhere above (unless I missed it) so I thought I'd ask for some feedback before opening a follow-up for that.

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

Issue summary:View changes

Updating summary

Unfortunately, the changes to the installer weren't sufficient here.

If there is an existing settings.php already, and if both $databases + $config_directories are set up correctly, then settings.php will not be touched or rewritten in any way by the installer.

As a consequence, if you re-install your Drupal site, the resulting settings.php will not contain the install_profile setting. On /admin/modules, you get the following error message:

Notice: Undefined index: distribution_name in drupal_install_profile_distribution_name() (line 106 of core\includes\install.inc).

Likewise, installation of Drupal halts with a WSOD in the installer, if you choose any other installation profile than the Standard profile.

Created follow-up issue for that: #2156401: install_profile setting is not written to settings.php when re-installing