[Yum-devel] [PATCH] Fix 0 vs None Epoch handling in compareEVR().

[Panu Matilainen]

On Mon, 14 Jun 2010, seth vidal wrote:

> On Mon, 2010-06-14 at 19:26 +0300, Panu Matilainen wrote:
>
>> Well, current compareEVR() behavior in the first case is just plain wrong.
>> Maybe this doesn't come up in yum/repomd context, but if somebody uses
>> compareEVR() as a convenience wrapper (which it basically is) over
>> labelCompare() when working with actual headers it can bite you.
>
> That's great - it's still odd we've not ever seen a bug report on it
> afaict.
>
> which is my point. I don't like it when I hear the words:
> "obviously wrong" and "plain wrong" and no one can find a bug that
> anyone has seen to PROVE this point.

It's quite possible it simply does not come up in the yum codebase, due to 
other code arranging it not to happen, including repomd containing zeros 
for non-existent epochs.

But for a convenience wrapper function to return something fundamentally 
different than the function its wrapping is "obviously wrong" in my book:

>>> compareEVR(('0', '1', '1'), (None, '1', '1'))
1
>>> labelCompare(('0', '1', '1'), (None, '1', '1'))
0

> It gets my hackles up that we're fixing things that might be in a "I've
> been relying on that bug" place.
>
> but I read the example cases and I said "sure let's give the patch a
> shot and see what happens".
>
> hopefully nothing happens, but I'm not that optimistic of a person.

IMO if something relies on the current compareEVR() behavior wrt 0 vs None 
epoch then it's about time we fix that something instead. The issue is 
getting a bit long in the tooth, 8 years and still it keeps popping up 
every now and then.

Oh and FWIW, that you actually need a convenience wrapper around
labelCompare() is just plain silly. labelCompare() should perform the 
legitimate conversions as needed by itself. Will fix, but of course before 
that's available widely enough for yum to use... oh well.

 	- Panu -