From f17aa638649fb8de730fecdbc906dc869b626ba5 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 16 Nov 2021 11:49:18 +0200 Subject: [PATCH 1/2] Fix spurious %transfiletriggerpostun execution (RhBug:2023311) If a package has multiple %transfiletriggerpostun triggers, any one of them matching would cause all of them to run, due to disconnect in the intel gathering stage: we'd gather all the headers with matching files into a lump, and then add any postun triggers found in them, but this loses the triggering file information and causes all postuns to run. The triggers need to be added while looping over the file matches, like runFileTriggers() does. Doing so actually simplifies the code. These should really be unified to use the same code, but leaving that exercise to another rainy day. --- lib/rpmtriggers.c | 64 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/lib/rpmtriggers.c b/lib/rpmtriggers.c index 0827af0c2..dc457f7cc 100644 --- a/lib/rpmtriggers.c +++ b/lib/rpmtriggers.c @@ -97,19 +97,37 @@ static void rpmtriggersSortAndUniq(rpmtriggers trigs) } } +static void addTriggers(rpmts ts, Header trigH, rpmsenseFlags filter) +{ + int tix = 0; + rpmds ds; + rpmds triggers = rpmdsNew(trigH, RPMTAG_TRANSFILETRIGGERNAME, 0); + + while ((ds = rpmdsFilterTi(triggers, tix))) { + if ((rpmdsNext(ds) >= 0) && (rpmdsFlags(ds) & filter)) { + struct rpmtd_s priorities; + + if (headerGet(trigH, RPMTAG_TRANSFILETRIGGERPRIORITIES, + &priorities, HEADERGET_MINMEM)) { + rpmtdSetIndex(&priorities, tix); + rpmtriggersAdd(ts->trigs2run, headerGetInstance(trigH), + tix, *rpmtdGetUint32(&priorities)); + } + } + rpmdsFree(ds); + tix++; + } + rpmdsFree(triggers); +} + void rpmtriggersPrepPostUnTransFileTrigs(rpmts ts, rpmte te) { - rpmdbMatchIterator mi; rpmdbIndexIterator ii; - Header trigH; const void *key; size_t keylen; rpmfiles files; - rpmds rpmdsTriggers; - rpmds rpmdsTrigger; ii = rpmdbIndexIteratorInit(rpmtsGetRdb(ts), RPMDBI_TRANSFILETRIGGERNAME); - mi = rpmdbNewIterator(rpmtsGetRdb(ts), RPMDBI_PACKAGES); files = rpmteFiles(te); /* Iterate over file triggers in rpmdb */ @@ -121,39 +139,19 @@ void rpmtriggersPrepPostUnTransFileTrigs(rpmts ts, rpmte te) rpmfi fi = rpmfilesFindPrefix(files, pfx); while (rpmfiNext(fi) >= 0) { if (RPMFILE_IS_INSTALLED(rpmfiFState(fi))) { - /* If yes then store it */ - rpmdbAppendIterator(mi, rpmdbIndexIteratorPkgOffsets(ii), - rpmdbIndexIteratorNumPkgs(ii)); - break; + unsigned int npkg = rpmdbIndexIteratorNumPkgs(ii); + const unsigned int *offs = rpmdbIndexIteratorPkgOffsets(ii); + /* Save any matching postun triggers */ + for (int i = 0; i < npkg; i++) { + Header h = rpmdbGetHeaderAt(rpmtsGetRdb(ts), offs[i]); + addTriggers(ts, h, RPMSENSE_TRIGGERPOSTUN); + headerFree(h); + } } } rpmfiFree(fi); } rpmdbIndexIteratorFree(ii); - - if (rpmdbGetIteratorCount(mi)) { - /* Filter triggers and save only trans postun triggers into ts */ - while ((trigH = rpmdbNextIterator(mi)) != NULL) { - int tix = 0; - rpmdsTriggers = rpmdsNew(trigH, RPMTAG_TRANSFILETRIGGERNAME, 0); - while ((rpmdsTrigger = rpmdsFilterTi(rpmdsTriggers, tix))) { - if ((rpmdsNext(rpmdsTrigger) >= 0) && - (rpmdsFlags(rpmdsTrigger) & RPMSENSE_TRIGGERPOSTUN)) { - struct rpmtd_s priorities; - - headerGet(trigH, RPMTAG_TRANSFILETRIGGERPRIORITIES, - &priorities, HEADERGET_MINMEM); - rpmtdSetIndex(&priorities, tix); - rpmtriggersAdd(ts->trigs2run, rpmdbGetIteratorOffset(mi), - tix, *rpmtdGetUint32(&priorities)); - } - rpmdsFree(rpmdsTrigger); - tix++; - } - rpmdsFree(rpmdsTriggers); - } - } - rpmdbFreeIterator(mi); rpmfilesFree(files); } -- 2.35.1 From e617e7c550d3523998707c55f96b37ede2c48c78 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Wed, 2 Feb 2022 13:46:23 +0200 Subject: [PATCH 2/2] Really fix spurious %transfiletriggerpostun execution (RhBug:2023311) Commit b3d672a5523dfec033160e5cc866432a0e808649 got the base reasoning in the ballpark but the code all wrong, introducing a severe performance regression without actually fixing what it claimed to. The missing incredient is actually comparing the current prefix with the triggers in matched package (trying to describe this makes my head spin): a package may have multiple triggers on multiple prefixes and we need to make sure we only execute triggers of this type, from this prefix. This stuff really needs more and better testcases. Fixes: b3d672a5523dfec033160e5cc866432a0e808649 --- lib/rpmtriggers.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/rpmtriggers.c b/lib/rpmtriggers.c index dc457f7cc..c652981be 100644 --- a/lib/rpmtriggers.c +++ b/lib/rpmtriggers.c @@ -97,14 +97,16 @@ static void rpmtriggersSortAndUniq(rpmtriggers trigs) } } -static void addTriggers(rpmts ts, Header trigH, rpmsenseFlags filter) +static void addTriggers(rpmts ts, Header trigH, rpmsenseFlags filter, + const char *prefix) { int tix = 0; rpmds ds; rpmds triggers = rpmdsNew(trigH, RPMTAG_TRANSFILETRIGGERNAME, 0); while ((ds = rpmdsFilterTi(triggers, tix))) { - if ((rpmdsNext(ds) >= 0) && (rpmdsFlags(ds) & filter)) { + if ((rpmdsNext(ds) >= 0) && (rpmdsFlags(ds) & filter) && + strcmp(prefix, rpmdsN(ds)) == 0) { struct rpmtd_s priorities; if (headerGet(trigH, RPMTAG_TRANSFILETRIGGERPRIORITIES, @@ -141,12 +143,13 @@ void rpmtriggersPrepPostUnTransFileTrigs(rpmts ts, rpmte te) if (RPMFILE_IS_INSTALLED(rpmfiFState(fi))) { unsigned int npkg = rpmdbIndexIteratorNumPkgs(ii); const unsigned int *offs = rpmdbIndexIteratorPkgOffsets(ii); - /* Save any matching postun triggers */ + /* Save any postun triggers matching this prefix */ for (int i = 0; i < npkg; i++) { Header h = rpmdbGetHeaderAt(rpmtsGetRdb(ts), offs[i]); - addTriggers(ts, h, RPMSENSE_TRIGGERPOSTUN); + addTriggers(ts, h, RPMSENSE_TRIGGERPOSTUN, pfx); headerFree(h); } + break; } } rpmfiFree(fi); -- 2.35.1