Active
Project:
Features
Version:
7.x-2.0-beta1
Component:
User interface
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
23 Jan 2013 at 12:24 UTC
Updated:
1 Aug 2019 at 21:07 UTC
Jump to comment: Most recent
Comments
Comment #1
pfrenssenThis 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.
Comment #2
donquixote commented@pfrenssen
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.
Comment #3
donquixote commentedI am going to be a heretic and re-open this :)
Comment #4
donquixote commentedAnother 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.
Comment #5
donquixote commentedArchitecture 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.
Comment #6
donquixote commentedPreparation: #3060520: Refactor features_export_build_form_submit()
Comment #7
donquixote commentedOption 5
Simpler than the above.
In the
$filesarray fromfeatures_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.