Problem/Motivation

As a user I want to be able to install Drupal from a package of configuration that is maintained in git.

At DrupalCon New Orleans, a workflow was discussed that would allow a profile would contain the config sync directory and this full config export should be used at install time. T

Proposed resolution

If a install profile contains a config/sync directory this will be use to install the site.

To create an install profile that works this way on an existing site you can use https://www.drupal.org/project/install_profile_generator

To be clear, this issue would only pertain to install profiles. The separate use case of allowing a site to be installed from an existing configuration export which is not part of a profile is covered in #1613424: Allow a site to be installed from existing configuration.

Remaining tasks

Add config validation
Add more test coverage

User interface changes

None

API changes

No API changes per se, there are new steps possible in the installer.

Data model changes

None

Files: 

Comments

mtift created an issue. See original summary.

alexpott’s picture

Issue summary: View changes
alexpott’s picture

claudiu.cristea’s picture

alexpott’s picture

Status: Active » Needs work
Issue tags: +Configuration system
FileSize
2.7 KB

Initial work to support the profile key. Nothing to test because there's no changes yet to anything under test.

alexpott’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
12.78 KB
13.55 KB

So to use this you need a profile with config_install: true in its .info.yml and then a full config export in the profile's config/sync directory.

alexpott’s picture

I think an improvement would be to rename the .info.yml config_sync</code and instead of a boolean allow it to be a directory so the name and location of the config sync directory can be specified. This would allow it to be outside of webroot.

alexpott’s picture

Beginning to add test coverage - no real tests yet just the necessary fixtures. Manual testing of multi-lingual (the most complex core case) shows we need to set the install language to the already selected default language in config. No interdiff just it'd be huge.

Status: Needs review » Needs work

The last submitted patch, 8: 2788777-8.patch, failed testing.

claudiu.cristea’s picture

Re #8: I wonder why we need so many config YAMLs just to test this. Probably we can stick to a minimum default configs.

claudiu.cristea’s picture

I think an improvement would be to rename the .info.yml config_sync

Hm, anyway, config/sync doesn't make much sense. Why not directly config/install? If a site is installed from the existing config stored in a profile, when a developer updates and export the config he will get diffs in config/install. Then he decides what will be committed. I don't see the reason of having different directories in the case when the site is installed from a profile containing the default config. Or I'm missing something.

andypost’s picture

Using langcode from existing config is great challenge!

Why not directly config/install?

because better prevent mix of signed/unsigned config, otherwise we will endup with a mess in profiles

PS: looks related https://github.com/previousnext/drush_cmi_tools

Eli-T’s picture

Fix coding standard issue (stray space between ][).
In install_begin_request(), don't set $install_state['parameters']['langcode'] only to immediately overwrite it.

bircher’s picture

Status: Needs work » Needs review
FileSize
133.52 KB
2.7 KB

Ok, so I spent most of Friday of the DCdublin sprint on manually testing this patch and I found some discrepancies.

First of all it only works when settings.php is relatively vanilla. For example it doesn't work when the $config_directories['sync'] is set to a different folder than the profiles config/sync folder, but I guess that is a limitation that when documented we can live with for now, and fix later.

On the other hand there is something going wrong with the configuration synchronisation.
I wrote a test to demonstrate it. The language negotiation for example has some quirks, and when manually testing it I also found that the translations get messed up.

I am setting it to needs review so that the test bot picks it up and puts it back to needs work.

Status: Needs review » Needs work

The last submitted patch, 14: 2788777-14.patch, failed testing.

Eli-T’s picture

Another issue with this approach is that the config sync directory then becomes predictable when an install profile was used. If this is obfuscated normally for security purposes, then why is it acceptable to deobfuscate it in this instance?

Eli-T’s picture

We definitely shouldn't have @see references to config_installer functions in core.

jribeiro’s picture

Great work guys!

I've tested the patch #6 with a simple custom profile, with no multi-languages, the only issue that I had was when I exported the "system.site.yml" without the UUID information, during the installation I received the Notices:

