libxbps: globally check for unresolved reverse dependencies.

Close #46. See NEWS for more information.

NEWS

@@ -1,5 +1,10 @@
 xbps-0.44 (???):
 
+ * libxbps: globally check for unresolved reverse dependencies before accepting
+   the transaction. This catches more cases of broken packages due to incompatible
+   upgrades or unwanted removals. This also implements #46.
+   https://github.com/voidlinux/xbps/issues/46
+
  * xbps-query(8): in the ownedby mode, do not follow symbolic links and if the
    matching file is a symlink print its target file too:
 

bin/xbps-remove/main.c

@@ -48,7 +48,8 @@ usage(bool fail)
 	    " -C --config <dir>        Path to confdir (xbps.d)\n"
 	    " -c --cachedir <dir>      Path to cachedir\n"
 	    " -d --debug               Debug mode shown to stderr\n"
-	    " -F --force-revdeps       Force package removal even with revdeps\n"
+	    " -F --force-revdeps       Force package removal even with revdeps or\n"
+	    "                          unresolved shared libraries\n"
 	    " -f --force               Force package files removal\n"
 	    " -h --help                Print help usage\n"
 	    " -n --dry-run             Dry-run mode\n"
@@ -126,26 +127,12 @@ state_cb_rm(const struct xbps_state_cb_data *xscd, void *cbdata _unused)
 }
 
 static int
