Non-required config gets marked as Conflict

I think this comes back to my disagreement with the default weights of the assignment plugins. I believe the "Existing: Add exported config to existing packages." plugin should be much earlier in the list (right after the Packages and Exclude).

Can somebody remind me why the Existing plugin is added last?

Because, yes, this *does* result in very confusing UI results. And I'm still not seeing the normal use case for reassigning config out of its current package export.

Yes, if you are playing around with different bundle settings to change how config is being packaged, then you can go ahead and move Existing to the end. But in the 90%+ use cases you don't want Features to reassign config that is already exported to an existing Feature.

I *really* don't want the "Required" data in the features.yml file to be necessary. That's back to what we had in Features D7. In D8 it should use the config yml files in the config/install directory to represent the config that is part of the Feature by default. Only exceptions should be placed in the features.yml file.

We need to resolve this difference before we roll a Beta version. Having Features work smoothly for non-developer users who just want to organize their config into packages is the #1 use case for this module in my opinion.

Note that moving the "Existing" plugin higher in the list does fix this issue. So as a patch, here is a change to the default bundle config weights.

since it requires manually deleting required information from the features yml file.

Well, honestly I'd like to get rid of all that "required" stuff. Again, I think the features.yml file should only contain exception information and not information required for Features to work. We want Features to work if we just take a normal module that contains config and then add "true" to the features.yml file for it. I don't want the list of config in the config/install directory needed to be added to the features.yml file.

Basically, I want Features to work well when people are manually manipulating their config, which is something they do when they aren't using the Features UI. I want Features to just be a way to organize config into modules and not get in the way of other methods for organizing that config manually.

As you say, the problem only exists if you move plugins around. But there are likely all sorts of issues that arise if you move plugins around.

(Also, FYI this issue has come up already on 3 separate projects because config is being manipulated via CMI and not necessarily always with Features)

Add #2691625: Add option to mark a feature as "Required: All" related issue that would address the Required stuff.

Then will still need to see if that fixes this issue with conflicts.

#2691625: Add option to mark a feature as "Required: All" helps with this. But it requires editing old packages to add the "required: true"

I think this might also need to be done with config that is within "Installed" modules. Right now I have a module called "cu_article" that contains the node.article content type, with no "required" in the info file. Features is creating a new auto-detected feature called just "article" and placing node.article within it as a non-exported feature even though cu_article is enabled on the site.

Doesn't seem like it should ever override enabled modules like this.

Here are more specifics:

1) content type is "homepage". currently exported to a "research_homepage" feature module which IS ENABLED
2) Base Type plugin is assigning the "homepage" content type to a new "homepage" feature.
3) Existing plugin runs last and cannot reassign the config because it's already in a package.

If required: true or required: homepage is set then the Packages plugin will assign the content type to it's current research_homepage module.
If Existing plugin runs earlier then research_homepage keeps it's config
If I add "bundle: research" then it keeps the content type within research_homepage but creates a bundle I don't want (research isn't a bundle)

Yes, this is a case of a poorly named content type and poorly named feature module. But that's the kind of stuff we get to inherit and work with sometimes.

PROPOSAL:

Modify the Packages plugin to treat INSTALLED MODULES as if all their config was required. We don't want to be changing installed modules, right? Any messing around with auto-assignment via plugins should be done on uninstalled or not-exported packages.

Or, nedjo if you disagree with this because you want to play with reassigning config across installed modules, then let's make that an option you can enable, but not the default. I think moving config out of enabled modules is really asking for trouble.

Also, we need to have a global Features settings page anyway at some point to allow editing the export folder.

Here is a patch that assigns config to installed modules.

I think I found the problem.

1) "exclude" plugin runs. Marks config provided by modules as "isExtensionProvided"
2) "base" runs and skips the content marked as "isExtensionProvided"
3) "existing" runs and STILL skips the "isExtensionProvided" because in assignConfigPackage we have this code:

        $extension_provided = ($config_collection[$item_name]->isExtensionProvided() === TRUE);
        $already_assigned = !empty($config_collection[$item_name]->getPackage());
        // If this is the profile package, we can reassign extension-provided configuration.
        $is_profile_package = $this->getAssigner()->getBundle($package->getBundle())->isProfilePackage($package->getMachineName());
        // An item is assignable if:
        // - it is not extension provided or this is the profile package, and
        // - it is not flagged as excluded.
        $assignable = (!$extension_provided || $is_profile_package) && !$config_collection[$item_name]->isExcluded();
        $excluded_from_package = in_array($package_name, $config_collection[$item_name]->getPackageExcluded());
        $already_in_package = in_array($item_name, $package->getConfig());
        if (($force || (!$already_assigned && $assignable && !$excluded_from_package)) && !$already_in_package) {

notice that $assignable only looks at $extension_provided config if is_profile_package is set.

Hrm, the "exclude" plugin is *not* marking it as isExtensionProvided, although even once that is fixed I think the issue in #14

Looks like the 'namespace' option in the "exclude" plugin detects the config as belonging to the "default" namespace and is thus not excluded.

I'll take a fresh look at this next week. The logic behind all of this is making my brain hurt ;)

OK, here is a new patch to review. It does a couple things:

1) Adds a new "extensionEnabled()" helper function to FeaturesManager

2) Fixes the Exclude plugin so when applying the namespace filter it only excludes non-enabled modules matching the namespace.

3) Fixes the Existing assignment plugin so that config marked as isExtensionProvided still gets force-assigned to the original package.

With this in place and looking at #13, now the config from the installed cu_article module gets excluded (marked as ExtensionProvided) and then the final Existing plugin properly allows this ExtensionProvided config to be assigned to its original package.

Here is an updated patch based on IRC discussions. It cleans up some of the code to make it more understandable. It also reverts the previous patch changes to the Existing plugin in favor of changing the FeaturesManager::assignConfigPackage() to allow config already assigned to a module be assigned to that module, alleviating the need for the "force" option in the Existing plugin.

This also cleaned up a lot of problems with Conflicts showing up if you marked an entire "type" of config as excluded. For example, disable "Fields" and see the difference with this patch.

TODO: Still want to make the check in the Exclude plugin for non-installed modules in namespaces have an option to support nedjo's usecase (allowing config to get reassigned when it is already installed)

Edited: Also reapplied this after the refactoring changes from #2655286: Rename extensionProvided and providingFeature properties of ConfigurationItem

Oops, here is the patch

OK, here is the "final" version that adds a new "namespace_any" option that can be used to allow reassignment of any namespace config regardless of it's installed state. Leaving this turned off by default since most people will not want config from installed modules to be reassigned.

OK, lets try this patch

There we go. OK, going to commit this so we can move forward with the beta. We can open new issues for any tweaking that might be needed in the future. But this seems to solve all my problems and still supports nedjo's use case.