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. This would be facilitated by allowing an install profile to declare config_install: true in its info.yml file.

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.

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.