When Features tries to determine if a component is overridden, it calls a "sanitize" function. This function is responsible for sorting and cleaning the object so it can be compared. It currently only sorts arrays. If the component contains an object, it is skipped.

If you implement an alter hook to modify a component (as with Features Override, or doing it manually), often the properties of an object will get added in a different order, causing the component to be marked as Overridden with no way to fix it.

The following patch attempts to also sort objects by their property key values. This same issue was encountered when writing the Features Override module and it had code for deeper sanitization of both arrays and objects. This patch brings over code from Features Override into a core features_sanitize function. This code was brought over in a way that Features Override can also re-use to reduce duplicate code in the future.

I've tested this pretty extensively within several Open Atrium sites. It fixes several Overrides (alters to Panelizer specifically) and doesn't seem to cause any new issues. But I need more testers on this and probably need some help writing some automated tests for this.

Patch incoming...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpotter’s picture

mpotter’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: features_alter_hooks_causing_overrides-2497139-1.patch, failed testing.

mpotter’s picture

Status: Needs work » Needs review
FileSize
8.31 KB

New patch to fix the missing function. Kind of a nasty namespace collision where Features Override has a function called features_get_ignore_keys(). Since this should be an internal function, I added it to Features as _features_get_ignore_keys() and changed the hook to be in the Features namespace.

If this gets removed in Features Override then any other module implementing the hook_features_override_ignore will need to change to use hook_features_ignore.

claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

Thank you. This fixes the issue.

claudiu.cristea’s picture

Status: Reviewed & tested by the community » Needs work

Sorry. The patch fixed the false override but I got a new one, also false :(

mpotter’s picture

Status: Needs work » Needs review

This doesn't necessarily fix *all* false overrides. If you mark this as "needs work" you need to be really specific about the exact override you are still getting.

mpotter’s picture

Status: Needs review » Fixed

Committed to 1905744.

  • mpotter committed 1905744 on 7.x-2.x
    Issue #2497139 by mpotter: Alter hooks causing overrides due to object...
das-peter’s picture

Priority: Normal » Major
Status: Fixed » Needs review
FileSize
479 bytes

Sorry, I've to re-open this.
I'm exporting rules configuration entities and this patch causes "Fatal error: Maximum function nesting level of '500' reached, aborting!" errors.
The reason seems to be that you can't just cast a rules entity object to an array (using (array)), I'm assuming the issues is caused by the fact that a rules entity objects work with iterators.
The attached patch uses get_object_vars() to fetch the public available properties as an array - this seems to work fine so far.
However, I can't tell if the desired behaviour of this patch suffers from this change.

  • mpotter committed b9f90cc on 7.x-2.x authored by das-peter
    Issue #2497139 by mpotter, das-peter: Alter hooks causing overrides due...
mpotter’s picture

Status: Needs review » Fixed

Thanks! Nice catch. Interesting that it all worked for ctools, views, entity, but not for Rules. I tested your patch and it still seems to work fine and handles the original issue. I've committed this fix to b9f90cc so it will be in the 2.6 release later this week.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

a.milkovsky’s picture

Strange, but the #10 patch caused a big issue for me. Rules were not exported properly after update to 7.x-2.6. features_sanitize() only returned some general rules info(label, tags) but did not include rules actions, conditions.
I had to revert the fix.

geek-merlin’s picture

@a.milkovsky: I fixed this regression in #2701957: Rules deployment breaks, due to overridden rules not recognized - you may want to test that patch and set this issue RTBC.