Problem/Motivation

As of #2140511: Configuration file name collisions silently ignored for default configuration, ModuleInstaller::install throws a PreExistingConfigException exception when a module is installed that provides configuration that already exists on the site.

This change presents a problem for packages because the usual workflow is:

  1. Configure site.
  2. Create packages.
  3. Enable packages on site.
  4. Continue to edit packages.

This workflow will break on step 3, since the configuration already exists on the site.

Proposed resolution

Possibly, break package creation into two steps. First, generate an empty package. Then, once enabled, add configuration.

Remaining tasks

User interface changes

API changes

Comments

mpotter’s picture

mpotter CreditAttribution: mpotter commented

This definitely seems like a blocking issue. Right now I can create packages and put them into my profile. But the package modules don't/can't get enabled on my dev site because of the core issue raised above.

The core issue was closed and instead they just added error messages telling users why a module wasn't enabled. The fact that there doesn't seem to be a way to enable the module anyway and just ignore the config conflict seems annoying.

I will try the proposed resolution, but it seems that only helps with the initial problem. What happens when we want to move a "package" to another site and install it outside of an install profile?

Given we have the config_update module to potentially reload the config from the module, we really need a way to install a module with config outside of config_packager somehow. In the above issue they refer to the exception of allowing profiles to override existing config, so maybe we need to find out how this exception is implemented?

mpotter’s picture

mpotter CreditAttribution: mpotter commented

I wonder if we should add a way to handle "feature enable/disable" that is different from "module enable/disable". Since you can't enable a module that contains config already in the active store, Features could add drush commands and a UI around enabling/disabling packages. Then we could potentially fix this by:

1) Find all conflicting config and move to a separate folder
2) Install the module
3) Move the conflicting config back into config directory
4) Optionally run config_update to "revert" the config

I hate playing these kind of games with folder names, but unless D8 core does something to get around the exception error I'm not sure what else to do.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented

Likely the way to handle this is something similar to what I've done in Configuration Share:

mpotter’s picture

mpotter CreditAttribution: mpotter commented

I think we will be able to identify "features-type modules" because I was going to change the info.yml file from 'config_devel' to 'features' since we don't actually want config_devel to manage this config. So I'll already have a method to return a list of "features modules".

I'll take a look at the custom config service. That seems like a cleaner route. I think it's ok for this installer service to be in config_packager (Features) since it only is an issue when initially creating the package.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Project: Configuration Packager » Features
Version: 8.x-1.x-dev » 8.x-3.x-dev
mpotter’s picture

mpotter CreditAttribution: mpotter commented
Status: Active » Needs review

Did something similar to #3 but found the findPreExistingConfiguration() function that could also be easily overridden. We grab the module's info.yml file and check for the "features" key to allow a feature package to be installed.

Committed to 950c9cc

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented

Sounds good. I'm not seeing the commit. Maybe not pushed yet?

  • mpotter committed 950c9cc on 8.x-3.x 
    Issue #2418961: Packages can't be enabled on originating site
mpotter’s picture

mpotter CreditAttribution: mpotter commented

Not sure what took it so long to appear, but there it is: http://cgit.drupalcode.org/features/commit/?id=950c9cc

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented

Nice!

Minor nits:

  // allow a Features package to be enabled even if it's config conflicts

Code comments should be full sentences.

+ * Contains Drupal\config_share\ConfigShareConfigInstaller.

Leftover copied from config_share. Ditto in FeaturesServiceProvider.php.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented 
+ $module_path = drupal_get_path('module', $name);
+ $info_file_uri = "$module_path/$name.info." . FileStorage::getFileExtension();
+ $info = \Drupal::service('info_parser')->parse($info_file_uri);
+ if (isset($info['features'])) {
+ // allow a Features package to be enabled even if it's config conflicts
+ return array();
+ }

Since a similar test is appearing in multiple places, it should be pulled into a helper public method, probably in FeaturesManagerInterface.

public method isFeatureModule($name);

  • mpotter committed 09c596a on 8.x-3.x 
    Issue #2418961: Some code cleanup and refactoring
mpotter’s picture

mpotter CreditAttribution: mpotter commented

Made the changes from #11 along with some other refactoring to make this even cleaner. Thanks for the review and let me know what else you find.

Committed in 09c596a.

mpotter’s picture

mpotter CreditAttribution: mpotter commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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