Notice: Undefined index: uuid in install_config_import_batch() (line 2225 of core/includes/install.core.inc).
install_config_import_batch(Array) (Line: 593)
install_run_task(Array, Array) (Line: 542)
install_run_tasks(Array) (Line: 119)
install_drupal(Object) (Line: 44)
Notice: Undefined index: uuid in Drupal\Core\Config\StorageComparer->validateSiteUuid() (line 395 of core/lib/Drupal/Core/Config/StorageComparer.php).
Drupal\Core\Config\StorageComparer->validateSiteUuid() (Line: 75)
Drupal\system\SystemConfigSubscriber->onConfigImporterValidateSiteUUID(Object, 'config.importer.validate', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('config.importer.validate', Object) (Line: 726)
Drupal\Core\Config\ConfigImporter->validate() (Line: 513)
Drupal\Core\Config\ConfigImporter->initialize() (Line: 2244)
install_config_import_batch(Array) (Line: 593)
install_run_task(Array, Array) (Line: 542)
install_run_tasks(Array) (Line: 119)
install_drupal(Object) (Line: 44)

Since the site UUID is a required config, should we validate and throw an exception during the installation if this information is not present?

jribeiro’s picture

If #18 make sense, follow the patch including the Site UUID validation.

dixon_’s picture

Status: Needs work » Needs review

The last submitted patch, 5: 2788777-5.patch, failed testing.

The last submitted patch, 17: allow_a_profile_to_be-2788777-17.patch, failed testing.

alexpott’s picture

the only issue that I had was when I exported the "system.site.yml" without the UUID information, during the installation I received the Notices:

So you hacked the system.site file to get in this situation. That said since we're depending on it this validation makes sense. Also we can't automatically populate it unless the installer can write back the changes to the sync config.

I had a discussion with @dixon_ recently where he was wondering whether or not the install profile sync config can come without UUIDs. If it does and we can't write back to it then the moment you've installed you'll not be able resync. If we want to support that then I think we should discuss that in a followup and not part of this issue.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

Rerolled and brought some more fixes over from the config_installer.

Status: Needs review » Needs work

The last submitted patch, 25: 2788777-25.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
1.47 KB
136.07 KB

The array syntax change is fun!

Status: Needs review » Needs work

The last submitted patch, 27: 2788777-26.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
137.33 KB
1.26 KB

Fixing one bug... stuff still fails though.

Status: Needs review » Needs work

The last submitted patch, 29: 2788777-29.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
6.66 KB
138.48 KB

Status: Needs review » Needs work

The last submitted patch, 31: 2788777-31.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
570 bytes
138.46 KB

Abstract test base classes are not supposed to run...

Status: Needs review » Needs work

The last submitted patch, 33: 2788777-33.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
jibran’s picture

Here is a minor review. The patch looks quite solid to me. It has tests now so going to remove the tag. I think we are moving https://www.drupal.org/project/config_installer to the core. Do we need some approvals here? @alexpott is already working on this so I think subsystem and framework reviews are not required. Maybe, he should comment to state his reasons. I think we do need a release manager review here but before that let's update the issue summary.

  1. +++ b/core/includes/install.core.inc
    @@ -445,10 +449,16 @@ function install_begin_request($class_loader, &$install_state) {
    +  if (isset($install_state['profile_info']['config_install']) && $install_state['profile_info']['config_install']) {
    

    Why do we need the second part of the condition? Can it be just !empty()?

  2. +++ b/core/includes/install.core.inc
    @@ -789,6 +799,25 @@ function install_tasks($install_state) {
    +      // @todo add a load of commentary about what is happening.
    

    Good idea. It took me while to understand the code the docs will help a lot here.

  3. +++ b/core/includes/install.core.inc
    @@ -1239,11 +1268,16 @@ function _install_select_profile(&$install_state) {
    +  if (!Settings::get('extension_discovery_scan_tests')) {
    

    Is there an existing issue for this? It seems like a bug. Do we need a test coverage for this?

  4. +++ b/core/includes/install.core.inc
    @@ -1239,11 +1268,16 @@ function _install_select_profile(&$install_state) {
    +      return !isset($profile_info['hidden']) || !$profile_info['hidden'];
    

    !(isset($profile_info['hidden']) && $profile_info['hidden']) Would be more efficient here, imo.

  5. +++ b/core/includes/install.core.inc
    @@ -2217,3 +2251,190 @@ function install_write_profile($install_state) {
    +    \Drupal::service('string_translation')
    ...
    +      'finished' => 'install_config_import_batch_finish',
    +      'title' => t('Synchronizing configuration'),
    +      'init_message' => t('Starting configuration synchronization.'),
    +      'progress_message' => t('Completed @current step of @total.'),
    +      'error_message' => t('Configuration synchronization has encountered an error.'),
    ...
    +    drupal_set_message(\Drupal::translation()->translate('The configuration synchronization failed validation.'));
    

    Let's create a local variable for translation service.

  6. +++ b/core/includes/install.core.inc
    @@ -2217,3 +2251,190 @@ function install_write_profile($install_state) {
    +      drupal_set_message(\Drupal::translation()->translate('The configuration was imported with errors.'), 'warning');
    ...
    +    $message = \Drupal::translation()
    

    This can also be converted to a local variable.

  7. +++ b/core/includes/install.core.inc
    @@ -2217,3 +2251,190 @@ function install_write_profile($install_state) {
    +      \Drupal::service('string_translation')
    ...
    +      drupal_set_message(\Drupal::translation()->translate('The configuration synchronization failed validation.'));
    

    Here as well.

  8. +++ b/core/includes/install.inc
    @@ -482,12 +483,20 @@ function _drupal_rewrite_settings_dump_one(\stdClass $variable, $prefix = '', $s
    +    // @todo Should the info key be config_install or config_sync? The directory
    +    //   can't be config/install because that would clash with module install.
    ...
    +      $config_directories[CONFIG_SYNC_DIRECTORY] = $install_state['profiles'][$profile]->getPath() . '/config/sync';
    

    config/sync seems alright. Can we remove the @todo now?

  9. +++ b/core/lib/Drupal/Core/Installer/Form/SelectProfileForm.php
    @@ -30,7 +31,7 @@ public function buildForm(array $form, FormStateInterface $form_state, $install_
    -      if ($details['hidden'] === TRUE && !drupal_valid_test_ua()) {
    +      if ($details['hidden'] === TRUE && !(drupal_valid_test_ua() || Settings::get('extension_discovery_scan_tests'))) {
    

    Once again this seems bug to me. We should move this to a separate issue and tests for this.

  10. +++ b/core/profiles/testing_config_install/config/sync/README.txt
    @@ -0,0 +1 @@
    \ No newline at end of file
    

    EOF missing.

alexpott’s picture

Thanks for the review @jibran.

  1. I think you are right - !empty() is fine
  2. Still @todo
  3. The test coverage here is the test coverage :) This is the type of thing that is really hard to test because the entire test system relies on it being TRUE
  4. Or even just return empty($profile_info['hidden']);
  5. Hmmm... that's probably not a good idea because of potx extraction. I'd rather just use t() everywhere and leave the t() in procedural code issues for other issues.
  6. Same as 5 - lets use t()
  7. Same as 5 - lets just use t() like alot of the rest of the installer
  8. That @todo is about the key in the profile's info.yml and whether it should be config_install: true or config_sync: true or even some_key: DIR. I don't think the last one works for what it is worth. I think it still worth getting opinions. At the moment the is using config_install: true.
  9. I don't know - this is the first issue that adds test profiles that need to be selected via the UI. Happy to make a blocking issue BUT adding a specific test for it when the tests added here will break the moment it break seems unnecessary.
  10. Well this file is added by the config system by the code. Not sure we should be updating it. And if the sync directory is writable we're just going to replace it anyway. See drupal_install_config_directories()

Also adding the needs tests tag back - yes we have a couple of perfect path test cases but we need to test config validation and what happens when there is an error. This might lead me to change the way the test install profiles are created and instead do that doesn't require fixing the profile selection page for test profiles.

bircher’s picture

8) I think some_key: DIR is not a bad idea, it gives the flexibility of defining the sync directory to be wherever one wants it to be. And since the limitation of this approach here is anyway that it works only with custom profiles I don't think this is too much of a stretch. The code to detect the directory is added here anyway.

9) I would not change the profile selection UI much and instead add the test profiles similar to how config_installer does. Ie adding the test profile on the fly, this would also allow us to have the configuration zipped.

When thinking about the limitation this approach has over the dance we have to do with "any" profile (ie a distribution) I think we could simplify it even more and instead just change the UUID on install, if you have the profile be the site you could even add the uuid to the profiles info file. (Or detect it from an existing sync directory wherever that may be). Then the site is installed the same way it is now and the config sync will work (provided you chose the same profile). Following this train of thought we could also just add a checkbox to the config sync that when checked removes all content entities and sets the UUID to the one in the sync directory and then runs a normal config import (limited to sites installed with the same profile as the one in the config to sync).

Otherwise we add a limited functionality to install existing sites that will give developers false hopes and the illusion sites will be able to be re-installed. This will be especially true if the profile inheritance becomes a reality (because it will fail horribly when you uninstall modules the base profile expects).

So one day we will have to face the fact that we allow installation profiles to expect modules to be installed in the hook_install but allow site builders to uninstall modules the profile depended on.
There are two options, re-write the core provided distributions (standard) and tell other distribution maintainers to be aware of that, or execute the profiles hook_install in a try catch block and allow it to fail.

I don't want to derail this issue and I would be happy to use this for our projects and use custom install profiles.
After all it is very easy for a developer to change which profile a site was installed with in core.extension and re-install the site after the configuration is exported.

I will give this a spin next week or so and give more feedback on the actual code.

dawehner’s picture

I tried out this patch yesterday with some custom profile.

It worked fine once I figured out that I have to get rid of the core.extension.yml file and put the list of modules into the $profile.info.yml.
I wonder what we could do about that?

alexpott’s picture

@dawehner that's strange - neither the tests added here nor a project I use this patch on need to do that. In fact adding dependencies to info.yml is exactly what not to do because profile dependencies aren't real dependencies.

alexpott’s picture

So I've implemented a new approach to testing - it creates a profile from a tarball - including the profile machine name so it is as simple as doing a config export and running tar -zcvf SOMETESTNAME.tar.gz * in the config export directory and placing that in a fixtures folder and writing a very simple test that extends \Drupal\system\Tests\Installer\InstallerExistingConfigTestBase(). This approach also makes it simple for contrib to use the same test functionality - just incase they do something funky on config import.

So I've managed to avoid fixing how we scan for profiles when $settings['extension_discovery_scan_tests'] = TRUE; - we probably should file another issue for that - but it is not related.

Plus I talked to @dawehner in real life and he didn't realise that he had to add config_install: true to his profile. The ensuing discussion resulted in the realisation that we can just detect the presence of a config/sync directory can go from there. No flag to add in the install profile :)

I think the next patch will move $install_state['profile_info']['config_install'] out of profile_info and make it a generic thing on install state.

alexpott’s picture

So now the install_state has two new properties that are copied from the profile if the profile has a config/sync directory. This will play much nicer with the work that @bircher is doing in #1613424: Allow a site to be installed from existing configuration - all that issue will have to do is set up the $install_state['config_install'] property (which is now a directory) and also read the system.site config into the $install_state['config'] property.

Status: Needs review » Needs work

The last submitted patch, 42: 2788777-42.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
4.7 KB
47.91 KB

Fixing patch.

Status: Needs review » Needs work

The last submitted patch, 44: 2788777-44.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
5.09 KB
47.89 KB

Whoops - inverted some logic by mistake.

alexpott’s picture

Issue summary: View changes
Kingdutch’s picture

Status: Needs review » Needs work

The change in 42 seems to have broken something. Specifically as the $config_directories[CONFIG_SYNC_DIRECTORY] is now set to the value of $install_state['config_insta''] this creates a discrepancy in the path used to load the initial system.site and what is later used for the configuration import.

In my case all configuration is in the profile's config/sync folder. This properly triggers the installer to use this folder for configuration and I can see it loads the system.site file properly. However, once it comes time to import the config it'll now use the value of $config_directories[CONFIG_SYNC_DIRECTORY] which is ../config/sync. That folder references to the root config folder (install is in /web) which is empty as no site has ever been installed before.

This is probably fixed by prefixing $config_directories[CONFIG_SYNC_DIRECTORY] with $config_directories[CONFIG_SYNC_DIRECTORY] = again like was done in the patch of #41 around:

+++ b/core/includes/install.inc
@@ -488,9 +488,9 @@ function drupal_install_config_directories() {

This prefixing with the profile path is actually properly done in install_profile_info

EDIT:
I'm unsure why the test didn't fail but my best guess would be that testing is being done with a non-empty config/sync directory?

A follow-up issue could be that if this install method is used the config/sync folder no longer needs to exist (which is the case now).

alexpott’s picture

@Kingdutch thanks for testing - I can see how that occurs we need to ensure that the config being imported from is in the profile and error earlier if not. Your use-case sounds like it will be fixed by #1613424: Allow a site to be installed from existing configuration - which will allow a site to be installed from directory outside of webroot that is already declared in settings.php. This issue is going to get the use-case of an install profile have a full config export working.

Kingdutch’s picture

Hi @alexpott, no problem! In my test I had actually moved the export from my website from config/sync (which is outside the web root) into web/profiles/myprofile/config/sync (which is inside the web root). The profiler seemed to error out because although it found the configuration in the install profile initially, when it came to actually importing the configuration it decided to look into the config/sync (outside of the web root) folder. Which of course was empty, causing the installation to crash.

I've also tried removing the config/sync (outside of web root) folder but then the installation won't start because it requires the folder needs to exist and/or be writable. That seems like it may be a good follow-up (no longer require a config/sync folder outside of the profile if you choose this installation path).

andypost’s picture

btw Having whole config/sync anywhere in web-root accessible from outside is a big question for me.

Guess changing docs and .htaccess also needed

@alexpott The issue looks a duplicate of #1613424: Allow a site to be installed from existing configuration with a difference that no way here to bypass a directory with config snapshot

dawehner’s picture

Once someone actually reads the manual this patch works great for me.

  1. +++ b/core/includes/install.core.inc
    @@ -445,10 +449,16 @@ function install_begin_request($class_loader, &$install_state) {
    +  if (!empty($install_state['config_install'])) {
    +    $install_state['parameters']['langcode'] = $install_state['config']['system.site']['default_langcode'];
    

    Should we check whether this value is actually set?

  2. +++ b/core/includes/install.core.inc
    @@ -2217,3 +2253,189 @@ function install_write_profile($install_state) {
    +  if ($success) {
    +    if (!empty($results['errors'])) {
    ...
    +  }
    +  else {
    +    // An error occurred.
    

    This is quite confusing ... how can $success be true, if there are errors? Oh I see this is about batch errors vs. config errors, maybe a comment would be helpful.

jibran’s picture

As per #47.

jibran’s picture

+++ b/core/includes/install.inc
@@ -1090,6 +1100,12 @@ function install_profile_info($profile, $langcode = 'en') {
+    // If the profile has a config/sync directory use that to install drupal.
+    if (is_dir($profile_path . '/config/sync')) {
+      $info['config_install'] = $profile_path . '/config/sync';
+      $sync = new FileStorage($profile_path . '/config/sync');

Can we allow profiles to nominate this directory in info.yml file? Default can stat '/config/sync'.

dawehner’s picture

Can we allow profiles to nominate this directory in info.yml file? Default can stat '/config/sync'.

Is there a usecase in placing the config somewhere else? Otherwise it feels a bit like an overarchitecture ...

jibran’s picture

I'm planning to use this patch with #2793445: Allow BTB to test an existing, already installed Drupal site instead of installing from scratch. on D8 site. My current workflow to run the tests is

drush -r webroot sql-sync @dev @self -y
drush -r webroot updb -y
drush -r webroot entity-updates -y
drush -r webroot cimy -y --skip-modules=module1,module2 --source=config-export --install=config-install --delete-list=config-delete.yml
./bin/phpunit

If I can nominate sync dir as config-export dir then after this patch it would be

drush -r webroot si my_profile -y
./bin/phpunit

Right now, I can do that with cd webroot/profiles/custom/my_profile/config;ln -s ../../../../../config-export sync but it'd be nice to nominate the directory.

Note: cimy command can be found at https://github.com/previousnext/drush_cmi_tools

Kingdutch’s picture

It turns out my problems were caused by a comment in #14 that I missed.

First of all it only works when settings.php is relatively vanilla. For example it doesn't work when the $config_directories['sync'] is set to a different folder than the profiles config/sync folder, but I guess that is a limitation that when documented we can live with for now, and fix later.

Setting $config_directories[CONFIG_SYNC_DIRECTORY] = $app_root . '/profiles/myprofile/config/sync'; made the patch work like a charm.

It sounds like it should be possible to just not set this variable since we can't be sure where the install profile is located? (Although the user controls settings.php and/or the profile location). If it is on purpose that this should always point to the profile's sync folder then I don't understand all the logic in the patch for figuring out the config/sync folder's location relative to the install profile.

alexpott’s picture

Issue summary: View changes

@Eli-T and I worked on a Drush command to create an install profile for an existing site that works with this patch - see https://www.drupal.org/project/install_profile_generator

mkalkbrenner’s picture

We're very interested in this feature. But for us, #2792877: Symfony YAML parser fails on some strings when running PHP 7 blocks this feature :-(

alexpott’s picture

Status: Needs work » Needs review
FileSize
6.89 KB
50.89 KB

@jibran and @Kingdutch we are explicitly reducing the scope of this change to only deal with install profiles containing configuration. Doing anything else is the scope of #1613424: Allow a site to be installed from existing configuration. The point of the issue is to solve a very specific use-case and get the plumbing for configuration sync during import in place. This is already outlined in the issue summary. If it is not clear enough feel free to update it.

Patch attached adds non-happy-path testing to ensure that configuration is validated and the user receives the expected messages at the right moment and ensure that the user can't weirdly continue installation.

jibran’s picture

@alexpott thanks, sounds like a good plan.

alexpott’s picture

I think we need a change record and big issue summary update.

bircher’s picture

Status: Needs review » Needs work
Issue tags: -Needs change record, -Needs issue summary update

So I tested it a couple of times and there is a slight complication if the settings.php contains the database settings but not the correctly set up $config_directories it will fail. Not a problem, just noting it here. It only works for its narrow use case, other ones can be covered in next steps.
This functionality is purely opt-in for custom profiles.

On the code level the patch uses: $install_state['config_install'] and $install_state['config'] but they are not documented with the default values in install_state_defaults(). Also there is still the todo for documenting. But other than that I think it is really close.

DamienMcKenna’s picture

Should this include some extra documentation in default.settings.php?

bircher’s picture

Status: Needs work » Needs review
Issue tags: +Needs change record, +Needs issue summary update
FileSize
51.72 KB
1.37 KB

Ok, So I added the documentation and now it should be good to go.
I don't think we need to change the documentation in default.settings.php but rather in the change notice.

This feature we add here is strictly limited to profiles that have their configuration in its own config/sync folder. (other locations in follow ups.) So the documentation needed for this is rather how to use this feature in the first place.

PS: I didn't mean to remove the tags..
PPS: I don't know why the patch is so much smaller I guess there is something going on with the fixtures. Lets see what the testbot has to say about it.

andypost’s picture

Btw having config in profile it makes it exposed to docroot, it needs htaccess changes & mention in change record

vpeltot’s picture

The patch #65 works fine :

  • Patch applied on Drupal core 8.3.3
  • drush site-install command
  • multilingual
  • config entities translations (configuration located in languages subfolder)

Everything is OK!

Do you think apply this patch on version 8.3.x?

GoZ’s picture

Status: Needs review » Needs work
+++ b/core/includes/install.core.inc
@@ -1464,6 +1501,12 @@ function install_load_profile(&$install_state) {
+    $install_state['config'] = $install_state['profile_info']['config'];

We should test $install_state['profile_info']['config'] is not empty.

In case no 'config' is defined in profile info, installation works but a notice is displayed in browser:
Notice: Undefined index: config in install_load_profile() (line 1508 of core/includes/install.core.inc).

GoZ’s picture

Status: Needs work » Needs review
FileSize
53.43 KB
665 bytes
mpotter’s picture

Tested this a bit and it seemed cool at first. Until I realized that this doesn't make sense for "distribution" profiles where the config/sync for the profile is committed as part of the profile itself, because every site you install using this profile would have the SAME SITE UUID!

Doesn't that defeat the purpose of the site uuid? I mean, it makes sense for the dev/stage/prod environments of a site to have the same uuid, but when building a distro like Lightning, or Atrium where I expect lots of different sites to install using the profile, you wouldn't want them to have the same uuid. Am I missing something here? Maybe my use-case for profiles is different from what this patch addresses.

skyredwang’s picture

I was wondering if we could do this:

1. do a drush site-install with minimal profile
2. get the new site UUID
3. get the path of the existing config files either from settings.php or from command line arguments
4. replace the existing site UUID with the new site UUID, using this command grep -rl 'old-site-uuid' /PATH/TO/CONFIG | xargs sed -i 's/old-site-uuid/new-site-uuid/g'
5. import the existing configs, using drush config-import -y

If this workflow works, then we can update "drush site-install" to automate this workflow.

I haven't tested the workflow above. But, this would solve #70