Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When I'm reverting features via drush, it doesn't detect any overrides, despite the rules are shown as Overridden in UI at /admin/config/workflow/rules and I'm able to revert them correctly, but not at /admin/structure/features/my_rules.
The problem is with _features_sanitize() function which sanitizing too much of code (e.g. weights).
$ drush -y fr my_rules
Current state already matches defaults, aborting. So:
- /admin/structure/features/my_rules or drush -> Default
- /admin/config/workflow/rules -> Overridden
---
1. Checked this code:
<?php
module_load_include('inc', 'features', 'features.export');
$feature = feature_load('my_rules');
$overrides = features_detect_overrides($feature);
and $feature is listed correctly, but $overrides is empty. Tested by drush scr test.php.
2. Further more printing states saying the rules_config is as FEATURES_DEFAULT (0), tested code:
$module_name = 'my_rules';
$states = features_get_component_states(array($module_name), FALSE);
3. I've checked the signatures (normal, default and cache), it gives the same MD5:
$module_name = 'my_rules';
$normal = features_get_signature('normal', $module_name, 'rules_config');
$default = features_get_signature('default', $module_name, 'rules_config');
$codecache = features_get_signature('cache', $module_name, 'rules_config', $reset);
var_dump($normal, $default, $codecache);
Tested by:
$ drush scr test.php
string(32) "2b5951031ccd9a8ff49f6f1eab5d567b"
string(32) "2b5951031ccd9a8ff49f6f1eab5d567b"
string(32) "2b5951031ccd9a8ff49f6f1eab5d567b"Problem
When I've tested manually the default and normal objects, they're only differ when not sanitized, code:
module_load_include('inc', 'features', 'features.export');
$module_name = 'my_rules';
$def_objects = (array)features_get_default('rules_config', $module_name, TRUE);
$norm_objects = (array)features_get_normal('rules_config', $module_name);
// _features_sanitize($def_objects); // Does not work when sanitized (no difference).
// _features_sanitize($norm_objects); // Does not work when sanitized (no difference).
file_put_contents(__DIR__ . '/defaults.txt', features_var_export($def_objects));
file_put_contents(__DIR__ . '/normal.txt', features_var_export($norm_objects));
When enabled _features_sanitize(), the code removes a lot of code (trimming 1.5M into 200K).
Using above code I've generated 4 files (defaults.nosan.txt, defaults.san.txt, normal.nosan.txt, normal.san.txt) depending on whether file was sanitized or not.
Sanitized code returned no differences (diff -u defaults.san.txt normal.san.txt).
I've disabled sanitization, and I've found the following differences:
--- defaults.nosan.txt 2016-04-07 16:46:13.000000000 +0100
+++ normal.nosan.txt 2016-04-07 16:46:22.000000000 +0100
@@ -32,7 +32,7 @@
"form" : [ "form" ],
"form_state" : [ "form_state" ],
"element" : "radios:field_yes_or_no:und",
- "value" : "999",
+ "value" : "986",
"regex" : 0
}
}
@@ -209,7 +209,7 @@
{ "rules_forms_element_value" : {
"form" : [ "form" ],
"form_state" : [ "form_state" ],
- "element" : "radios:field_payment_type:und:0:value",
+ "element" : "radios:field_payment_type:und",
"value" : "2",
"regex" : 0
}
@@ -1946,7 +1946,7 @@
"form" : [ "form" ],
"form_state" : [ "form_state" ],
"element" : "radios:field_yes_or_no:und",
- "value" : "999",
+ "value" : "986",
"regex" : 0
}
},
@@ -5941,12 +5929,6 @@
},
{ "rules_forms_set_access" : {
"form" : [ "form" ],
- "element" : "foo_stepper:groups:group_additional_evidence",
- "access" : "1"
- }
- },
- { "rules_forms_set_access" : {
- "form" : [ "form" ],
"element" : "foo_stepper:groups:group_eligibility_assessment_org",
"access" : "1"
}
@@ -6057,12 +6039,6 @@
},
{ "rules_forms_set_access" : {
"form" : [ "form" ],
- "element" : "foo_stepper:groups:group_additional_evidence",
- "access" : "1"
- }
- },
- { "rules_forms_set_access" : {
- "form" : [ "form" ],
"element" : "foo_stepper:groups:group_eligibility_assessment_org",
"access" : "1"
}
...
// and a lot of more
So basically this rule:
'my_rules_apply_skip_permit_dates' => { "my_rules_apply_skip_permit_dates" : {
"LABEL" : "Apply for skip permit - dates",
"PLUGIN" : "reaction rule",
"OWNER" : "rules",
"REQUIRES" : [ "my_entityform_rules", "rules_forms" ],
"ON" : { "rules_forms_apply_skip_permit_entityform_edit_form_form_built" : [] },
"IF" : [
{ "my_entityform_element_compare_duration_value" : {
"form" : [ "form" ],
"form_state" : [ "form_state" ],
"element" : "date_combo:field_date1:und:0",
"element_2" : "date_combo:field_date_calendar:und:0",
"use_today" : "0",
"type" : "d",
"op" : "lt",
"value" : "0"
}
}
],
"DO" : []
}
},
becomes:
'my_rules_apply_skip_permit_dates' => array(
'dependencies' => array(
0 => 'my_entityform_rules',
1 => 'rules_forms',
),
'label' => 'Apply for skip permit - dates',
'name' => 'my_rules_apply_skip_permit_dates',
'owner' => 'rules',
),
So there is a lot of information missing from the object, such as weight, condition, etc.
| Comment | File | Size | Author |
|---|---|---|---|
| #36 | features-revert-rules-2701957.patch | 1.23 KB | eigentor |
| #25 | features-rules_dont_revert-2701957-25.patch | 1.24 KB | drupov |
| #21 | features-rules_dont_revert-2701957-21.patch | 479 bytes | Alex Bukach |
| |||
| #16 | Features - Rules Bug demo.mp4 | 8.77 MB | arlinsandbulte |
| #11 | features-2701957-11-Detecting-rule-changes-broken.patch | 762 bytes | geek-merlin |
| |||
Comments
Comment #2
kenorb CreditAttribution: kenorb commentedComment #3
kenorb CreditAttribution: kenorb commentedComment #4
kenorb CreditAttribution: kenorb commentedComment #5
kenorb CreditAttribution: kenorb commentedComment #6
kenorb CreditAttribution: kenorb commentedComment #7
kenorb CreditAttribution: kenorb commentedComment #8
mpotter CreditAttribution: mpotter at Phase2 commentedUnfortunately sanitization is absolutely required for many other pieces of config, so turning it off won't be a solution. You'll probably need to dig into the function to figure out why it's causing this issue with rules.
Without it a lot of things cause overrides that are not real, such as integer vs string values and array ordering. It's also a very tricky function because it need to remove recursion within the nested data. So my guess is that the parts that are different in rules are because those pieces are getting trimmed from the comparison tree for some reason.
Comment #9
clemens.tolboomThanks for reporting this. I found sandbox CTools Export code - diff [edit] https://www.drupal.org/sandbox/roderik/2409467 [/edit] which shows 5 out of 6 empty diff in my situation and helped me to fix/revert my 6th rules component.
As that module is called CTools guess we need to report at that project.
Trouble with my 6th rules component is it had a real diff but features failed to get it.
We have also Search API overridden on admin/config/search/search_api which features does not pick up. That could be empty too but there is no diff in the actions menu for search api. I'm not even sure I can run a
drush ctools-export-view XXXfor it as it's config lands in my-module.features.incComment #10
geek-merlinstill an issue, kills rules deployment so setting high.
Comment #11
geek-merlinThanks @kenorb for your really fine debug work and @mpotter for your guidance.
With some git blame i could track this down to a regression from #2497139: Alter hooks causing overrides due to object properties not sorted.
Huh, this line is scary, given what PHP considers false (amongst others, FALSE, '', '0', 0, [] ):
I'd strongly suggest to filter only NULLs, as any other value might be important, and false positive overrides are less harmful than non-deploying stuff. (OK we might discuss about empty arrays if that helps but i cant think so)
Why does this break rules? In rules the 'active' property defaults to TRUE (and is ommitted in export), but active=false is filtered, so active and inactive rules won't be noticed as override.
Patch flying in that fixes this and worksforme.
EDIT: the line that sorts non-assoc arrays also scares me...
Comment #12
kenorb CreditAttribution: kenorb commentedGreat work, thanks, I'll test the patch.
Comment #13
mpotter CreditAttribution: mpotter at Phase2 commentedInterested in hearing the results of people testing this patch who were having the original problem. I do think that empty arrays might also be an issue. This will also need testing to see if making this change causes new false overrides to be shown as was fixed in the original issue linked in #11. So some tests for this would be welcome. I know that the code being changed here can cause lots of side-effects.
(Also, lowering this to Normal since it just affects Rules, so not the majority of users)
Comment #14
kenorb CreditAttribution: kenorb commentedTested using
drush -y fu my_rulescommand, however I didn't get any meaningful results yet.1st test
Before the test, I had some rule changes (especially extra
'required_message' => '',), but when I tried to reproduce the same thing again (after reseting the files), it didn't make any difference (with and without the patch). It taken 797MB of memory (706 rules), a bit quicker after applying the patch, but it could be just caching.2nd test
This involved dumping of my db, and trying patch and no-patch from the same point of db after clearing caches (cc all & memcached). No differences.
3rd test
For 3rd test, I did the following steps:
- removed features_codecache variable
drush vdel features_codecache -y- enabled features_modules_changed (
drush -y vset features_modules_changed 1)- cleared memcached (
echo flush_all > /dev/tcp/localhost/11211)Feature still didn't recognised any changes.
I couldn't test with features_rebuild_on_flush (
drush -y vset features_rebuild_on_flush 1), since it's crashing with infinite loop (another bug: #2698553: entity_defaults_rebuild triggers another entity_defaults_rebuild causing an infinite loop).So my test failed for some reason by Features not picking up the changes, could be some caching hacks. I didn't know how exactly to force feature-update to pick up the new changes. Or maybe rules didn't have any changes. I'll try to re-test that again or in clean env.
Comment #15
geek-merlin@mpotter #13:
>This will also need testing to see if making this change causes new false overrides to be shown as was fixed in the original issue linked in #11.
I strongly doubt that with a general approach on this we can have both goals of
a) Remove all items that are meaningless and produce false overrides
b) Do not remove any item that has meaning for some module.
Violating a) will get us false positive overrides, while violating b) will give us broken deploys. I think the choice is clear.
Comment #16
arlinsandbulte CreditAttribution: arlinsandbulte as a volunteer commentedI ran into this same issue.
It is easily reproducible on simplytest.me.
Steps to demonstrate/reproduce:
I created a video of the above steps demonstrating this issue on simplytest.me and have uploaded that video to this issue.
I also tried applying the patch from #11 with simplytest.me, but received an error. I assume the patch does not apply correctly.
Comment #17
arlinsandbulte CreditAttribution: arlinsandbulte as a volunteer commentedOK, I managed to test the patch in #11 above, but it does NOT resolve the issue demonstrated in #16.
Comment #18
geek-merlin@arlinsandbulte: Did you clear all caches? This is important.
;-)
Comment #19
arlinsandbulte CreditAttribution: arlinsandbulte as a volunteer commentedYep, I just verified... even if I clear all caches (at /admin/config/development/performance), the feature still says 'default' after I edit a rule.
Note:
Features 'state' is not working correctly at /admin/structure/features.
But, Rules 'status' is working correctly at /admin/config/workflow/rules (for the most part).
Maybe that provides a clue.
Comment #20
geek-merlinClarifying title.
Comment #21
Alex Bukach CreditAttribution: Alex Bukach commentedIn my case the issue happened since Ruls objects store settings in non-public fields, while
get_object_vars()used for calculating checksums doesn't fetch that fields at all. Here's the patch that fixes the issue for me.Comment #22
mpotter CreditAttribution: mpotter at Phase2 commentedNo, the patch in #21 is not good. You can't just use (array) to convert an object, it will cause errors in some situations. We used to have it that way and had to change it to get_object_vars() a while back, so I'm not going to bounce back and forth.
Rules shouldn't be storing stuff that it needs in non-public fields. That sounds like an issue to file with the Rules module.
Edited: In fact, the *real* way to fix all of this would be for Rules to add a proper "export()" method like ctools does. Then it could control all of this and not rely on Features trying to figure out how to convert their object class into a data array.
Comment #23
Alex Bukach CreditAttribution: Alex Bukach commented@mpotter thanks for reviewing the patch and for giving a hint how the issue could be resolved correctly. Do you mean making rules CTools-exportable instead of relying upon Entity API?
Comment #24
drupov CreditAttribution: drupov commentedShould we move this to the Rules project? Should we at all, as changing the export mechanism for Rules sounds like a big overhaul.
BTW with patch from #11 features does not find my overridden rules configuration at all and with #21 I get a "PHP Fatal error: Maximum function nesting level of '256' reached, aborting!" in my local setup, so both are not really doing any good, at least for me.
Comment #25
drupov CreditAttribution: drupov commentedThe attached patch would not remove recursion from 'rules_config' objects and later not sanitize each one object.
I am aware it's not a clean solution at all, but the rules module is deeply integrated into so many projects (thanks to commerce also) so that an exclusion from the features side seems at least worth thinking about. Also not reverting rules configuration during deployment causes so many headaches. And last but not least I don't see any solution as mentioned in #22 ("proper export method") coming any time soon for Drupal 7.
Comment #26
joseph.olstadneed a 'rules' export function similar to ctools export
Comment #27
geek-merlinSorry, but the #21 thread is imho hijacking this issue which was - maybe with similar symptoms - a totally different thin in #11.
If we have different causes for this symptom we should sort them out somehow, e.g. in subtickets.
Comment #28
WorldFallz CreditAttribution: WorldFallz commentedThe patch in #25 may not be desirable, but it still applies cleanly and does work to get rules features deployable again.
I don't mean to be naive-- but what are folks doing to get features to deploy properly if not this patch?
With over 300k rules sites, there have to be a large number of them using a pretty off-the-shelf standard features deploy workflow. It seems hard to believe they're just manually correcting broken rules deployments.
Comment #29
eigentor CreditAttribution: eigentor commentedI had the same issue with Features 7.2.10.
I can confirm the patch in #25 applies cleanly and solves the problem.
Comment #30
pvdjay CreditAttribution: pvdjay as a volunteer commentedI was having the same problem on my sites as described in this issue. The patch in #25 solved the issue. Now corresponding features show as overridden as well.
Comment #31
jurgenhaasI can also confirm that the patch from #25 fixed the issue for me.
Comment #32
xaa CreditAttribution: xaa as a volunteer commentedcomment deleted. wrong issue sry.
Comment #33
TR CreditAttribution: TR commented#25 is a patch for the Features module, not for Rules. It does not apply to the Rules module, and we can not use that patch to fix the Rules module.
This issue was moved out of the Features module issue queue because Features will not be adding that patch.
The suggestion in #22 and #26 is that this can be solved in Rules, but the proper solution is unclear to me. Because I do not use Features, I will not be working on this.
This is unlikely to be solved unless someone from the community works on and contributes a solution. Thus I am marking this issue as "Postponed" until someone steps up to help.
Comment #34
klausiI don't think we can solve this in Rules because Rules use the Entity API Features export functionality. It turns exports into arbitrary complex objects that have protected/private properties and then Features is not smart enough and strips properties from objects. Features in Drupal 7 is not really ready for object-oriented code being used for example in Rules - you cannot simply run get_object_vars() on those.
We could introduce a features_sanitize() hook in the Features module so that modules like Entity API/Rules can perform their own sanitization on things? Might be a bit late for that.
Or we could refactor features_detect_overrides() to work better, but I don't see another way how Rules could influence features_sanitize()?
Also going with Features patch #25 for now which is quite hacky but at least a stop gap.
Comment #35
cdmo CreditAttribution: cdmo commentedThis issue gets me every time and then I forget. Honestly, feels like rules shouldn't advertise features support if this issue can't be permanently solved.
Comment #36
eigentor CreditAttribution: eigentor commentedSince the patch from #25 is very old and does not apply anymore, here an updated patch for Features 7.x-2.15