I'm not sure if this is a duplicate - there seem to be several other issues with roughly similar complaints, but none that matches our issue exactly.

We have a node type definition in one Feature (A), with fields for that node type defined in another feature (B). Unfortunately, Feature A shows some of its components as overridden, and claims that feature module B is now a dependency of feature module A.

We're not sure how to tell Feature A to ignore the content of Feature B.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Cameron Tod’s picture

Version: 7.x-1.0-beta6 » 7.x-1.0-rc1
hefox’s picture

My fault :(

Possible solutions:

Solution a:

/**
 * Iterates over data and convert to dependencies if already defined elsewhere.
 */
function _features_resolve_dependencies(&$data, &$export, $module_name, $component) {
   ...
        $export['dependencies'][$map[$item]] = $map[$item]; // remove this

Remove the line at the bottom should fix it. Whether something should add a dependency or not is complicated to guess. Keep getting false dependencies.

Solution b:
Remove from node_features_export

      $fields = field_info_instances('node', $type);
      foreach ($fields as $name => $field) {
        $pipe['field'][] = "node-{$field['bundle']}-{$field['field_name']}";
      }

Solution 3:
Add some detecting to live above to not pipe in already defined fields.
Solution 4:
Change piping to somehow indicate "this is suggested by not dependency, so only add if not already defined" "This is a dependency and needed for this feature to work"

      $fields = field_info_instances('node', $type);
      foreach ($fields as $name => $field) {
        $pipe['field']['suggestion'][] = "node-{$field['bundle']}-{$field['field_name']}";
      }

So what's needed is either removing or change the dependency detection or make piping smarter to know whether things are dependent part of the feature or just suggestions.

hefox’s picture

This is a quickie, untested, probably-has-a-syntax-error patch for solution 4.

Not happy with it.

hefox’s picture

Cameron Tod’s picture

Doesn't seem to work in our case. If it helps at all, the dependency override is in the views component of the feature.

Cameron Tod’s picture

Actually ignore that, I don't think the patch applied to my copy of rc1.

Cameron Tod’s picture

Version: 7.x-1.0-rc1 » 7.x-1.x-dev
FileSize
1.79 KB

The patch was missing a closing bracket, here's a copy with it back in.

Cameron Tod’s picture

OK, so it appears that this patch fixes the issue as posted, thanks! :)

I'm having other issues with weird overrides on other types of elements, but I'll try to work out exactly what is happening before I post a bug.

hefox’s picture

Solution 5) Add a new hook called hook_features_export_suggestions (eventually that can be configured to run or not [so those that don't want non-dependencies items added can opt out]).

Totally favoring this idea now.

hefox’s picture

Status: Active » Needs review
FileSize
16.44 KB

This is sleep deprived take 1 on solution 5, which is prob what should have done originally with _features_resolve_depenenencies logic. There was quite a bit of extra code that should have removed originally from the respective hook_features_export; all code checking if item was already exported to another wasn't needed anymore so a lot of moving around was due to that.

Needs documentation (introduces a hook, changes purpose of another sorta)/testing.

There is api addition, but not removal.

Api addition hook_features_pipe_suggestion (not really attached to that name) + alter hooks: component hook where components can suggest additional items to complete this export (i.e. node suggest fields, views/contextes can suggests content types) that are not required for basic operating of the module.

This allows processing for any duplicates, but indicates the items are not required so 'gracefully' disappear from the suggested. Existing pipe/export hooks should generally still work, but may be vulnerable to this issue till updated.

So tl;dr: new suggestions hook, pipe and features_export to be focused on adding items required for basic functioning.

Cameron Tod’s picture

Thanks for this. As tested it solves the fields issues but the image style and views dependency stuff is still reporting overridden.

Interestingly enough, the old features have a dependency like this:

-	features[ctools][] = "field_group:field_group:1"
 	 
-	features[ctools][] = "views:views_default:3.0"

which are now showing as overriden, as they are no longer part of the 'current' feature.

I have done only very rudimentary testing.

I have some work to do today on some other bits and pieces but I'll try to play around with the new hooks and changes to see if I can massage things just right.

hefox’s picture

Are you on latest dev? I suspect that is related the recent ctools api issues.

Here's a new patch with features.api.php updated.

hefox’s picture

Had added a new function around export options calling, so updated other places to use that function

Cameron Tod’s picture

Nope, we're on rc1, will move to rc2. We need to maintain a stable module release set for our production sites but I can test ctools dev locally to sort out this bug. Gimme a bit...

hefox’s picture

Tests were missing

Grayside’s picture

Status: Needs review » Needs work

Tested and it works.

Now on to bikeshedding and petty issues. (Posting this, will reroll the patch with at least some of these changes next.)

features_test_additional Not the greatest name, but I can't think of anything better. Would like to see a better desription for the module and a /tests directory readme that explains both features_test modules and what they are for.

Agreed, hook_features_pipe_suggestion is a bit of a strange name. It's not so much suggesting pipes as it is suggested components for the feature, which it does by adding them to the pipe. Separate hook from hook_features_pipe_alter() is intended to maintain backwards compatibility, but let's clarify names more. hook_features_component_suggestion?

And some minor bits:

+++ b/features.api.php
@@ -73,12 +73,10 @@ function hook_features_api() {
+ * Any further Delegating further detection and export tasks to related ¶

whitespace eol

+++ b/features.api.php
@@ -111,10 +109,39 @@ function hook_features_api() {
+ * Each suggestion processor should kickoff returning a keyed array (aka the ¶

whitespace eol

+++ b/features.api.php
@@ -111,10 +109,39 @@ function hook_features_api() {
+    $pipe['othercomponent'][$component] = $component;

Other issue followup--examples that use generic words like "component" only help developers that have already figured out what we mean by component.

+++ b/features.api.php
@@ -235,10 +262,10 @@ function hook_features_export_alter(&$export, $module_name) {
+ * Alter the pipe (new depedent and suggested components) array for a given
+ * component. This hook should be implemented with the name of the component
+ * type in place of `component` in the function name, e.g. ¶
+ * `features_pipe_node_alter()` will alter the pipe for the node component.

Would move explanation of pipe to end and expand to full sentence instead of a couple words in parentheses. (Also, spelling of dependent)

+++ b/features.api.php
@@ -235,10 +262,10 @@ function hook_features_export_alter(&$export, $module_name) {
+ * type in place of `component` in the function name, e.g. ¶

whitespace eol

+++ b/features.api.php
@@ -255,7 +282,7 @@ function hook_features_pipe_COMPONENT_alter(&$pipe, $data, $export) {
+ * Alter the pipe (new depedent and suggested components) array.

Move an explanation of the pipe to next paragraph and expand. Can reuse text from above.

+++ b/features.api.php
@@ -274,6 +301,41 @@ function hook_features_pipe_alter(&$pipe, $data, $export) {
+  if (in_array($data, 'my-node-type')) {
...
+  if (in_array($data, 'my-node-type')) {

in_array($needle, $haystack)

+++ b/features.export.inc
@@ -45,17 +45,36 @@ function _features_populate($pipe, &$export, $module_name = '') {
+
+      // Gather any suggestions for items to add to feature if not already provided.

provided by what?

+++ b/features.export.inc
@@ -45,17 +45,36 @@ function _features_populate($pipe, &$export, $module_name = '') {
+      // Even if empty suggestions, call alter so new suggestions can be added.

Grammar a bit fuzzy.

+++ b/tests/features.test
@@ -167,3 +167,101 @@ class FeaturesUserTestCase extends DrupalWebTestCase {
+      'field_name' => $field_name, ¶

whitespace eol, this and another ten or so below.

Grayside’s picture

Status: Needs work » Needs review
FileSize
49.61 KB

Addressed most of the concerns. In the end, could not identify a better hook name (my proposed would have confused hook_pipe_COMPONENT_alter() and hook_component_suggestions_alter() a little to close for comfort.

Also, didn't feel like writing a README for the testing section, but did bundle features_test into it's own subdirectory to clean things up.

kclarkson’s picture

I applied the patch in #17 but it did not clear my conflicts.

hefox’s picture

This is a patch against latest head + #1537838: Upgrading to 7.x-1.0-rc2 (from rc1) breaks taxonomy creation

It also removes the move of the feature features_test to folder features_test to keep from conflicting again.

hefox’s picture

Status: Needs review » Needs work

white space issues

hefox’s picture

+++ b/features.export.inc
@@ -85,12 +102,17 @@ function _features_populate($pipe, &$export, $module_name = '') {
+      if (isset($map[$item]) && $map[$item] != $module_name && module_exists($module_name)) {

Yeaaa this is suppose to be module_exists($map[$item]) not module_exists($module_name)

*head desk*

mpotter’s picture

Status: Needs work » Closed (won't fix)

This issue has languished and needs attention if it's still a problem. However, with the split of field_base and field_instance you shouldn't run into any problem anymore with fields in a different feature from the node.

Re-open this with an updated patch if this is still something needed in 2.x