Recognize existing modules in the curent feature set

Ahh, ok. So if we modify the info.yml file to have a section called "features" instead of the current "config_devel", would that satisfy this issue? Then have a function that returns a list of "features modules".

You are correct. This would be bad because then a feature wouldn't work without the Features module. My bad...I just *knew* it was too easy to be a good idea ;)

The problem with "sets" right now is that the exported module doesn't identify what the namespace was. The namespace is prepended to the module name, but there isn't an obvious way to unintangle this, and we don't want to be tied to the module name. I think we need to store the set namespace in the info.yml file.

If we are working with a single set at a time, does a single set span the full site config? In other words, could packages in apple and banana export the same config (like the same view). Or is it the full collection of sets that represents the full site config?

You are correct that the info.yml probably doesn't need to list the config.

I think I am still wrapping my head around some of the assignment plugins like exclude and namespace.

I will post my AssignmentExisting plugin during the migration project so that it's a clean commit to be reviewed.

One thing that I did need to add to the current "package" project object was a "status" so we could tell in a "drush features-list" which packages have been exported to code (and whether the modules are enabled/disabled) vs the autoassigned packages that have not yet been exported. Right now I have a "not exported", "disabled", "enabled" status.

One problem I'm running into here...

I am trying to figure out what config is stored in an exported (but not installed) package module. It looks like the configInstaller class has a findPreExistingConfiguration() function. It *mostly* works, but does not return any of the "core.*" exports.

For example, here is the exported article package:

core.entity_form_display.node.article.default.yml 
core.entity_view_display.node.article.default.yml  
core.entity_view_display.node.article.rss.yml      
core.entity_view_display.node.article.teaser.yml   
field.field.node.article.body.yml                  
field.field.node.article.comment.yml
field.field.node.article.field_image.yml
field.field.node.article.field_tags.yml
node.type.article.yml
rdf.mapping.node.article.yml

but findPreExistingConfiguration('module','test_article') returns:

    [] => Array
        (
            [0] => field.field.node.article.body
            [1] => field.field.node.article.comment
            [2] => field.field.node.article.field_image
            [3] => field.field.node.article.field_tags
            [4] => node.type.article
            [5] => rdf.mapping.node.article
        )

Oh nevermind, the findPreExistingConfiguration() isn't for what I thought it was for. It basically shows the conflicting config already in the store. Will keep looking for the right way to list the config within the subdir of an uninstalled module.

First attempt to detect existing packages is in Features 8.x here: http://cgit.drupalcode.org/features/commit/?id=6071eae

I think it needs work around the Profile namespace.

The ExtensionInstallStorage::getComponentNames(). in #8 works well, so I actually refactored the listExtensionConfig() to use it instead of the code in #9. Seems cleaner and is the same kind of solution used in config_update.

There might be a couple of other refactors mixed in here. I think I needed a getFullName()function and didn't want to duplicate code. Again, looks like config_update has the same code for this.

Even though I didn't post a patch and posted a commit record instead, I'm going to mark this as Needs Review just so we know which tickets have code to be reviewed.

Oh, also need this change:

http://cgit.drupalcode.org/features/commit/?id=949cdb1

which adds the "features" key to the exported module info file. We can decide later if we want to remove the config_devel list.

The problem I am running into now is that after exporting I get a features-list like this:

$ drush fl
 Name                        Machine name              Status        Version  State
 D8 Test D8 Test Article     d8_test_d8_test_article   Not exported
 D8 Test D8 Test Core        d8_test_d8_test_core      Not exported
 D8 Test D8 Test Image       d8_test_d8_test_my_image  Not exported
 D8 Test D8 Test Basic Page  d8_test_d8_test_page      Not exported
 D8 Test Article             d8_test_article           Disabled
 D8 Test Image               d8_test_my_image          Disabled
 D8 Test Basic Page          d8_test_page              Disabled
 D8 Test Core                d8_test_core              Disabled

