This is an automated email from the git hooks/post-receive script. guillem pushed a commit to branch master in repository dpkg.
View the commit online: https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=7f43bf5f93c857bdb419892abfc014a5e9c3c273 commit 7f43bf5f93c857bdb419892abfc014a5e9c3c273 Author: Guillem Jover <[email protected]> AuthorDate: Tue Nov 6 03:06:33 2018 +0100 dpkg: Introduce a new dependency try level for trigger processing The introduction of dependency satisfiability for trigger processing, which was in the original spec but not the implementation, there has been countless problems with bogus detection of trigger cycles. The problem is that whenever we try to process triggers for a package, we might not be able to due to dependency unsatisfiaiblity, which means we have to put the package back into the queue. If we add the state into the cycle detection tracker, then multiple visits to these packages will hit the cycle detection for artificially generated cycles. But we cannot avoid performing the checks because that will miss dynamic cycles coming from maintainer scripts, for example. To avoid most of these problems (while possibly not fixing all potential ones), we should delay trigger processsing entirely until we have emptied the processing queue as much as possible. We do that by introducing a new dependency try level, after the dependency cycle breaking one. We will also make the trigger cycle detection unconditional of the dependency try, because for the trigproc try-queued it will not matter anymore as we will only ented on higher dependency tries, and for the other trigproc types we should not care about any queue-specific dependency try level. Closes: #810724, #854478, #911620 --- debian/changelog | 5 +++++ src/main.h | 13 +++++++++---- src/packages.c | 4 ++-- src/trigproc.c | 42 +++++++++++++++++++++++++----------------- 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/debian/changelog b/debian/changelog index f0d38635f..6b223549f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -42,6 +42,11 @@ dpkg (1.19.3) UNRELEASED; urgency=medium only reset the cycle detection in case we are not bailing out from the processing with an error, otherwise we could come back to this package and detect an artificial trigger cycle. + * dpkg: Introduce a new dependency try level for trigger processing. This + completely defers trigger processing until after the dependency cycle + breaking level, so to avoid generating artificial trigger cycles, when we + end up trying to process triggers with yet unsatisifiable dependencies. + Closes: #810724, #854478, #911620 * Perl modules: - Dpkg::Changelog::Debian: Preserve modelines at EOF. Closes: #916056 Thanks to Chris Lamb <[email protected]> for initial test cases. diff --git a/src/main.h b/src/main.h index c60ccfab0..ff6d4364b 100644 --- a/src/main.h +++ b/src/main.h @@ -238,17 +238,22 @@ void deferred_configure(struct pkginfo *pkg); * Find a cycle and break it (see above). * Do as for try 1. * - * Try 3 (only if --force-depends-version): + * Try 3: + * Start processing triggers if necessary. + * Do as for try 2. + * + * Try 4 (only if --force-depends-version): * Same as for try 2, but don't mind version number in dependencies. * - * Try 4 (only if --force-depends): + * Try 5 (only if --force-depends): * Do anyway. */ enum dependtry { DEPEND_TRY_NORMAL = 1, DEPEND_TRY_CYCLES = 2, - DEPEND_TRY_FORCE_DEPENDS_VERSION = 3, - DEPEND_TRY_FORCE_DEPENDS = 4, + DEPEND_TRY_TRIGGERS = 3, + DEPEND_TRY_FORCE_DEPENDS_VERSION = 4, + DEPEND_TRY_FORCE_DEPENDS = 5, DEPEND_TRY_LAST, }; diff --git a/src/packages.c b/src/packages.c index d12d8b728..55b56569e 100644 --- a/src/packages.c +++ b/src/packages.c @@ -236,8 +236,8 @@ void process_queue(void) { internerr("exceeded dependtry %d (sincenothing=%d; queue.length=%d)", dependtry, sincenothing, queue.length); } else if (sincenothing > queue.length * 2 + 2) { - /* XXX: This probably needs moving into a new dependtry instead. */ - if (progress_bytrigproc && progress_bytrigproc->trigpend_head) { + if (dependtry >= DEPEND_TRY_TRIGGERS && + progress_bytrigproc && progress_bytrigproc->trigpend_head) { enqueue_package(pkg); pkg = progress_bytrigproc; progress_bytrigproc = NULL; diff --git a/src/trigproc.c b/src/trigproc.c index 3836c90fa..174683348 100644 --- a/src/trigproc.c +++ b/src/trigproc.c @@ -48,10 +48,11 @@ * we add it to that queue (unless --no-triggers). * * - * We want to prefer configuring packages where possible to doing - * trigger processing, but we want to prefer trigger processing to - * cycle-breaking and dependency forcing. This is achieved as - * follows: + * We want to prefer configuring packages where possible to doing trigger + * processing, although it would be better to prefer trigger processing + * to cycle-breaking we need to do the latter first or we might generate + * artificial trigger cycles, but we always want to prefer trigger + * processing to dependency forcing. This is achieved as follows: * * Each time during configure processing where a package D is blocked by * only (ie Depends unsatisfied but would be satisfied by) a t-awaiter W @@ -59,10 +60,11 @@ * (If --no-triggers and nonempty argument list and package isn't in * argument list then we don't do this.) * - * Each time in packages.c where we increment dependtry, we instead see - * if we have encountered such a t-pending T. If we do, we trigproc T - * instead of incrementing dependtry and this counts as having done - * something so we reset sincenothing. + * Each time in process_queue() where we increment dependtry, we instead + * see if we have encountered such a t-pending T. If we do and are in + * a trigger processing try, we trigproc T instead of incrementing + * dependtry and this counts as having done something so we reset + * sincenothing. * * * For --triggers-only and --configure, we go through each thing in the @@ -390,17 +392,25 @@ trigproc(struct pkginfo *pkg, enum trigproc_type type) pkg_name(pkg, pnaw_always), pkg_status_name(pkg)); - if (dependtry >= DEPEND_TRY_CYCLES) { - gaveup = check_trigger_cycle(pkg); - if (gaveup == pkg) - return; + if (dependtry < DEPEND_TRY_TRIGGERS && + type == TRIGPROC_TRY_QUEUED) { + /* We are not yet in a triggers run, so postpone this + * package completely. */ + enqueue_package(pkg); + return; + } + if (dependtry >= DEPEND_TRY_CYCLES) { if (findbreakcycle(pkg)) sincenothing = 0; } ok = dependencies_ok(pkg, NULL, &depwhynot); if (ok == DEP_CHECK_DEFER) { + gaveup = check_trigger_cycle(pkg); + if (gaveup == pkg) + return; + varbuf_destroy(&depwhynot); enqueue_package(pkg); return; @@ -435,11 +445,9 @@ trigproc(struct pkginfo *pkg, enum trigproc_type type) varbuf_destroy(&depwhynot); } - if (dependtry < DEPEND_TRY_CYCLES) { - gaveup = check_trigger_cycle(pkg); - if (gaveup == pkg) - return; - } + gaveup = check_trigger_cycle(pkg); + if (gaveup == pkg) + return; printf(_("Processing triggers for %s (%s) ...\n"), pkg_name(pkg, pnaw_nonambig), -- Dpkg.Org's dpkg
