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.
Hi,
i run into the problem that in some cases ( i have no idea why and at which time ) the features exports the field instance display settings with the field weight value as string and sometimes as integer. Example:
$fields['node-xxx']['field_instance']['display']['weight'] = 14
$fields['node-xxx']['field_instance']['display']['weight'] = '14'
this problem causes in a permanent override status. If someone had a idea where the right place for debugging i keep an eye on it.
thanks
Dennis
Comment | File | Size | Author |
---|---|---|---|
#36 | features-type_discrepancies-1721926-36.patch | 1.2 KB | Anonymous (not verified) |
#34 | features-numeric_string_export-1721926-34.patch | 921 bytes | jweowu |
#35 | features-numeric_string_export-1721926-35.patch | 795 bytes | jweowu |
#28 | 1721926-features-intval-ping-pong.patch | 624 bytes | impleri |
#14 | 1721926.features.field-weight-ping-pong.patch | 882 bytes | joachim |
Comments
Comment #1
mrsimonelliott CreditAttribution: mrsimonelliott commentedYeah, I have this exact issue too.
It seems that in table field_config_instance, some fields weights are stored as integers and some fields weight as strings. Features only accommodates weights stored as strings. Whilst the actual weight value corresponds between the db and Feature code, the method of storage is causing feature to be overridden.
From two similar fields in table field_config_instance > data
s:6:"weight";s:1:"5"; == Features is happy with this
vs
s:6:"weight";i:8; == always overridden
Comment #2
YesCT CreditAttribution: YesCT commentedon my server, the override is without the quotes, and the default (which comes from a feature generated on my laptop localhost MAMP) has the quotes.
'weight' => '6',
vs
'weight' => 6,
Could this be fixed by changing the database... ?
Comment #3
YesCT CreditAttribution: YesCT commentedcould this be because of a module I added, like that new module had a type for weight were as before, it was not set and was defaulting... I wonder if it would be helpful for us to post what modules we have.
Comment #4
YesCT CreditAttribution: YesCT commentedI mentioned local, I'm on a MAMP. The server is on Pantheon.
Here is a drush pm-list
Comment #5
Blackice2999 CreditAttribution: Blackice2999 commentedHi,
please try the patch from #1042088: Feature stuck in overridden state due to buggy hook detection - it fixes that problem. But we need more reviews for mpotter
thanks
Comment #6
mpotter CreditAttribution: mpotter commentedSome background on this:
The way Features works is to compare the exportable array currently in code with the same exportable array created from the current db state. To get the "default" value from the code, Features directly calls the default hook function. To get the "normal" value from the DB, Features creates an in-memory export of the current DB and then "evals" it to get the resulting array. Then Features sanitizes the results and uses md5 to compute a "signature" that can be compared.
Because of the "eval" step, a value of (weight = 5) and (weight = '5') result in a match. So if this is the only difference in a feature, the feature will actually show as "Default". It's only when you get another difference that causes the feature to be marked as Overridden that the 5 vs '5' will be displayed in the Diff panel because it's just doing a source-code "diff" for display purposes.
Bottom line is that these kind of differences aren't really "real" and should not cause a feature to be marked as overridden. There is usually some other difference that is really causing the feature to be marked as overridden and the 5 vs '5' is more of a red-herring in the diff output.
Not sure how the patch from 1042088 would have any impact on this, but I'll look into it.
Comment #7
Blackice2999 CreditAttribution: Blackice2999 commentedHi mpotter,
thanks for your reply the patch from #5 not solved this problem. I run again into this ping/pong effect. I cant see why it happens and at which point it happend. But after your explain i keep an eye on other changes in the feature.
thanks
Dennis
Comment #8
mpotter CreditAttribution: mpotter commentedYeah, if you get it in the future, post the *complete* list of diffs. The screenshot in this original post was cut off so it's hard to see if there are any other differences.
After reviewing the issue in #1042088: Feature stuck in overridden state due to buggy hook detection I think that would fix this only if you also have another module that is implementing one of the default hooks being used by features. That would cause the feature to be stuck in Overridden and when looking at the Diff tab it would show these 5 vs '5' issues and could confuse people.
I'm going to commit a version of the patch in the other issue since that's definitely a problem, but will leave this issue open in case somebody else runs into a case that can be reproduced reliably.
(btw, the underlying issue is that parts of the core Fields handling stores a string value instead of an integer value in some cases. Probably when the Weight field is read from the field submit form and isn't converted to intval() before saving it into the field_instance array. Features itself is just doing a var_export of the value and has no way to really know that Weight should always be an integer value)
Comment #9
mrsimonelliott CreditAttribution: mrsimonelliott commentedHi mpotter,
Since you asked, here is a full diff of a feature suffering this issue. As you can see, the only reason this feature is overridden is because of multiple instances of this problem.
I've experienced this on several project recently.
I have other features on this project which include fields with weights, but they don't display this issue.
Simon
Comment #10
mpotter CreditAttribution: mpotter commentedStill not able to reproduce this, with a lot of trying here. My guess is that the feature you show above still has something missing that isn't showing up in the plain code diff. It might help to post the *.info file so we can see exactly what all is being exported in the feature. It's usually something like a module that isn't enabled or some other reason the feature is stuck in the override state.
Comment #11
YesCT CreditAttribution: YesCT commentedI have a gut feeling to agree with "My guess is that the feature you show above still has something missing that isn't showing up in the plain code diff."
Comment #12
mpotter CreditAttribution: mpotter commentedOK, in my continued attempt to keep the issue queue here more under control, I'm going to mark this as cannot reproduce for now. But feel free to re-open it if you come up with a case where you can reproduce it for further help.
Comment #13
joachim CreditAttribution: joachim commentedGetting this too on my features, but I'm not sure how I can help with reproducibility. The features in use on my project have all been through many, many updates so it would be impossible to know exactly how they got to this weights ping-pong state.
Could this be addressed perhaps by forcing field weight when exported to always be one or the other of number or string?
Comment #14
joachim CreditAttribution: joachim commentedI had a look through my features and I can't see any kind of pattern. I thought at first that the 0 weights were numerical and the non-zero were strings, but that's not the case.
How about this for a fix?
Comment #15
mpotter CreditAttribution: mpotter commentedDoes the above patch cause your feature to be Default instead of Overridden? As I've said, since Features does an eval() on the results, I don't think this is a *real* override difference, it's just a difference in the raw code that Diff is using. But let me know if it really fixes your feature. Seems a bit kludgy but the real problem lies buried somewhere in core I think, so this might be ok for a workaround.
Comment #16
joachim CreditAttribution: joachim commented> Does the above patch cause your feature to be Default instead of Overridden
It causes it to be overridden because it standardizes all weight values as integers. Re-export the feature one more time so the code & the database agree. After that it's fixed for good.
> I don't think this is a *real* override difference, it's just a difference in the raw code that Diff is using
I'm not sure that's the case. My feature code had some weights as integers, some as strings. I *think* that the field config table's serialized options had a mixture too, but I'm not certain.
Comment #17
cesarmiquel CreditAttribution: cesarmiquel commentedWe are seeing this exact same issue. I'm attaching a drush fd so that you can see it. I did the following experiment: when I saw the error I corrected by hand the file that was causing the issue until I had only one weight which was different. When I do drush fd I get:
Now I edited the file and fixed the weight and got:
I think this confirms that this is the only thing that makes the feature overwritten.
Two more things:
I'll try the patch in #14 and see if it works.
Offtopic: 'newbie question': should I change the Version field in the issue to 7.x-1.0 ?
Comment #18
manuelBS CreditAttribution: manuelBS commentedPatch at #14 solved my problem with weights in field instances export
Comment #19
mpotter CreditAttribution: mpotter commentedComment #20
lise.perceives CreditAttribution: lise.perceives commentedI have posted 2 versions of a feature, that create this error for me.
http://drupal.org/node/1061178#comment-6762978
I have tested the patch. It works (it no longer reports the problematic feature as overridden). However, now all my other features are reported as overridden ... And revert doesn't work on them. So maybe there should be a step, where existing features are updated when the features module is updated?
Patch + recreate all features works for me, and that's what I ended up doing.
Comment #21
mpotter CreditAttribution: mpotter commentedHmm, sounds like this needs more work then. We shouldn't be changing anything that causes all existing features to be marked as overridden. And Features will never automatically update your features since that can cause a lot of sites to break who have made local DB changes and have purposely overridden features.
Comment #22
joachim CreditAttribution: joachim commented> We shouldn't be changing anything that causes all existing features to be marked as overridden.
I've found that happens fairly often -- or at least, module changes cause later feature updates to show loads of extra cruft that's not related to the actual changes.
Comment #23
lise.perceives CreditAttribution: lise.perceives commentedBTW. When I work with features, I get a lot of these:
Notice: Array to string conversion in EntityAPIControllerExportable->applyConditions() (line 624 of C:\Program Files\BitNami Drupal Stack\apps\drupal\htdocs\profiles\agnitio\modules\contrib\entity\includes\entity.controller.inc).
Notice: Array to string conversion in features_get_orphans() (line 700 of C:\Program Files\BitNami Drupal Stack\apps\drupal\htdocs\profiles\agnitio\modules\contrib\features\features.module).
Comment #24
mpotter CreditAttribution: mpotter commentedlise.perceives: please search the issue queue before posting a new issue and never post something like this that is unrelated to the specific issue thread. If it doesn't already exist, create a new issue ticket.
Comment #25
lise.perceives CreditAttribution: lise.perceives commentedI didn't know whether this new behavior was related to this error. However, you're right. This has already been handled in http://drupal.org/node/1514764 .
Comment #26
impleri CreditAttribution: impleri commented@mpotter, is there a reason why this is eval'ing the DB code and comparing the two arrays rather than reading the existing file and comparing it against the file generated from the DB? When I've got a bit of time, I'll try working out a patch unless I hear back that it's already been tried.
Comment #27
lencrockett CreditAttribution: lencrockett commentedI can confirm this is happening to me on a simple feature. Local site is running MAMP (PHP 5.3.6) and exports string weights, staging server exports integers (PHP 5.3.5). I ran both the complete modules through file merge and found that the only differences were in how the weights were expressed. Simply editing the staging version to remove the single quotation marks did indeed fix the perpetual "Overridden" state. Oddly this only occurred on a single feature whereas others I have created and deployed without issue. Might have to do with one component.
Comment #28
impleri CreditAttribution: impleri commentedHere's a simple patch against 7.x-2.x-dev HEAD which simply adds intval and floatval routines in features_var_export(). It doesn't necessarily fix the deeper issue, but it does ensure that all exported numerics are integers or floats which should at least keep the values consistent.
Comment #29
danielnolde CreditAttribution: danielnolde commentedThe patch in #28 solves the issue for me and works like a charm, even with features version 1. Thanks!! Hope to see this small but relieving change committed to features soon.
Comment #30
mpotter CreditAttribution: mpotter commentedCool, that's a much less intrusive solution. I agree that there is probably a deeper issue here that is very dependent on specific components and past database states, but it's going to be nearly impossible to debug and reproduce. So we'll go with fixing the integer Weight values for now. Committed #28 to 3fc3a94.
Comment #32
jweowu CreditAttribution: jweowu commentedPost-patch, I've encountered a features over-ride situation which will make the following change after a drush features-update:
The string
'000000'
is a RGB colour (black; another example would be'jquery_colorpicker' => 'a621a6',
), and this all comes from the jquery_colorpicker module.This seems to point to
is_numeric($var)
not being a reliable condition if you don't know the purpose to which the value will be put.Interestingly, the drush features-diff output for the over-ride is:
Comment #33
jweowu CreditAttribution: jweowu commentedMaybe it would be enough to verify that when you convert back to a string, you get the same value you started with?
Comment #34
jweowu CreditAttribution: jweowu commentedAck. I generated this patch in the wrong directory. #35 is the correct one.
Comment #35
jweowu CreditAttribution: jweowu commentedComment #36
Anonymous (not verified) CreditAttribution: Anonymous commentedShould features really care about what is stored in the field_config* tables? I think there's a case to be made that features should not interfere with whatever FieldAPI decides to stuff in the database - if there's glitches in FieldAPI or contrib around types, they should really be fixed at source. Here's a simple patch that ensures that whatever is exported is stored as the same type by using the identity operator instead of equivalence.
Comment #37
jweowu CreditAttribution: jweowu commented0xdeadbeef: Keep in mind that #28 was committed (albeit still in need of #35 if this approach is retained). My impression is that your patch is intended to replace all of that, so you should also be reverting 3fc3a94.
Comment #38
Anonymous (not verified) CreditAttribution: Anonymous commentedIf the consensus is that the approach is sound, i'll re-roll with a revert. As it is, the already committed solution is awkward for us as it would mean re-creating a load of features from scratch.
Comment #39
Ronino CreditAttribution: Ronino commentedI had issues similar to those described in #32 with the patch from #28. After applying, I get changes like this:
This might lead to undesired changes.
However, the patch from #36 worked fine.
Comment #40
mpotter CreditAttribution: mpotter commentedI think #36 is a separate issue. The #28 fix that was already committed attempted to deal with exporting values in the correct format based on their data type and applies to all types of features.
#36 just applies to Field exports. For example, #39 describes a case well beyond the simple 14 vs '14' weight issue in the original post. #36 seems like a reasonable idea and should be able to be applied even keeping #28.
So let's get some additional review of this and I'll consider committing it.
Comment #41
jweowu CreditAttribution: jweowu commentedAssuming #28 stays committed, then I believe #35 is needed (see #32 and #33 for details), so if #36 is unrelated, that's a bit confusing.
Should #36 be split out into an actual separate issue?
Comment #42
Ronino CreditAttribution: Ronino commentedmpotter wrote:
Hmm, I'm not sure. For me, #36 totally fixes the original problem, whereas #28 leads to issues described in #32 and #39. Since applying #36 (and not #28), I don't encounter 14 vs '14' issues any more...
Comment #43
mpotter CreditAttribution: mpotter commented#36 was actually already handled in a similar patch in #2088771: Unable to update a field/field_instance/field_base when equality operator things not-same arrays are the same commit 70daa4a.
I committed #35 in 78772d5.
Comment #45
PawelR CreditAttribution: PawelR commentedHi,
I would like to reopen this issue because I don't think it's fixed properly.
The way how it tries to fix this problem introduces a serious bug.
It converts numeric strings like "0" (returned by from field_info_instance() for example) to integers and these integers are getting written in to the various database configuration tables when features gets reverted.
Unfortunately this breaks Drupal core in at least two places:
https://www.drupal.org/node/2248129
https://www.drupal.org/node/2298975
I think this code:
should be removed asap from features_var_export() function.
Comment #46
joachim CreditAttribution: joachim commentedI don't know if this is related or not, but I've just made a feature that I'm using to define a view for a test case. I added 'hidden = TRUE' to the feature's info file, and I've just rerolled it for a change to the view, and now the diff shows this:
Comment #48
joachim CreditAttribution: joachim commentedI've just noticed something that might or might not be relevant :)
ctools_var_export() and features_var_export() more or less do the same thing, but differ in their handling of integer values: features_var_export() ensures they are written to code as integers, whereas ctools_var_export() quotes them as strings.
Might it be that somehow in some circumstances the other export function is used? Eg, on creating the feature one is used, and on feature update the other?
Comment #49
kenorb CreditAttribution: kenorb commentedFixed as per #43. If there is any other issue, please raise the new ticket.