xbps_get_pkg_fulldeptree(): fix all known bugs and make it 50x faster.

faster: use a hash table with pkg names on the transaction dict,
 the process of collecting and sorting is now 50x faster or
 even more (kde5).

bugs: this now detects cyclic deps and returns with an appropropiate
 return value: ELOOP and ENOENT in xbps-query(1) --fulldeptree.
 Ping me if you need more details :-)

Close https://github.com/void-linux/xbps/issues/16
Close https://github.com/void-linux/xbps/issues/5

lib/package_fulldeptree.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2014-2015 Juan Romero Pardines.
+ * Copyright (c) 2014-2019 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -31,58 +31,132 @@
 
 #include "xbps_api_impl.h"
 
-struct pkgdep {
+struct item;
-	SLIST_ENTRY(pkgdep) pkgdep_entries;
+
-	const char *pkg;
+struct depn {
+	struct depn *dnext;
+	struct item *item;
+};
+
+struct item {
+	struct item *hnext;
+	struct item *bnext;
+	struct depn *dbase;
+	char *pkgn;
+	const char *pkgver;
 	xbps_array_t rdeps;
 };
 
-static SLIST_HEAD(pkgdep_head, pkgdep) pkgdep_list =
+#define ITHSIZE	1024
-    SLIST_HEAD_INITIALIZER(pkgdep_list);
+#define ITHMASK	(ITHSIZE - 1)
 
-static xbps_dictionary_t pkgdep_pvmap;
+static struct item *ItemHash[ITHSIZE];
+static xbps_array_t result;
 
 static int
-collect_rdeps(struct xbps_handle *xhp, xbps_dictionary_t pkgd, bool rpool)
+itemhash(const char *pkgn)
 {
-	xbps_array_t rdeps, currdeps, provides = NULL;
+	int hv = 0xA1B5F342;
-	struct pkgdep *pd;
+	int i;
-	const char *pkgver;
+
+	assert(pkgn);
+
+	for (i = 0; pkgn[i]; ++i)
+		hv = (hv << 5) ^ (hv >> 23) ^ pkgn[i];
+
+	return hv & ITHMASK;
+}
+
+static struct item *
+lookupItem(const char *pkgn)
+{
+	struct item *item;
+
+	assert(pkgn);
+
+	for (item = ItemHash[itemhash(pkgn)]; item; item = item->hnext) {
+		if (strcmp(pkgn, item->pkgn) == 0)
+			return item;
+	}
+	return NULL;
+}
+
+static struct item *
+addItem(xbps_array_t rdeps, const char *pkgn)
+{
+	struct item **itemp;
+	struct item *item = calloc(sizeof(*item), 1);
+
+	assert(pkgn);
+	assert(item);
+
+	itemp = &ItemHash[itemhash(pkgn)];
+	item->hnext = *itemp;
+	item->pkgn = strdup(pkgn);
+	assert(item->pkgn);
+	item->rdeps = xbps_array_copy(rdeps);
+	*itemp = item;
+
+	return item;
+}
+
+static void
+addDepn(struct item *item, struct item *xitem)
+{
+	struct depn *depn = calloc(sizeof(*depn), 1);
+
+	assert(item);
+	assert(xitem);
+
+	depn->item = item;
+	depn->dnext = xitem->dbase;
+	xitem->dbase = depn;
+}
+
+/*
+ * Recursively calculate all dependencies.
+ */
+static struct item *
+ordered_depends(struct xbps_handle *xhp, xbps_dictionary_t pkgd, bool rpool)
+{
+	xbps_array_t rdeps, provides;
+	xbps_string_t str;
+	struct item *item, *xitem;
+	const char *pkgver;
+	char *pkgn;
+
+	assert(xhp);
+	assert(pkgd);
 
-	xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &pkgver);
-	assert(pkgver);
 	rdeps = xbps_dictionary_get(pkgd, "run_depends");
 	provides = xbps_dictionary_get(pkgd, "provides");
+	xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &pkgver);
+
+	pkgn = xbps_pkg_name(pkgver);
+	assert(pkgn);
+	item = addItem(rdeps, pkgn);
+	item->pkgver = pkgver;
+	assert(item);
+	free(pkgn);
 
 	for (unsigned int i = 0; i < xbps_array_count(rdeps); i++) {
 		xbps_dictionary_t curpkgd;
-		const char *curdep, *curpkgver;
+		const char *curdep;
 		char *curdepname;
-		bool virtual = false, found = false;
 
 		xbps_array_get_cstring_nocopy(rdeps, i, &curdep);
 		if (rpool) {
-			if ((curpkgd = xbps_rpool_get_pkg(xhp, curdep)) == NULL) {
+			if ((curpkgd = xbps_rpool_get_pkg(xhp, curdep)) == NULL)
 				curpkgd = xbps_rpool_get_virtualpkg(xhp, curdep);
-				virtual = true;
-			}
 		} else {
-			if ((curpkgd = xbps_pkgdb_get_pkg(xhp, curdep)) == NULL) {
+			if ((curpkgd = xbps_pkgdb_get_pkg(xhp, curdep)) == NULL)
 				curpkgd = xbps_pkgdb_get_virtualpkg(xhp, curdep);
-				virtual = true;
-			}
 		}
-		if (curpkgd == NULL) {
+		assert(curpkgd);
-			xbps_dbg_printf(xhp, "%s: cannot find `%s' dependency\n",
+		if ((curdepname = xbps_pkgpattern_name(curdep)) == NULL)
-			    __func__, curdep);
+			curdepname = xbps_pkg_name(curdep);
-			return ENOENT;
-		}
-		if (((curdepname = xbps_pkgpattern_name(curdep)) == NULL) &&
-		    ((curdepname = xbps_pkg_name(curdep)) == NULL))
-			return EINVAL;
 
-		xbps_dictionary_get_cstring_nocopy(curpkgd, "pkgver", &curpkgver);
+		assert(curdepname);
-		currdeps = xbps_dictionary_get(curpkgd, "run_depends");
 
 		if (provides && xbps_match_pkgname_in_array(provides, curdepname)) {
 			xbps_dbg_printf(xhp, "%s: ignoring dependency %s "
@@ -90,98 +164,30 @@ collect_rdeps(struct xbps_handle *xhp, xbps_dictionary_t pkgd, bool rpool)
 			free(curdepname);
 			continue;
 		}
-		if (virtual) {
+		xitem = lookupItem(curdepname);
-			xbps_dictionary_set_cstring_nocopy(pkgdep_pvmap, curdepname, pkgver);
+		if (xitem == NULL)
-		}
+			xitem = ordered_depends(xhp, curpkgd, rpool);
+
+		assert(xitem);
+		addDepn(item, xitem);
 		free(curdepname);
-		/* uniquify dependencies, sorting will be done later */
-		SLIST_FOREACH(pd, &pkgdep_list, pkgdep_entries) {
-			if (strcmp(pd->pkg, curpkgver) == 0) {
-				found = true;
-				break;
-			}
-		}
-		if (!found) {
-			pd = malloc(sizeof(*pd));
-			assert(pd);
-			pd->pkg = curpkgver;
-			pd->rdeps = xbps_array_copy(currdeps);
-			SLIST_INSERT_HEAD(&pkgdep_list, pd, pkgdep_entries);
-			if (xbps_array_count(currdeps)) {
-				int rv;
-
-				if ((rv = collect_rdeps(xhp, curpkgd, rpool)) != 0)
-					return rv;
-			}
-		}
 	}
-	return 0;
+	/* all deps were processed, add item to head */
-}
+	str = xbps_string_create_cstring(item->pkgver);
+	assert(str);
+	xbps_array_add_first(result, str);
+	xbps_object_release(str);
 
