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

@@ -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

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;
 }

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 {

qpkg/test/cyclic

@@ -0,0 +1,34 @@
+#!/bin/sh
+. ./Common
+
+###############################################################################
+
+qpkg_fail "cyclic dependency (one step)" prereq foo <<EOF
+Package: foo
+Version: 0
+Architecture: test
+Depends: foo
+Filename: foo_0_test.ipkg
+EOF
+expect <<EOF
+package foo version 0 has cyclic dependency
+EOF
+
+###############################################################################
+
+qpkg_fail "cyclic dependency (two steps)" prereq foo <<EOF
+Package: bar
+Version: 1
+Architecture: test
+Depends: foo
+Filename: bar_1_test.ipkg
+
+Package: foo
+Version: 0
+Architecture: test
+Depends: bar
+Filename: foo_0_test.ipkg
+EOF
+expect <<EOF
+package foo version 0 has cyclic dependency
+EOF