I created a feature including a Menu and a Vocabulary. I generated the feature through the new Button "generate feature". Later I removed the Menu Item and the Vocabulary again and hit "generate feature" again. However, as Menu and Vocabulary are generated as separate files, they were still existent in the feature directory. This caused the feature to show as overridden albeit it wasn't. After deleting these two file manually the feature correctly displayed as default again. I expected the regular behavior of this function to wipe the directory clean (probably not including .svn or .git folders), and then write all the files to that directory again.

Comments

pfrenssen’s picture

Status: Active » Closed (works as designed)

This is by design. Features are standard drupal modules and are often extended with custom code, configuration screens, theming etc. Features can not distinguish between old exported code and manually written code, so it will not touch any existing code.

If you want to remove components from an existing feature you will have to do this manually.

donquixote’s picture

Issue summary: View changes

@pfrenssen

Features can not distinguish between old exported code and manually written code, so it will not touch any existing code.

This is already not 100% true now.
If you put your custom code into "the wrong file", e.g. NAME.features.field_base.inc or NAME.strongarm.inc, then a future features update will destroy your custom code.

I am currently writing a D7 module which should have features integration.
One of the major pain points I have with features is git conflicts from having multiple items in the same file.
For this new module I am considering a features integration that generates one file per exportable item. Perhaps even yml files :) The main hook would then just scan these yml files.
See also #3060408: Support nested subdirectories for $files['_files']..

With such a system, it is no longer an option to expect a site builder to remove leftover files manually.
To make this work properly, we would need some way to determine which files are "owned" by features and which are not.

Option 1

One idea would be to add additional hooks to allow contrib modules to "clean up" before or after a feature was generated.
E.g. hook_features_post_export().
The problem here: The module that would clean up a NAME.features.SOMETHING.inc file might no longer be enabled. Or perhaps it was never enabled, if the feature was generated on a different website.

Option 2

The second idea would be to do this by naming convention: Anything NAME.features.*.inc is "owned" by features.
A problem here is that some of the features integrations do not follow this naming convention. E.g. it is "NAME,strongarm.inc", not "NAME.features.strongarm.inc".

Option 3

Another idea could be to let the feature itself "remember" which files were generated: E.g.
- store a list of generated files in the *.info file.
- add a "@file" doc that marks generated files.
- maintain a separate file per feature module to store the list of generated files.

The problem here: Such additional code would cause disruption and noise in existing features, similar to the format changes introduced earlier (e.g. having a dot at the end of comments).

Conclusion

I think at the current time the best would be option 1: Adding some hooks that allow other modules to delete files.
This does have the problems mentioned above, but it would shift the responsibility away from features, and make all of this opt-in.

donquixote’s picture

Status: Closed (works as designed) » Active

I am going to be a heretic and re-open this :)

donquixote’s picture

Another idea! Not mutually exclusive with option 1, rather in addition to that.

Option 4

In _drush_features_export() and features_export_build_form_submit():
Call features_export_render() twice, if the feature already exists:
- One time based on the current contents of the *.info file
- One time with the new $export array.

Use array_diff_key() to determine the files to be deleted.
A dedicated array_diff_key() needs to be called for the '_files' array.

When downloading a tarball, there could be an option to include custom files that already exist in the module, but without the to-be-deleted files.
Then the developer can delete the existing module and replace it with the downloaded tarball.

Limitations:
- If a module has changed the path where files are generated, this method will fail to delete the now "deprecated" files.
- If a contrib module is no longer enabled, features won't know what to delete.
- If a tarball is downloaded without the "include custom files" option, features has no way to delete anything.

Other than that, I think this mechanism can be a great improvement.

donquixote’s picture

Architecture notes

Btw, some architecture improvements:
- The same or very similar logic exists in two places: The submit handler and the drush callback. Logic in such places is hard to reuse from anywhere else.
- The $files and $files['_files'] could be unified into a single array, or perhaps into separate arrays $copy, $create and $delete, to then have a single loop where the files are written.
- The filesystem-writing and the tarball-sending could be split out to separate functions.

Perhaps I should open a separate issue with this.

donquixote’s picture

Option 5

Simpler than the above.

In the $files array from features_export_render(), add a new key $files['_delete'], with files to be deleted.
Or alternatively, use the existing $files['_files'] array with $files['_files'][$path]['delete'] = TRUE;.

This moves all the responsibility to 3rd party modules.
It would allow a new 3rd party module to implements one of the other options mentioned before.