-static xbps_array_t
+	return item;
-sortfulldeptree(void)
-{
-	struct pkgdep *pd;
-	xbps_array_t result;
-	unsigned int ndeps = 0;
-
-	result = xbps_array_create();
-	assert(result);
-
-	SLIST_FOREACH(pd, &pkgdep_list, pkgdep_entries) {
-		if (!pd->rdeps) {
-			xbps_array_add_cstring_nocopy(result, pd->pkg);
-			SLIST_REMOVE(&pkgdep_list, pd, pkgdep, pkgdep_entries);
-		}
-		ndeps++;
-	}
-	while (xbps_array_count(result) < ndeps) {
-		bool found = false;
-
-		SLIST_FOREACH(pd, &pkgdep_list, pkgdep_entries) {
-			unsigned int i = 0, mdeps = 0, rdeps = 0;
-
-			rdeps = xbps_array_count(pd->rdeps);
-			for (i = 0; i < rdeps; i++) {
-				const char *pkgdep;
-				char *pkgname;
-
-				xbps_array_get_cstring_nocopy(pd->rdeps, i, &pkgdep);
-				if (((pkgname = xbps_pkgpattern_name(pkgdep)) == NULL) &&
-				    ((pkgname = xbps_pkg_name(pkgdep)) == NULL))
-					return NULL;
-
-				if (xbps_match_pkgname_in_array(result, pkgname)) {
-					mdeps++;
-					free(pkgname);
-					continue;
-				}
-				if (xbps_dictionary_get(pkgdep_pvmap, pkgname)) {
-					mdeps++;
-					free(pkgname);
-					continue;
-				}
-				free(pkgname);
-			}
-			if (mdeps == rdeps) {
-				found = true;
-				break;
-			}
-		}
-		if (found && !xbps_match_string_in_array(result, pd->pkg)) {
-			xbps_array_add_cstring_nocopy(result, pd->pkg);
-			SLIST_REMOVE(&pkgdep_list, pd, pkgdep, pkgdep_entries);
-			free(pd);
-		}
-	}
-	return result;
 }
 
 xbps_array_t HIDDEN
 xbps_get_pkg_fulldeptree(struct xbps_handle *xhp, const char *pkg, bool rpool)
 {
 	xbps_dictionary_t pkgd;
-	int rv;
+
+	result = xbps_array_create();
+	assert(result);
 
 	if (rpool) {
 		if (((pkgd = xbps_rpool_get_pkg(xhp, pkg)) == NULL) &&
@@ -192,11 +198,7 @@ xbps_get_pkg_fulldeptree(struct xbps_handle *xhp, const char *pkg, bool rpool)
 		    ((pkgd = xbps_pkgdb_get_virtualpkg(xhp, pkg)) == NULL))
 			return NULL;
 	}
-	if (pkgdep_pvmap == NULL)
+	(void)ordered_depends(xhp, pkgd, rpool);
-		pkgdep_pvmap = xbps_dictionary_create();
 
-	if ((rv = collect_rdeps(xhp, pkgd, rpool)) != 0)
+	return result;
-		return NULL;
-
-	return sortfulldeptree();
 }

