Exports produce inconsistent ordering causing issues with Features [#970470]

Attached is a small patch to the way Flag does its exports to ensure that the exported code is always consistent. Features will sort arrays that are passed to it, so it behooves the module to produce consistently-sorted arrays. This patch does that by doing what most others do and switching to text-based array indices. This does not appear to effect flag_get_default_flags since it simply does a foreach loop over the returned array without any concern for what the index is. It also sorts the types array before export so that it's always in a consistent state as well.

Comment	File	Size	Author
#5	flag_export_consistent_output.patch	1.66 KB	mooffie
	flag_export_consistent_output.patch	816 bytes	mgriego

Comments

Comment #1

mgriego commented 23 December 2010 at 17:26

We've been using this on our production site since the issue was created, and it's been working very well. Features exports only change when there are real changes.

Comment #2

mooffie commented 5 January 2011 at 15:42

We certainly need to fix this.

This patch does that by doing what most others do

But isn't this only half of the truth?

Now the flag name will appear in two places, and the programmer will have to update both.

Comment #3

mooffie commented 5 January 2011 at 15:49

Can't we just sort the flags in flag_features_export_render() ?

(I admit that having default flags be keyed by the flag name, as you suggest, is a more elegant solution, but your implementation has the flaw I mentioned.)

Comment #4

mooffie commented 5 January 2011 at 15:52

+ // Sort the types array for consistent exports
+ sort($new_flag['types']);

BTW, do other modules do this too? I'm asking out of curiosity only, I don't mind adding this.

Comment #5

mooffie commented 7 January 2011 at 11:57

Status	File	Size
new	flag_export_consistent_output.patch	1.66 KB

It seems we have two options:

(1) We can sort the flags in flag_features_export_render(). It's a one-line solution.

(2) We can change our export format to have the flag names be the keys of the array. The names won't repeat in the array values. I'm attaching a patch to show this (I haven't actually tested it). If this technique ($name => $info) is prevalent in the Drupal world, we should certainly use it.

Comment #6

mooffie commented 10 January 2011 at 08:59

(1) We can sort the flags in flag_features_export_render(). It's a one-line solution.

Scrap it. This won't work. Features' sanitization will override it anyway.

I've looked into how Features sanitizes arrays: _features_sanitize().

- We don't need to sort the $flag->types array because it already gets sorted.

- As for sorting the flags themselves: Features simply does sort() and although PHP has rules for comparing arrays, inconsistencies may still result when moving flags from system to another. So we have no choice but to use the flag names for the keys.

I'll commit a patch similar to #5, after I test it.