Here, I used d8_test as the "profile name". It seems to be adding the profile name twice to existing modules.

Here is the change to prevent auto-assignment for config that is already assigned to a package.

http://cgit.drupalcode.org/features/commit/?id=afb3a01

Now the output of drush fl is:

$ drush fl
 Name                        Machine name              Status        Version  State
 D8 Test D8 Test Article     d8_test_d8_test_article   Not exported
 D8 Test D8 Test Core        d8_test_d8_test_core      Not exported
 D8 Test D8 Test Image       d8_test_d8_test_my_image  Not exported
 D8 Test D8 Test Basic Page  d8_test_d8_test_page      Not exported
 D8 Test Basic Page          d8_test_page              Disabled

In the above case I removed a couple files from the d8_test_page config/install directory to simulate a "partial features" where something was excluded. It correctly identifies that and puts those removed items into the auto-detected d8_test_page package.

So in the above, the first 4 are actually the existing packages detected in FeaturesAssignmentExisting. But since they get the profile name added to the front, they are not detected as modules when getting the status, so the status shows as "not exported". The last package is actually the auto-detected package from FeaturesAssignmentBaseType but since the profile name is not duplicated, it actually matches the exported module and shows as a disabled module.

Once we get the proper machine names sorted out the Status field should fall into line too.

I'm still confused by #14. Maybe this is related to #2446805: Add "required" components for a given feature?

If I specifically place a config into a feature, then I want it to remain in that feature and not be reassigned.

The way config_packager did the auto-assignment was interesting, but in real use cases I'm not so sure about it. In Panopoly and Open Atrium, the config part of a Feature is often tied to additional code with the module itself. For example, the oa_discussion App has content type, field, views, etc as config, but also has code for handling various alter functions and messages related to discussions.

In that normal Features use-case, you never want Features to accidentally auto-assign your config to some other module just because the Assignment options have changed.

Typically we have different developers working on different "Features" even within the same "set" (like within OA). So you are typically only importing/updating/reverting that single Feature you are working on and not the others being done by other developers.

So I'm worried the concept of re-generating "packages" all the time based on the assignment settings is going to cause confusion and potential problems since it requires all of the features to be re-exported all of the time.

Probably need a separate issue to discuss the philosophy of this. In the meantime, I will separate the AssignmentExisting as recommended and think about how to "lock" config settings into a specific Feature.

OK good, that makes sense. So to confirm, this issue should just create the packages but not assign the config. Sort of like:

  public function assignPackages() {
    $existing = $this->featuresManager->getExistingPackages();

    foreach ($existing as $name => $info) {
      $this->featuresManager->initPackage($name, $info['name'], !empty($info['description']) ? $info['description'] : '');
    }
  }

and then #2401589: "Existing configuration" assignment plugin would actually do the assignment:

  public function assignPackages() {
    $existing = $this->featuresManager->getExistingPackages();

    foreach ($existing as $name => $info) {
      $config = $this->featuresManager->listExtensionConfig($info['module']);
      $this->featuresManager->assignConfigPackage($name, $config);
    }
  }

The first assignment would have a really low weight to run first, but the last one would have a high weight to run last. Then #2446805: Add "required" components for a given feature would be a new assigner that would include/exclude config as needed that would have a weight to run before the other auto assignments.

If that is correct then I can refactor this into the two bits above and then start working on #2446805 next.

A couple of commits to review in #21 above (Ignore #20, that was a derp on my part). Also fixed a regression where the getEnabledAssigners() method wasn't returning the list of methods sorted by weight.

Probably need some additional refactoring here. I dislike how the method weight stuff is handled outside of the FeaturesAssigner manager.

OK, so still a bit confused here.

With the above refactoring, after I have exported all the packages, I still have duplicates like "d8_test_page" and just "page", where "d8_test" was my export namespace.