tests/xbps/libxbps/find_pkg_orphans/main.c

@@ -28,19 +28,19 @@
 
 static const char expected_output[] =
 	"xbps-git-20130310_2\n"
-	"xbps-triggers-1.0_1\n"
 	"libxbps-git-20130310_2\n"
 	"confuse-2.7_2\n"
-	"proplib-0.6.3_1\n"
 	"libarchive-3.1.2_1\n"
-	"libfetch-2.34_1\n"
 	"bzip2-1.0.5_1\n"
 	"liblzma-5.0.4_3\n"
 	"expat-2.1.0_3\n"
 	"attr-2.4.46_5\n"
+	"proplib-0.6.3_1\n"
+	"libfetch-2.34_1\n"
 	"libssl-1.0.1e_3\n"
 	"zlib-1.2.7_1\n"
-	"glibc-2.20_1\n";
+	"glibc-2.20_1\n"
+	"xbps-triggers-1.0_1\n";
 
 static const char expected_output_all[] =
 	"unexistent-pkg-0_1\n"

tests/xbps/libxbps/shell/cyclic_deps.sh

@@ -61,8 +61,6 @@ cyclic_dep_full_head() {
 }
 
 cyclic_dep_full_body() {
-	atf_set "timeout" 5
-	atf_expect_timeout "Known bug: see https://github.com/voidlinux/xbps/issues/92"
 	mkdir some_repo
 	mkdir -p pkg_A/usr/bin pkg_B/usr/bin
 	cd some_repo
@@ -86,5 +84,5 @@ cyclic_dep_full_body() {
 atf_init_test_cases() {
 	atf_add_test_case cyclic_dep_vpkg
 	atf_add_test_case cyclic_dep_vpkg2
-	#atf_add_test_case cyclic_dep_full
+	atf_add_test_case cyclic_dep_full
 }