-remove_pkg(struct xbps_handle *xhp, const char *pkgname, int cols,
+remove_pkg(struct xbps_handle *xhp, const char *pkgname, bool recursive)
-	   bool recursive)
 {
-	xbps_array_t reqby;
-	const char *pkgver;
 	int rv;
 
 	rv = xbps_transaction_remove_pkg(xhp, pkgname, recursive);
 	if (rv == EEXIST) {
-		/* pkg has revdeps */
-		reqby = xbps_pkgdb_get_pkg_revdeps(xhp, pkgname);
-		printf("WARNING: %s IS REQUIRED BY %u PACKAGE%s:\n\n",
-		    pkgname, xbps_array_count(reqby),
-		    xbps_array_count(reqby) > 1 ? "S" : "");
-		for (unsigned int x = 0; x < xbps_array_count(reqby); x++) {
-			xbps_array_get_cstring_nocopy(reqby, x, &pkgver);
-			print_package_line(pkgver, cols, false);
-		}
-		printf("\n\n");
-		print_package_line(NULL, cols, true);
 		return rv;
 	} else if (rv == ENOENT) {
 		printf("Package `%s' is not currently installed.\n", pkgname);
@@ -183,14 +170,12 @@ main(int argc, char **argv)
 	struct xbps_handle xh;
 	const char *rootdir, *cachedir, *confdir;
 	int c, flags, rv;
-	bool yes, drun, recursive, ignore_revdeps, clean_cache;
+	bool yes, drun, recursive, clean_cache, orphans;
-	bool orphans, reqby_force;
 	int maxcols;
 
 	rootdir = cachedir = confdir = NULL;
 	flags = rv = 0;
-	drun = recursive = ignore_revdeps = clean_cache = false;
+	drun = recursive = clean_cache = yes = orphans = false;
-	reqby_force = yes = orphans = false;
 
 	while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) {
 		switch (c) {
@@ -204,7 +189,7 @@ main(int argc, char **argv)
 			flags |= XBPS_FLAG_DEBUG;
 			break;
 		case 'F':
-			ignore_revdeps = true;
+			flags |= XBPS_FLAG_FORCE_REMOVE_REVDEPS;
 			break;
 		case 'f':
 			flags |= XBPS_FLAG_FORCE_REMOVE_FILES;
@@ -291,20 +276,12 @@ main(int argc, char **argv)
 	}
 
 	for (int i = optind; i < argc; i++) {
-		rv = remove_pkg(&xh, argv[i], maxcols, recursive);
+		rv = remove_pkg(&xh, argv[i], recursive);
-		if (rv == 0)
+		if (rv != 0) {
-			continue;
-		else if (rv != EEXIST) {
 			xbps_end(&xh);
 			exit(rv);
-		} else {
-			reqby_force = true;
 		}
 	}
-	if (reqby_force && !ignore_revdeps && !drun) {
-		xbps_end(&xh);
-		exit(EXIT_FAILURE);
-	}
 	if (orphans || (argc > optind)) {
 		rv = exec_transaction(&xh, maxcols, yes, drun);
 	}

include/xbps.h.in

@@ -48,7 +48,7 @@
  *
  * This header documents the full API for the XBPS Library.
  */
-#define XBPS_API_VERSION	"20150111"
+#define XBPS_API_VERSION	"20150203"
 
 #ifndef XBPS_VERSION
  #define XBPS_VERSION		"UNSET"
@@ -190,6 +190,14 @@
  */
 #define XBPS_FLAG_REPOS_MEMSYNC 	0x00000400
 
+/**
+ * @def XBPS_FLAG_FORCE_REMOVE_REVDEPS
+ * Continue with transaction even if there are broken reverse
+ * dependencies, due to unresolved shared libraries or dependencies.
+ * Must be set through the xbps_handle::flags member.
+ */
+#define XBPS_FLAG_FORCE_REMOVE_REVDEPS 	0x00000800
+
 /**
  * @def XBPS_FETCH_CACHECONN
  * Default (global) limit of cached connections used in libfetch.

lib/transaction_dictionary.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2009-2014 Juan Romero Pardines.
+ * Copyright (c) 2009-2015 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -304,9 +304,13 @@ xbps_transaction_prepare(struct xbps_handle *xhp)
 	 */
 	xbps_transaction_revdeps(xhp, pkgs);
 	array = xbps_dictionary_get(xhp->transd, "missing_deps");
-	if (xbps_array_count(array))
+	if (xbps_array_count(array)) {
-		return ENODEV;
+		if (xhp->flags & XBPS_FLAG_FORCE_REMOVE_REVDEPS) {
-
+			xbps_dbg_printf(xhp, "[trans] continuing with broken reverse dependencies!");
+		} else {
+			return ENODEV;
+		}
+	}
 	for (i = 0; i < xbps_array_count(pkgs); i++)
 		xbps_pkg_find_conflicts(xhp, pkgs, xbps_array_get(pkgs, i));
 	/*
@@ -319,8 +323,13 @@ xbps_transaction_prepare(struct xbps_handle *xhp)
 	 * Check for unresolved shared libraries.
 	 */
 	if (xbps_transaction_shlibs(xhp, pkgs,
-	    xbps_dictionary_get(xhp->transd, "missing_shlibs")))
+	    xbps_dictionary_get(xhp->transd, "missing_shlibs"))) {
-		return ENOEXEC;
+		if (xhp->flags & XBPS_FLAG_FORCE_REMOVE_REVDEPS) {
+			xbps_dbg_printf(xhp, "[trans] continuing with unresolved shared libraries!");
+		} else {
+			return ENOEXEC;
+		}
+	}
 	/*
 	 * Check for packages to be replaced.
 	 */

lib/transaction_ops.c

@@ -272,7 +272,7 @@ xbps_transaction_remove_pkg(struct xbps_handle *xhp,
 			    bool recursive)
 {
 	xbps_dictionary_t pkgd;
-	xbps_array_t pkgs, orphans, orphans_pkg, reqby;
+	xbps_array_t pkgs, orphans, orphans_pkg;
 	xbps_object_t obj;
 	const char *pkgver;
 	int rv = 0;
@@ -314,16 +314,6 @@ xbps_transaction_remove_pkg(struct xbps_handle *xhp,
 			return EINVAL;
 		xbps_dbg_printf(xhp, "%s: added into transaction (remove).\n", pkgver);
 	}
-	reqby = xbps_pkgdb_get_pkg_revdeps(xhp, pkgname);
-	/*
-	 * If target pkg is required by any installed pkg, the client must be aware
-	 * of this to take appropiate action.
-	 */
-	if ((xbps_object_type(reqby) == XBPS_TYPE_ARRAY) &&
-	    (xbps_array_count(reqby) > 0))
-		rv = EEXIST;
-
-	xbps_object_release(orphans);
 	return rv;
 
 rmpkg:
@@ -335,15 +325,6 @@ rmpkg:
 	if ((rv = xbps_transaction_store(xhp, pkgs, pkgd, false)) != 0)
 		return EINVAL;
 	xbps_dbg_printf(xhp, "%s: added into transaction (remove).\n", pkgver);
-	reqby = xbps_pkgdb_get_pkg_revdeps(xhp, pkgver);
-	/*
-	 * If target pkg is required by any installed pkg, the client must be aware
-	 * of this to take appropiate action.
-	 */
-	if ((xbps_object_type(reqby) == XBPS_TYPE_ARRAY) &&
-	    (xbps_array_count(reqby) > 0))
-		rv = EEXIST;
-
 	return rv;
 }
 

lib/transaction_revdeps.c

@@ -107,28 +107,37 @@ check_virtual_pkgs(struct xbps_handle *xhp,
 	return matched;
 }
 
+static void
+broken_pkg(xbps_array_t mdeps, const char *dep, const char *pkg)
+{
+	char *str;
+
+	str = xbps_xasprintf("%s broken, needs `%s'", dep, pkg);
+	xbps_array_add_cstring(mdeps, str);
+	free(str);
+}
+
 void HIDDEN
 xbps_transaction_revdeps(struct xbps_handle *xhp, xbps_array_t pkgs)
 {
-	xbps_array_t mdeps, pkgrdeps, rundeps;
+	xbps_array_t mdeps;
-	xbps_dictionary_t revpkgd;
+
-	xbps_object_t obj;
+	mdeps = xbps_dictionary_get(xhp->transd, "missing_deps");
-	const char *pkgver, *curdep, *revpkgver, *curpkgver, *tract;
-	char *pkgname, *curdepname, *curpkgname, *str;
 
 	for (unsigned int i = 0; i < xbps_array_count(pkgs); i++) {
+		xbps_array_t pkgrdeps;
+		xbps_object_t obj;
+		const char *pkgver, *tract;
+		char *pkgname;
+
 		obj = xbps_array_get(pkgs, i);
-		/*
-		 * Only check packages in transaction being updated.
-		 */
-		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &tract);
-		if (strcmp(tract, "update"))
-			continue;
 		/*
 		 * if pkg in transaction is not installed,
 		 * pass to next one.
 		 */
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
+		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &tract);
+
 		pkgname = xbps_pkg_name(pkgver);
 		assert(pkgname);
 		if (xbps_pkg_is_installed(xhp, pkgname) == 0) {
@@ -149,15 +158,38 @@ xbps_transaction_revdeps(struct xbps_handle *xhp, xbps_array_t pkgs)
 		 * Time to validate revdeps for current pkg.
 		 */
 		for (unsigned int x = 0; x < xbps_array_count(pkgrdeps); x++) {
+			xbps_array_t rundeps;
+			xbps_dictionary_t revpkgd;
+			const char *curpkgver, *revpkgver, *curdep;
+			char *curpkgname, *curdepname;
 			bool found = false;
 
 			xbps_array_get_cstring_nocopy(pkgrdeps, x, &curpkgver);
 			revpkgd = xbps_pkgdb_get_pkg(xhp, curpkgver);
+			xbps_dictionary_get_cstring_nocopy(revpkgd, "pkgver", &revpkgver);
+			pkgname = xbps_pkg_name(curpkgver);
+			assert(pkgname);
+			/*
+			 * If target pkg is being removed, all its revdeps
+			 * will be broken unless those revdeps are also in
+			 * the transaction.
+			 */
+			if (strcmp(tract, "remove") == 0) {
+				if (xbps_find_pkg_in_array(pkgs, pkgname, "remove")) {
+					free(pkgname);
+					continue;
+				}
+				free(pkgname);
+				broken_pkg(mdeps, curpkgver, pkgver);
+				continue;
+			}
 			/*
 			 * First try to match any supported virtual package.
 			 */
-			if (check_virtual_pkgs(xhp, pkgs, obj, revpkgd))
+			if (check_virtual_pkgs(xhp, pkgs, obj, revpkgd)) {
+				free(pkgname);
 				continue;
+			}
 			/*
 			 * Try to match real dependencies.
 			 */
@@ -181,31 +213,28 @@ xbps_transaction_revdeps(struct xbps_handle *xhp, xbps_array_t pkgs)
 				}
 				free(curdepname);
 			}
-			if (!found)
+			free(curpkgname);
-				continue;
 
-			if (xbps_match_pkgdep_in_array(rundeps, pkgver))
+			if (!found) {
+				free(pkgname);
 				continue;
+			}
+			if (xbps_match_pkgdep_in_array(rundeps, pkgver)) {
+				free(pkgname);
+				continue;
+			}
 			/*
 			 * Installed package conflicts with package
 			 * in transaction being updated, check
 			 * if a new version of this conflicting package
 			 * is in the transaction.
 			 */
-			pkgname = xbps_pkg_name(curpkgver);
+			if (xbps_find_pkg_in_array(pkgs, pkgname, "update")) {
-			if (xbps_find_pkg_in_array(pkgs, pkgname, NULL)) {
 				free(pkgname);
 				continue;
 			}
 			free(pkgname);
-			mdeps = xbps_dictionary_get(xhp->transd, "missing_deps");
+			broken_pkg(mdeps, curpkgver, pkgver);
-			xbps_dictionary_get_cstring_nocopy(revpkgd,
-			    "pkgver", &revpkgver);
-			str = xbps_xasprintf("CONFLICT: `%s' "
-			    "update breaks `%s', needs `%s'",
-			    pkgver, revpkgver, curdep);
-			xbps_array_add_cstring(mdeps, str);
-			free(str);
 		}
 
 	}

