When updating an existing feature, the existing *.module file is being reused and not changed. When adding a component that requires the *.feature.inc file to be included, a warning is being issued that the include is not in the *.module file or rather a warning that it does not "seem to be included". If Features already knows that the include is not there but it should be, then it should just include it! Moreover when not using drush but the UI, the warning message may never be displayed to the user, since the result of the "recreate feature" form is not another page but a download. I was chasing this bug for hours until I finally saw the warning by chance and was able to search for that string. I did not expect it in the export routine but rather in the install or revert routine, since the symptom appears there.

The expected behavior should be, that a recreated feature is working as it was created. I assume the original idea was to not overwrite any custom code in *.module. With my patch a new empty *.module file will look like this:


/**
 * Drupal needs this file even though it might be empty.
 *
 * Features will put autogenerated code in between the
 * \--AUTOGENERATED_BEGIN and \--AUTOGENERATED_END comments.
 * It will replace any custom code you may have written there, so make sure to add
 * your code only outside of those comments and don't remove the comments.
**/
//--AUTOGENERATED_BEGIN
//--AUTOGENERATED_END

When recreating the feature, everything in between the comments //--AUTOGENERATED_BEGIN and //--AUTOGENERATED_END will be replaced with whatever the Feature module generates, which _for now_ may just be the include of *.features.inc.

If those comments do not exist in *.module, they are not being included and *.module is not being changed, so there is no interference with any custom code.

CommentFileSizeAuthor
features.patch3.12 KBVolx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drewish’s picture

drewish CreditAttribution: drewish commented

marked #921092: Doesn't add include_once('mymodule.features.inc') when needed as a duplicate because you posted a patch... you should really look for existing issues before posting.

patcon’s picture

patcon CreditAttribution: patcon commented

Oh man, thanks for pointing this out Volx!! I've been worrying about this for about a month! The patch didn't work for me, but the fix is simple.

Posted another issue through since my symptoms were a bit different, but in retrospect, maybe it could've gone here. Dah well.
#980472: Strongarm feature properties would not revert nor override

patcon’s picture

patcon CreditAttribution: patcon commented

Hm... just realized I may need to have run drush fu twice to see results... I ran once and say a template lik you copied above, but no include_once command...

Or maybe the patch only works if used from the beginning? Oh well, thanks for the help troubleshooting :)

Grayside’s picture

Grayside CreditAttribution: Grayside commented

The drush warning was my first stab at improving this. My use case was creating a module, handing editing the .info file, then using drush fu to generate the feature components. "Seem to be included" refers to the fact that you could, potentially, do something like module_load_include('inc', 'module', 'module.features') or other weird include approaches. While Features does auto-generate how the files relate to each other, I did not want to control what a developer does without greater study. I also didn't want to get involved with modifying the .module file, something which typically has a lot of manually crafted code, unlike the features components files.

I'm not really keen on the auto-inserted text/placeholders. This is closer to a workflow/process I could get behind:

  1. Using get_included_files() as a best-effort check to see if the file was included.
  2. [drush only] If not, back up the .module file.
  3. If it is absent, look for comments per coding standards and insert the include statement. For example, searching for //$Id or a @file comment block.
  4. [drush only] Compare the size of the .module file to it's previous size. If shorter, restore the backup and warn the user.
patcon’s picture

patcon CreditAttribution: patcon commented

Sorry, I think that a lot of this is over my head, since I'm just getting into the guts of Features, but couldn't an empty my_module.features.inc just be included no matter what?

@Grayside, I totally understand that you "did not want to control what a developer does without greater study", but I'm not sure whether this developer control (of include method) is worth the hassle that this might be causing. I'm sure there are many others who never find this issue queue (it's hard to know what to search unless you see that drush error), and I'm sure others have come across this problem and simply grappled with it unsuccessfully before flat-out recreating the feature.

And it's likely that the group who starts a feature off simplest (and hits this snag) is likely the new user trying to figure out how Features works, so I'm worried this could aggravate just the sort of people we'd last want to encounter it.

Anyhow, just my two cents :)

Volx’s picture

Volx CreditAttribution: Volx commented

Yes, the patch only works if you create the feature with the patch or manually add the AUTOGENERATED tags in your feature's module file.
One problem I encountered with my patch is, that the regular expression looking for AUTOGENERATED_END only matches if it is followed by a newline, I included that to so that the occurrence within the comment does not get matched. But if for some reason the trailing newline is removed, the patch will not work. So the regular expression should be replaced with something smarter.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Needs review » Needs work

Would it not be better to just do a search through the current *.module file for the normal include_once('modulename.features.inc'); line, rather than requiring new comments be added; also, adding new required comments to the module files would mean features created before this change would not be updatable.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

Also, what about when the *.features.inc file is removed?

ericduran’s picture

ericduran CreditAttribution: ericduran commented

I posted a patch over at #994140: Modify .module file when a features.inc is added technically that issue is a duplicate of this but is for drupal 7.x so for now I wont mark it a duplicate.

The patch I posted on the other issue is appending the inc file at the end of the .module file as a simple solution to the problem.

I happend to disagree with grayscale and agree more with what @Volx is doing on this patch. Features should add its include statement wrap around some comments to know this is the feature generate include which will allow us to detect that include file and if it's not there add it.

This should also account for Damien comment which is when the file is no longer needed we can easily parsed through it and remove it.

Normally I would be completely against feature modifying my module file, especially since I always seem to add extra code in there but if we wrap it around some comments it should be pretty save.

carn1x’s picture

carn1x CreditAttribution: carn1x commented

subscribe

bradjones1’s picture

bradjones1
Primary language English
CreditAttribution: bradjones1 commented

FYI, one of the symptoms of this problem is (among other things) views and other Features components disappearing when clearing the cache. See #1331656: Components from Features that contain field_collection views go missing.

Grayside’s picture

Grayside CreditAttribution: Grayside commented
Status: Needs work » Closed (duplicate)

It's reasonable to say one is a duplicate of the other. Technically, the one now "live" is the duplicate #994140: Modify .module file when a features.inc is added