From 0bc4b6046baa19f393ea3ebd6064178a080cfe35 Mon Sep 17 00:00:00 2001 From: Werner Almesberger Date: Sun, 21 Nov 2010 20:31:20 -0300 Subject: [PATCH] qpkg: added detection of cyclic dependencies We define a cyclic dependency as the possibility (!) of the prerequisites of a package X including a package that depends on X, and issue an error if encountering such a situation. Note that, if the dependencies of X can be resolved in a manner that does not include the cyclic dependency, qpkg will still fail if it encounters the cycle. Also note that qpkg (at least so far) does no perform an exhaustive search to ferret out cyclic dependencies. Furthermore, we don't consider that a cyclic dependency may not necessarily imply a real life problem. E.g., if a package A contains elements X and Y, with X needing package B, and the content of package B has a run-time dependency on Y, the cyclic dependency between A and B would not exist when considering its constituents. Since we don't have this information, we just err on the side of caution. - qpkg.h (enum flags): divide flags into categories (parse-time and run-time) and add flag QPKG_ADDING to mark packets whose dependencies we are processing - prereq.c (resolve, prereq): track which packages we're tentatively considering for installation and detect cyclic dependencies - test/cyclic: regression test for detection of cyclic dependencies - TODO: updated with recent changes --- qpkg/TODO | 4 ++++ qpkg/prereq.c | 53 +++++++++++++++++++++++++++++++++++++++++++----- qpkg/qpkg.h | 4 ++++ qpkg/test/cyclic | 34 +++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 5 deletions(-) create mode 100755 qpkg/test/cyclic diff --git a/qpkg/TODO b/qpkg/TODO index d1a2792..7de42d7 100644 --- a/qpkg/TODO +++ b/qpkg/TODO @@ -13,6 +13,8 @@ Still left to do - consider Architecture: + Update: we parse and record it now but don't use it yet. + - what to do with explicit and implicit replacement ? - if we can't resolve the prerequisites, give at least a hint of what one @@ -20,6 +22,8 @@ Still left to do - check database for internal consistency + Update: added detection of cyclic dependencies (in progress) + - implement keyword search - consider also supporting the similar but not identical (parent ?) format diff --git a/qpkg/prereq.c b/qpkg/prereq.c index cf9c7bd..6dc98d4 100644 --- a/qpkg/prereq.c +++ b/qpkg/prereq.c @@ -28,6 +28,11 @@ struct list { struct list *next; }; +struct stack { + struct pkg *pkg; + struct stack *next; +}; + static struct pkg **best = NULL; static struct pkg **installs = NULL; @@ -119,7 +124,6 @@ static void append_install(struct pkg *pkg) perror("realloc"); exit(1); } - } installs[n_install++] = pkg; assert(!pkg->mark && !(pkg->flags & QPKG_INSTALLED)); @@ -168,13 +172,14 @@ static int conflicts(const struct pkg *pkg, const struct list *conf) static void resolve(struct list *next_dep, const struct ref *dep, - struct list *conf) + struct stack *top, struct list *conf) { static int level = 0; struct list more_deps; struct list more_conf = { .next = conf }; + struct stack stack; struct pkg *pkg; while (!dep) { @@ -182,6 +187,9 @@ static void resolve(struct list *next_dep, const struct ref *dep, done(); return; } + assert(top->pkg->flags & QPKG_ADDING); + top->pkg->flags &= ~QPKG_ADDING; + top = top->next; dep = next_dep->refs; next_dep = next_dep->next; } @@ -189,10 +197,17 @@ static void resolve(struct list *next_dep, const struct ref *dep, if (best && n_install == n_best) return; if (debug) { + struct stack *p; + fprintf(stderr, "%*s", level, ""); fprintf(stderr, "%.*s %p", ID2PF(pkg->id), pkg); if (pkg->version) fprintf(stderr, " %.*s", ID2PF(pkg->version)); + fprintf(stderr, " ("); + for (p = top; p; p = p->next) + fprintf(stderr, "%s%.*s", + p == top ? "" : " ", ID2PF(p->pkg->id)); + fprintf(stderr, ")"); if (pkg->mark) fprintf(stderr, " +"); if (pkg->flags & QPKG_INSTALLED) @@ -201,9 +216,15 @@ static void resolve(struct list *next_dep, const struct ref *dep, } if (!satisfies(pkg, dep)) continue; + if (pkg->flags & QPKG_ADDING) { + fprintf(stderr, "package %.*s version %.*s " + "has cyclic dependency\n", + ID2PF(pkg->id), ID2PF(pkg->version)); + exit(1); + } if ((pkg->flags & QPKG_INSTALLED) || pkg->mark) { assert(!conflicts(pkg, conf)); - resolve(next_dep, dep->next, conf); + resolve(next_dep, dep->next, top, conf); continue; } if (conflicts(pkg, conf)) @@ -214,17 +235,39 @@ static void resolve(struct list *next_dep, const struct ref *dep, more_deps.next = next_dep; more_conf.refs = pkg->conflicts; more_conf.next = conf; - resolve(&more_deps, pkg->depends, &more_conf); + stack.pkg = pkg; + stack.next = top; + pkg->flags |= QPKG_ADDING; + resolve(&more_deps, pkg->depends, &stack, &more_conf); backtrack(); level--; } + + /* + * @@@ this shouldn't be all of the story yet. if we fail a dependency + * due to a conflict, the current algorithm may leave packages marked + * as QPKG_ADDING, possibly triggering a false cyclic dependency error. + * + * In the case if cycle detection, we could avoid all the hassle of + * maintaining this flag and just walk down the stack to see if we're + * already trying to add the package (the stack should be correct with + * the current algorithm), but we'll need this kind of accurate + * tracking of package state for ordering the dependencies and for + * "Provides" anyway. + */ } struct pkg **prereq(struct pkg *pkg) { + struct stack stack = { + .pkg = pkg, + .next = NULL + }; /* @@@ make list of pre-existing conflicts */ - resolve(NULL, pkg->depends, NULL); + pkg->flags |= QPKG_ADDING; + resolve(NULL, pkg->depends, &stack, NULL); + pkg->flags &= ~QPKG_ADDING; free(installs); return best; } diff --git a/qpkg/qpkg.h b/qpkg/qpkg.h index a554577..62147c8 100644 --- a/qpkg/qpkg.h +++ b/qpkg/qpkg.h @@ -14,7 +14,11 @@ #define QPKG_H enum flags { + /* parse-time flags */ QPKG_INSTALLED = 1 << 0, /* installed on target */ + + /* run-time flags */ + QPKG_ADDING = 1 << 10, /* resolving dependencies */ }; enum relop { diff --git a/qpkg/test/cyclic b/qpkg/test/cyclic new file mode 100755 index 0000000..1a0de04 --- /dev/null +++ b/qpkg/test/cyclic @@ -0,0 +1,34 @@ +#!/bin/sh +. ./Common + +############################################################################### + +qpkg_fail "cyclic dependency (one step)" prereq foo <