tests/xbps/libxbps/shell/remove_test.sh

@@ -246,6 +246,34 @@ remove_with_revdeps_body() {
 	atf_check_equal $? 19
 }
 
+atf_test_case remove_with_revdeps_force
+
+remove_with_revdeps_force_head() {
+	atf_set "descr" "Tests for package removal: remove a pkg with revdeps (force)"
+}
+
+remove_with_revdeps_force_body() {
+	mkdir some_repo
+	mkdir -p pkg_A/usr/bin pkg_B/usr/bin
+
+	cd some_repo
+	xbps-create -A noarch -n A-1.0_1 -s "A pkg" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-1.0_1 -s "B pkg" --dependencies "A-1.0_1" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -d -a $PWD/*.xbps
+	atf_check_equal $? 0
+	cd ..
+	xbps-install -r root --repository=some_repo -yvd B
+	atf_check_equal $? 0
+	xbps-remove -r root -Fyvd A
+	atf_check_equal $? 0
+	out=$(xbps-query -r root -l|wc -l)
+	atf_check_equal $out 1
+	out=$(xbps-query -r root -ppkgver B)
+	atf_check_equal $out B-1.0_1
+}
+
 atf_test_case remove_with_revdeps_in_trans
 
 remove_with_revdeps_in_trans_head() {
@@ -329,6 +357,7 @@ atf_init_test_cases() {
 	atf_add_test_case remove_symlinks_modified
 	atf_add_test_case remove_dups
 	atf_add_test_case remove_with_revdeps
+	atf_add_test_case remove_with_revdeps_force
 	atf_add_test_case remove_with_revdeps_in_trans
 	atf_add_test_case remove_with_revdeps_in_trans_inverted
 	atf_add_test_case remove_with_revdeps_in_trans_recursive