When I do a "features-list", the "d8_test_page" package is empty and all the config from it is in the "page" package. This is because the base assignment runs first, which reassigns all the config that was exported back to it's auto-detected packages, rather than the existing packages.

So I'm confused on how any already exported config still ends up being assigned to the correct package.

The problem here is that when using Drush, it doesn't know what feature set we are working on. Drush needs to be able to work with any feature set. This I have #2453111: Feature "sets" and namespacing talking about how to handle that.

But when I'm using Drush to manage a site that has multiple "sets", such as oa_* and panopoly_* sets, I do want Drush to show all of those and let me manipulate them unless I specify a certain namespace via the "--name" argument. So "drush fl" shows me *all* the packages, whereas "drush fl --name=panopoly" only shows me the panopoly set of packages.

I think the difficulty here is we are coming at this from opposite ends...I see the Namespace and "set" as an Export property. When the config is exported, it needs to store what it's "namespace" was when exported so that same namespace can be recreated during the assignment process.

For example, if I change the current namespace from "oa" to "panopoly" and then export/update the panopoly features, the current profile_name is now "panopoly" and yet a "drush fl" still needs to understand the existing exports from the "oa" set.

I think the following code in the AssignmentPackages (the one that runs first) helps address all of this:

  public function assignPackages() {
    $existing = $this->featuresManager->getExistingPackages();
    foreach ($existing as $name => $info) {
      // machine_name_short was stored in the "features" key in the info file
      $this->featuresManager->initPackage($info['features'], $info['name'], !empty($info['description']) ? $info['description'] : '');
      // set the *actual* full machine name from the module
      $packages = $this->featuresManager->getPackages();
      $packages[$info['features']]['machine_name'] = $name;
      $this->featuresManager->setPackages($packages);
    }
  }

So here we are creating a package from an existing module. We store the machine_name_short in the "features" key of the module's info file. We create the package using this short name, but then we update the full machine_name to be the name of the module itself.

Does that solution make sense?

Also, related to the name-change of this issue and the edit of the original issue:

You don't just want to detect packages within the current "set". The core idea (as I understand it) is that we are representing *all* of the site config across all sets at any given time. The UI might only show packages within a given "set", but the Features Manager needs to have the full list of packages so that we'll be able to move config from one place to another.

My understanding is that you wouldn't have the same config in two different sets. Is that where I am wrong? Could you really have a "oa_article" AND "panopoly_article" Feature/package on a site that contains the same config? That seems contrary to my understanding.

Instead, I see "sets" as more of a grouping of related features. Such as "oa_article" but "panopoly_page". And in that case I would use the namespace to control how a package is exported. So I could export "oa_article" with the "panopoly" namespace and it would change oa_article to panopoly_article.

Can you confirm that I've got this right or not?

No, I'm still stuck. Because I'm working with Drush first and not the UI. Drush doesn't know about what namespace the package was exported.

Think of it this way:
1) I export "article" on Site A using a namespace of "oa". I get a module called "oa_article" as expected.
2) I copy this module over to Site B. I haven't defined any "namespace" on Site B.
3) I run "drush fl" to get a list of Features. It should show "oa_article" and have it as a short_name of "article".

In order to do this we need to store the short_name in the info.yml file of the module. Either that or store the namespace itself, but the code is actually easier if we store the short_name and compute the namespace as the difference between the full module name and the short name.

I am not suggesting making major changes to how short_name and full_names are done. As you said, we can refactor that later as needed. But I need to get Drush working with multiple namespaces (or no namespace).

I don't want the functionality of the module itself tied to "what we are editing". Only the UI module should care about that. Drush is working with all the packages, so I can't do what you recommend in #25. "drush fl" needs to list all packages regardless of "set".

I'm not seeing any problems so far with the exclude assignment. With my code in #26 and moving the prepending of the namespace to the Generator I've got it all mostly working. I haven't needed to change any of the other short_name/full_name stuff and in fact am relying on that existing code.

