On Tue, 29 May 2007, Michael Schroeder wrote:
> On Tue, May 29, 2007 at 10:29:45AM +0300, Panu Matilainen wrote:
>> What JBJ does here is checks HDRID for identifying identical packages if
>> present, version comparison if not. But I don't think it's actually
>> identicality of headers (or not) we're really interested in here but
>> upgradability. It's trivial to produce packages with different HDRID
>> but same NEVRA (just rebuild the package and there you have it), I don't
>> think it makes much sense to allow to packages with same NEVRA in the
>> transaction, ever.
>>
>> I'd rather just use rpmdsCompare() for the check rather than add yet
>> another codepath for NEVRA comparison within rpm like the included patch
>> does. Optionally we could check for the HDRID as well (identical packages
>> are never wanted in ts) but dunno if it's worth the couple of more lines
>> because most of the time we'd be doing the version comparison anyway.
>
> The patch is actually about something different: rpm has two modes
> of operation: rpm refresh and rpm update. rpm refresh is done
> (after the patch) when the NEVRA matches. It differs from an update
> because rpm doesn't do an uninstall in that case. As there's no
> uninstall, the code must fake it by deleting the old header from the
> database when doing the install. Unfortunatelly this is detected in a
> different way: there are some some rpmdbSetIteratorRE() calls in
> rpmpsmStage(), stage PSM_PKGINSTALL.
Um, sorry but I'm not following... where does freshen come to play in the
bug https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=104066 mentioned
in the patch? Freshen won't even touch the other package there because the
name differs...
> I think checking the HDRID is actually the right thing to do. The
> patch stems from rpm-3, where there was no HDRID. Maybe there
> should be some rpmSameHeader() function as an abstraction. The
> code in psm.c should be changed to:
>
> psm->mi = rpmtsInitIterator(ts, RPMTAG_NAME, rpmteN(psm->te), 0);
> while ((psm->oh = rpmdbNextIterator(psm->mi)) != NULL) {
> if (!rpmSameHeader(fi->h, psm->oh))
> continue;
> fi->record = rpmdbGetIteratorOffset(psm->mi);
> psm->oh = NULL;
> /*@loopbreak@*/ break;
> }
>
> This doesn't slow down the installation, as all the rpmdbSetIteratorRE()
> are currently also done after fetching the header.
That's pretty much what jbj's version of the patch does, but not in psm.c
but rpmtsAddInstallElement() much like your original patch. A thinko wrt
location / mixup with some other patch perhaps?
BTW and here's the fix from jbj's tree, and brief testing shows it also
fixes the foo-1.1 vs bar-1.1 case from #104066.
- Panu -
diff -r 8a1bbb5918b9 lib/depends.c
--- a/lib/depends.c Tue May 29 13:43:06 2007 +0300
+++ b/lib/depends.c Fri Jun 01 12:29:16 2007 +0300
@@ -123,6 +123,34 @@ static int removePackage(rpmts ts, Heade
return 0;
}
+
+static int rpmHeadersIdentical(Header first, Header second)
+ /*@*/
+{
+ const char * one, * two;
+ rpmds A, B;
+ int rc;
+
+ if (!headerGetEntry(first, RPMTAG_HDRID, NULL, (void **) &one, NULL))
+ one = NULL;
+ if (!headerGetEntry(second, RPMTAG_HDRID, NULL, (void **) &two, NULL))
+ two = NULL;
+
+ if (one && two)
+ return ((strcmp(one, two) == 0) ? 1 : 0);
+ if (one && !two)
+ return 0;
+ if (!one && two)
+ return 0;
+ /* XXX Headers w/o digests case devolves to NEVR comparison. */
+ A = rpmdsThis(first, RPMTAG_REQUIRENAME, RPMSENSE_EQUAL);
+ B = rpmdsThis(second, RPMTAG_REQUIRENAME, RPMSENSE_EQUAL);
+ rc = rpmdsCompare(A, B);
+ A = rpmdsFree(A);
+ B = rpmdsFree(B);
+ return rc;
+}
+
int rpmtsAddInstallElement(rpmts ts, Header h,
fnpyKey key, int upgrade, rpmRelocation * relocs)
@@ -308,9 +336,9 @@ addheader:
if (tscolor && hcolor && ohcolor && !(hcolor & ohcolor))
continue;
- /* Skip packages that contain identical NEVR. */
- if (rpmVersionCompare(h, oh) == 0)
- continue;
+ /* Skip identical packages. */
+ if (rpmHeadersIdentical(h, oh))
+ continue;
xx = removePackage(ts, oh, rpmdbGetIteratorOffset(mi), pkgKey);
}