Problem/Motivation

Using a fully preconfigured settings.php with a config_sync_directory setting and a profile with a config/sync directory is broken.

Steps to reproduce

This is actually hard to reproduce at the moment due to the following bug https://www.drupal.org/project/drupal/issues/3247553

1. Before installing configure a settings.php with hash_salt, databases and config_sync_directory already set up.
2. During installation select a profile with a config/sync directory

Kaboom - you get an error.

Proposed resolution

To be decided... the problem is the user has expressed two conflicting intents for the location of the config/sync directory.

Option one

If $settings['config_sync_directory'] directory is empty copy the profile's config/sync directory there and continue with the install. If the directory is not empty then display en error to the user.

Option two

Trigger an error that's display on the installer requirements page that explains that choosing a profile that installs from configuration and having a configuration sync already defined in settings.php is an error. The error should list the following ways to resolve the error:

  • edit settings.php and set the setting to the profile's config/sync directory
  • remove the setting from settings.php
  • choose another profile

Remaining tasks

Decide on fix.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 copy_profile_config_before_install_if_config_dir_defined-3247561-11.patch1.76 KBrealityloop
#10 copy_profile_config_before_install_if_config_dir_defined-3247561-10.patch1.76 KBrealityloop

Issue fork drupal-3247561

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

realityloop created an issue. See original summary.

realityloop’s picture

realityloop commented
Issue summary: View changes
realityloop’s picture

realityloop commented
Issue summary: View changes
realityloop’s picture

realityloop commented
Title: Improve handling of config only install profile configuration » Improve handling of config only install profile configuration at installation time
realityloop’s picture

realityloop commented
Issue summary: View changes
realityloop’s picture

realityloop commented
Related issues: +#3247553: $install_state['settings_verified'] incorrectly set when config_sync_directory is missing
realityloop’s picture

realityloop commented

Additionally it's currently impossible to install a config only profile if you have already set the config_sync_directory before installation.

This is pre-defined in ddev for instance.

The proposal in this issue would resolve that.

realityloop’s picture

realityloop commented
Related issues: +#2980670: Install a site from config if the config directory is set in settings.php
realityloop’s picture

realityloop commented
Related issues: +#2982052: Allow an install hook in profiles to install from configuration
realityloop’s picture

realityloop commented
Status: Active » Needs review
StatusFileSize
new copy_profile_config_before_install_if_config_dir_defined-3247561-10.patch1.76 KB

I'm sure this isn't the best way to do it, but it does resolve this issue.

realityloop’s picture

realityloop commented
StatusFileSize
new copy_profile_config_before_install_if_config_dir_defined-3247561-11.patch1.76 KB

Fixed a typo in patch (comma that should have been period in comment)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@realityloop we cannot make this change like this. This satisfies the way you want it to work but not others. We cannot change the default behaviour here.

I think we need to go back to first principles and ask whether an install profile is really what you need here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I think given that what you want to be able to provide is a starting set of configuration and then no longer care about it we should not use install profiles at all. If install profiles are used you will aways have two full sets of config lying around being confusing for no reason.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Also is what is happening so wrong? I don't think so. I think you might even be able to use install profiles / composer to make everything behave the way you want. But the install profile would not be drupal.org. The install profile could live as part of a composer project and therefore is only instantiated when someone runs composer create-project.

Here's how I think it would work:

$ composer create-project my-suitably-named-project

Your project would create the install profile with a config/sync directory... you could even encourage people to name this something to do with what they are building. And it would just work™

realityloop’s picture

realityloop commented

@alexpott

#12: I didn''t believe I had changed the default behaviour? My patch only does something different if $settings['config_sync_directory'] is already defined in settings.php before you install the site.

#13: With the active sync directory configuration being written to settings.php on install, I personally struggle to see how this is any more confusing than having the code from an upstream contrib profile changing in your local project as you manage the site and export config.

With a workflow where you are using composer to build your codebase and git ignoring all contrib dependencies the existing behavior could either:

  • Leave you with ignored config changes
  • have your config changes overwritten every time you build the project
  • cause you to accidentally import and old version of configuration and break your site after a build

A cardinal rule in our development has for the longest time has been no code changes in core or contrib that aren't enacted via a patch in the issue queue at drupal.org and applied at build time. This has always been to guard against undocumented changes.

When using a profile containing a config/sync directory to package settings, the current behavior makes undocumented changes to the content of that contrib dependency in your project. To me at least this seems very undesirable.

#14: Yes, it could be done this way, but drupal.org has no way to host these afaik? Therefore we get no usage tracking of active installs.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

This will break config install from profiles on envs like ddev which try to set config_sync_directory incorrectly.

Don't put the profile in profiles/contrib put it in profiles/ and encourage people to make it theirs.

realityloop’s picture

realityloop commented

@alexpott I'm not sure how you believe this would break ddev install?

I tested it, and here is a video showing that after applying your patch in https://www.drupal.org/project/drupal/issues/3247553 installation of a config only profile will still fail anywhere that $settings['config_sync_directory'] is already defined unless my patch here is also applied.

I'd believe that if this has already been defined it means that the developer want's it to be stored in the defined directory, and would not expect it's definition to cause an install failure.

https://youtu.be/fiQwSV5CtkQ

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@realityloop a good question posed by this issue is exactly what should happen if you install a profile with a config/sync directory and the sync directory is already set in settings.php. As this issue makes clear ATM this is broken. I think we need to add an error to the installer if the config sync directories mismatch as the user has indicated two very different wishes - they're using a profile which has a config/sync directory and they set config_sync_directory to something else in settings.php.

As I've pointed out already in this issue I think the solution to the issue of using composer to grab a set of config and install from that could have a solution that does not involve install profiles and I think that we should do that to provide the feature that's requested in the issue summary.

realityloop’s picture

realityloop commented

@alexpott I’ve already refactored Foundry to package the config with the composer project.

But have left the issue open because I agree the current handling is broken.

One option would be to confirm if the configured sync directory is empty, if not copy the configured as I did with my patch.. if it isn’t show error as you suggested.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Improve handling of config only install profile configuration at installation time » Using a profile that has a config/sync directory and already have the config_sync_directory configured in settings.php is broken
Category: Feature request » Bug report
Issue summary: View changes
Status: Needs review » Needs work

@alexpott I’ve already refactored Foundry to package the config with the composer project.

I'd love to see a write of this approach somewhere. v. interested to see how we can streamline this.

I've re-titled the issue to be a bit more buggy and rewritten the issue summary to focus on the bug.

I think we need to go for option 2 because:

  • if a profile has a config/sync directory then if the sync directory ends up set to something else then we need to consider than an error
  • I think we'll get into a catch 22 when re-installing from configuration because the profile's config/sync will not empty and neither will the site's but the site's will contain the changes you want.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.