I think one of the problems I'm hitting here is that some of the Assignment plugins are really just for the UI. Like excluding packages that don't match the namespace.

We need to de-couple the UI from all of this. We want FeaturesManager to be a pure service that deals with packaging of config.

OK, I've got this working in commit 841762b now (which includes previous commits, so yeah, reviewing this gets a bit complicated, sorry).

Except for perhaps the issue with the Exclude plugin not working (which we can put in a different issue since that's related more to Sets and the UI), the current -dev version is working properly with drush and existing features.

Here are some simple drush tests that describe how this new version works:

1. Install Features 8.x on a fresh D8 site
2. List packages with "drush fl":
   

   $ drush fl
    Name        Machine name  Status        Version  State
    Article     article       Not exported
    Basic Page  page          Not exported
    Core        core          Not exported

3. List the specific package "drush fl article":
   

   $ drush fl article
    Configuration object
    node.type.article
    core.entity_form_display.node.article.default
    core.entity_view_display.node.article.default
    core.entity_view_display.node.article.rss
    core.entity_view_display.node.article.teaser
    field.field.node.article.body
    field.field.node.article.comment
    field.field.node.article.field_image
    field.field.node.article.field_tags
    rdf.mapping.node.article

4. In the UI for Features (admin/config/development/configuration/features), set a Profile Name of "D8 Test" (machine name d8_test)
5. Export all packages via "drush fex":
   

   $ drush fex
   The following extensions will be exported: article, my_image, page, core
   Do you really want to continue? (y/n): y
   Package Article written to modules/custom/d8_test_article.                                                   [status]
   Package Basic Page written to modules/custom/d8_test_page.                                                   [status]
   Package Core written to modules/custom/d8_test_core.                                                         [status]

6. List packages "drush fl":
   

   $ drush fl
    Name        Machine name      Status    Version  State
    Article     d8_test_article   Disabled
    Core        d8_test_core      Disabled
    Basic Page  d8_test_page      Disabled
   

   Note that the machine names are correct and these packages are being detected as disabled modules.

7. List the specific feature: "drush fl d8_test_article"
   

   $ drush fl d8_test_article
    Configuration object
    node.type.article
    core.entity_form_display.node.article.default
    core.entity_view_display.node.article.default
    core.entity_view_display.node.article.rss
    core.entity_view_display.node.article.teaser
    field.field.node.article.body
    field.field.node.article.comment
    field.field.node.article.field_image
    field.field.node.article.field_tags
    rdf.mapping.node.article

8. Manually remove a config item from d8_test_article. Basically, go into the modules/custom/d8_test_article/config/install directory and remove the "field.field.node.article.body.yml" file to remove the body field config from the package.
9. List the feature again "drush fl d8_test_article". Should be same output as above. Notice that even though the body field config file was removed, it is still listed as part of the feature to be exported. Eventually the State field will show that the feature is "overridden" and we'll be able to look at the diff to see this. And eventually we'll be able to flag the body field as specifically excluded. But for now it gets auto-detected.
10. Re-export the feature: "drush fex d8_test_article".
    

    $ drush fex d8_test_article
    The extension d8_test_article already exists at modules/custom/d8_test_article.
    Would you like to overwrite it? (y/n): y
    Package Article written to modules/custom/d8_test_article.

11. Look in the d8_test_article config install directory and you should see the body field yml file restored.
12. Now delete the entire custom/d8_test_article folder
13. List features "drush fl":
    

    $ drush fl
     Name        Machine name      Status        Version  State
     Core        d8_test_core      Disabled
     Image       d8_test_my_image  Disabled
     Basic Page  d8_test_page      Disabled
     Article     article           Not exported

    We correctly see that "article" is no longer a module but is available for export. We can re-export it if we want using the current namespace

If you can think of any more drush tests, please post them. Otherwise this looks pretty good to me.

Comment #25 is addressed in #2455535: Better support Namespaces with drush.

The tests in #32 all still work as described.