Conflicts: dependencies without release misfire

Bug #633208 reported by devzero2000
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Jeff Johnson
Unity Linux
Fix Released
PLD Linux
rpm (Ubuntu)

Bug Description

Posted from Anssi Hannula on rpm maint mailing list (http://<email address hidden>/msg01611.html)


Hi all!

Currently [1], if a package has
Provides: foo = 1.2
And another package has
Conflicts: foo < 1.2-1

The conflict does not trigger, as the releases will be ignored if one of the
compared EVRs does not have it. (mdv bug #55810 [2])

Another similar case is:
A: Provides: foo = 1.2
B: Requires: foo = 1.2-2 (or >= 1.2-2)
The provides will match, which it IMHO should not be doing.

One solution that may seem obvious at first is making the comparison ignore
releases only if neither has one. However, it would break a situation where
A: Provides: foo = 1.2-5
B: Requires: foo = 1.2

So, in this "requires/conflicts/etc has RPMSENSE_EQUAL on non-released EVR"
case we actually do not want to compare the releases. So, if we restrict that
case out, we get the attached patch rpm-nonreleased-compare.patch (which I
based on [3] which comes from Jeff Johnson and Per Øyvind Karlsen, just
removing cosmetic changes and changes relating to epoch handling).

However, in addition to changing the seemingly wrong behaviour of the first
case described in the beginning of this message, the following noteworthy case
is also changed:
A: Provides: foo = 1.2-1
B: Requires: foo > 1.2
Previously this didn't match, after the patch this matches.
IMHO this change in behaviour is not wanted, so the attached patch rpm-
nonreleased-compare-2.patch restricts this case out as well.

Also, I attach rpm-compare-against-norelease3.patch (that I just came up with
while writing this) which is AFAICS functionally equivalent to rpm-nonrelease-
compare-2.patch, but instead of modifying parseEVR() to return "" for
nonexistent releases, it treats empty release of a provide as "" for a
comparison in case the conflict/require/etc has a release.

I guess something like rpm-nonrelease-compare-2.patch or rpm-compare-against-
norelease3.patch should be applied (after proper testing, of course), unless
we consider the original issues not-a-bug. I don't know which approach is
preferable (or if neither is, or if the first patch with the caveat is), as
I'm not too familiar with the rpm internals.

Please comment.

Note that all the attached patches are untested for now. The original patch
[3] was tested with 4.6.0 at the time and it seemed to function expectedly
(i.e. only with the caveat described above).

[1] didn't confirm with git master, but the code seems the same

Jeff Johnson (n3npq)
Changed in rpm:
status: New → Confirmed
importance: Wishlist → Medium
assignee: nobody → Jeff Johnson (n3npq)
Changed in mandriva:
status: Unknown → Confirmed
Revision history for this message
Jeff Johnson (n3npq) wrote :

This bug was orignally identified by Mancoosi WP5 last December.

The bug is that Conflicts: assertions are not correctly verified when
the Release field is missing.

The impact is small because most RPM based distros tend
not to use Conflicts: assertions. The fact that the issue has
NEVER been reported or noticed (the bug has existed since rpm-3.0.2)
also indicates that the problem has low impact.

Adding an explicit version (and removing the missing value) is
likely the easiest solution because the behavior for Conflicts:
comparison is correct for all versions of RPM.

There are multiple patches that fix the problem attached to the Mandriva bug.

Since the root cause is behavior of a Conflicts: assertion with a missing value,
the returned boolean value is arbitrarily/conventionally defined.

The patch from if/when the necessary QA to identify affected packages
across multiple distros can be done. I personally think the problem is minor
(because there is a workaround by adding an explicit release to the comparison)
and becuase its more important to be bug-compatible with existing behavor
than to be logically correct. Behavior with missing values for any assertion checker
is always conventionally defined.

Changed in rpm:
importance: Medium → Low
status: Confirmed → Triaged
Revision history for this message
Per Øyvind Karlsen (proyvind) wrote :

From the one issue reported as a result of fixing this in Mandriva Linux, it in turn revealed a flaw in the packaging relying on what's generally considered as unexpected behaviour, which should rather be fixed in stead.
These cases are obviously few, easy to trigger and generally a result of "wrong" packaging practice that's both easy to fix and should be fixed.

For the issue that revealed the bug, it's not an issue easy to find by triggering it, and it's also a result of what would be consistent and the expected behaviour not behaving that way, which in turn makes these situations more plausible and easier hidden.

Analyzing metadata such as for whole distribution to fully investigate wouldn't hurt, and should probably be easy to do. IIRC we have some tools for these things in Mandriva Linux already, using the perl bindings also..

Jeff Johnson (n3npq)
tags: added: mandriva
Jeff Johnson (n3npq)
summary: - comparisons without release
+ Conflicts: dependencies without release misfire
Jeff Johnson (n3npq)
tags: added: conflicts dependencies
removed: comparisons release without
Revision history for this message
Patryk Zawadzki (patrys) wrote :

I was looking at our rpm-4.5 branch but unfortunately the changes do not apply cleanly. I'm not up to date with rpm internals so will leave it to someone else to fix this issue in PLD.

Jeff Johnson (n3npq)
Changed in rpm:
importance: Low → Medium
Jeff Johnson (n3npq)
affects: ubuntu → rpm (Ubuntu)
Revision history for this message
Jeff Johnson (n3npq) wrote :

Grrr, did the duplication the wrong way.

Revision history for this message
Per Øyvind Karlsen (proyvind) wrote :

So what to do for rpm5?
Okay with the following change, or should I push it under an #ifdef?

--- rpmdb/rpmevr.c~ 2010-03-06 17:20:46.000000000 +0100
+++ rpmdb/rpmevr.c 2010-10-15 20:21:28.120003324 +0200
@@ -277,6 +277,10 @@ assert(b->F[RPMEVR_D] != NULL);

     for (s = rpmEVRorder(); *s != '\0'; s++) {
        int ix;
+ if(*s == 'R' && (b->Flags & (~RPMSENSE_GREATER & RPMSENSE_EQUAL))
+ && *(b->F[RPMEVR_R]) == '\0')
+ break;
        switch ((int)*s) {
        default: continue; /*@notreached@*/ /*@switchbreak@*/break;
        case 'E': ix = RPMEVR_E; /*@switchbreak@*/break;

Revision history for this message
Jeff Johnson (n3npq) wrote :

Under an #ifdef RPM_VENDOR_MANDRIVA please ... its still not
clear (in general) whether "bug compatible" behavior or "logical/sensible behavior"
is preferred. This would _NOT_ be the first bug in RPM that gets
stuck into the concrete.

Personally, I'd like to just get through bug fixing (of the "logical/sensible" persuasion)
and get on to adding dependency ranges, and attribute qualifiers, and trigger filtering,
and file versioning, and ... enhancements.

Changed in mandriva:
status: Confirmed → Fix Released
Changed in mandriva:
importance: Unknown → Medium
status: Fix Released → Confirmed
Changed in mandriva:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.