567303 - VALIDATOR_DEPENDENCIES doesn't always sort field validation properly

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Max Kanat-Alexander]

Perl uses quicksort, which of course means that it does not compare every value in an array to every other value in an array. Also, it assumes that:

A > B > C === A > C

So if it's already compared A and B, and it's already compared B and C, then it doesn't compare A and C.

In _cmp_dependency, we are currently doing a "cmp" call if we don't explicitly match a dependency. But then we get into the A, B, C situation above. For example:

bug_status > bug_severity
bug_severity > product

Thus, bug_status comes before product, even though it shouldn't.

The simple solution is to make the comparison function always sort non-depdendent fields before any fields with dependencies, because that will assure that fields with dependencies fields always have their comparison done properly against each other.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

So, I think that this will fix the problem. I'm going to do some more extensive testing to be sure.

Comment 2

[Max Kanat-Alexander]

Comment on attachment 446652 [details] [diff] [review]
v1

Okay, this doesn't work either. If I pretend like "custom" depends on "visibility_value_id" in Bugzilla::Field, it doesn't work right. I may have to write a custom sort function.

Comment 3

[Max Kanat-Alexander]

Attachment: v2

Wow, welcome to Computer Science land. Okay, here's a patch using a sorting algorithm that is not fast, but is pretty simple and only operates on small sets of data, so we're OK. In all my testing (both on paper and with actual data), this sorts things properly.

Comment 4

[Tiago Mello [:timello]]

Alright! I'll check this very soon.

Comment 5

[Max Kanat-Alexander]

Attachment: v3

Oh, um, actually *using* the function might be good, too. :-)

Comment 6

[Tiago Mello [:timello]]

Comment on attachment 446903 [details] [diff] [review]
v3

It looks good! As you mentioned, we could sort the @$has_deps alphabetically as we do for the @$no_deps for consistency.

Comment 7

[Max Kanat-Alexander]

  Thanks for the review, timello! :-)

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Object.pm
modified Bugzilla/Install/Requirements.pm
modified template/en/default/global/code-error.html.tmpl
Committed revision 7195.

Comment 8

[Tiago Mello [:timello]]

Max,

I see that you change the List::MoreUtils requirement version to 0.23 but the latest version I can see is 0.22. install-module.pl scripts says 0.22 is the latest one. Is that right?

Comment 9

[Frédéric Buclin]

right!

Comment 10

[Frédéric Buclin]

Reopening! 0.23 doesn't exist, and 0.25_02 is a dev release.

Comment 11

[Tiago Mello [:timello]]

Maybe you meant 0.13?, see:

0.13  Wed Nov  9 16:29:02 CET 2005
	- nearly all functions receiving a CODE-block as first
	  argument had a hefty memory-leak: Fixed
	  (spotted by Thomas A. Lowery <tlowery AT cc3 DOT com>)

Comment 12

[Frédéric Buclin]

(In reply to comment #11)
> Maybe you meant 0.13?

No, 0.23:

http://cpansearch.perl.org/src/VPARSEVAL/List-MoreUtils-0.25_02/Changes

 fixed: Big memory leak with XS part()

But 0.23 and 0.24 are not (no longer?) available.

Comment 13

[Max Kanat-Alexander]

  Right, they had critical bugs.

  I've emailed the maintainer to see if he is going to come out with a newer version. In the mean time, I've reduced the requirement to 0.22 so people will at least be able to run 3.7.1:

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Install/Requirements.pm
Committed revision 7199.