From 39aca1bbc95b86a52f82af98ac77705816cb749e Mon Sep 17 00:00:00 2001
From: Juan RP <xtraeme@gmail.com>
Date: Fri, 12 Sep 2014 11:49:34 +0200
Subject: [PATCH] Implement shlib checks for all pkg revdeps in the
 transaction.

Added three new test cases to verify its correctness.
---
 bin/xbps-install/transaction.c            |   8 +-
 include/xbps.h.in                         |   4 +-
 lib/plist_find.c                          |   2 +
 lib/transaction_dictionary.c              |   5 +-
 lib/transaction_shlibs.c                  | 179 +++++++++++++---------
 tests/xbps/libxbps/common/Kyuafile        |   1 +
 tests/xbps/libxbps/shell/Makefile         |   1 +
 tests/xbps/libxbps/shell/update_shlibs.sh | 125 +++++++++++++++
 8 files changed, 249 insertions(+), 76 deletions(-)
 create mode 100644 tests/xbps/libxbps/shell/update_shlibs.sh

diff --git a/bin/xbps-install/transaction.c b/bin/xbps-install/transaction.c
index 13b7e669..8a9a3441 100644
--- a/bin/xbps-install/transaction.c
+++ b/bin/xbps-install/transaction.c
@@ -310,22 +310,22 @@ exec_transaction(struct xbps_handle *xhp, int maxcols, bool yes, bool drun)
 				print_array(array);
 				fprintf(stderr, "Transaction aborted due to unresolved dependencies.\n");
 			}
+		} else if (rv == ENOEXEC) {
 			array = xbps_dictionary_get(xhp->transd, "missing_shlibs");
 			if (xbps_array_count(array)) {
 				/* missing shlibs */
 				print_array(array);
 				fprintf(stderr, "Transaction aborted due to unresolved shlibs.\n");
 			}
-			goto out;
 		} else if (rv == EAGAIN) {
 			/* conflicts */
 			array = xbps_dictionary_get(xhp->transd, "conflicts");
 			print_array(array);
 			fprintf(stderr, "Transaction aborted due to conflicting packages.\n");
-			goto out;
+		} else {
+			xbps_dbg_printf(xhp, "Empty transaction dictionary: %s\n",
+			    strerror(errno));
 		}
-		xbps_dbg_printf(xhp, "Empty transaction dictionary: %s\n",
-		    strerror(errno));
 		goto out;
 	}
 	xbps_dbg_printf(xhp, "Dictionary before transaction happens:\n");
diff --git a/include/xbps.h.in b/include/xbps.h.in
index 4a6c592e..28e3de63 100644
--- a/include/xbps.h.in
+++ b/include/xbps.h.in
@@ -48,7 +48,7 @@
  *
  * This header documents the full API for the XBPS Library.
  */
-#define XBPS_API_VERSION	"20140909"
+#define XBPS_API_VERSION	"20140912"
 
 #ifndef XBPS_VERSION
  #define XBPS_VERSION		"UNSET"
@@ -1160,6 +1160,8 @@ int xbps_transaction_autoremove_pkgs(struct xbps_handle *xhp);
  *  previously called.
  * @retval ENODEV if there are missing dependencies in transaction ("missing_deps"
  *  array of strings object in xhp->transd dictionary).
+ * @retval ENOEXEC if there are unresolved shared libraries in transaction ("missing_shlibs"
+ *  array of strings object in xhp->transd dictionary).
  * @retval EAGAIN if there are package conflicts in transaction ("conflicts"
  *  array of strings object in xhp->transd dictionary).
  * @retval ENOSPC Not enough free space on target rootdir to continue with the
diff --git a/lib/plist_find.c b/lib/plist_find.c
index 8890888f..1ba298b9 100644
--- a/lib/plist_find.c
+++ b/lib/plist_find.c
@@ -228,8 +228,10 @@ vpkg_user_conf(struct xbps_handle *xhp,
 				continue;
 			}
 		}
+#ifdef FULLDEBUG
 		xbps_dbg_printf(xhp, "matched vpkg `%s' with `%s (provides %s)`\n",
 		    vpkg, pkg, vpkgver);
+#endif
 		free(vpkgname);
 		found = true;
 		break;
diff --git a/lib/transaction_dictionary.c b/lib/transaction_dictionary.c
index 208f747f..e8157cb2 100644
--- a/lib/transaction_dictionary.c
+++ b/lib/transaction_dictionary.c
@@ -290,8 +290,11 @@ xbps_transaction_prepare(struct xbps_handle *xhp)
 		xhp->transd = NULL;
 		return rv;
 	}
+	/*
+	 * Check for unresolved shared libraries.
+	 */
 	if (xbps_transaction_shlibs(xhp))
-		return ENODEV;
+		return ENOEXEC;
 
 	/*
 	 * Sort package dependencies if necessary.
diff --git a/lib/transaction_shlibs.c b/lib/transaction_shlibs.c
index 624cb34a..506d869c 100644
--- a/lib/transaction_shlibs.c
+++ b/lib/transaction_shlibs.c
@@ -43,67 +43,102 @@
  * Abort transaction if such case is found.
  */
 static bool
-shlib_in_pkgdb(struct xbps_handle *xhp, const char *pkgver, const char *shlib)
+shlib_trans_matched(struct xbps_handle *xhp, const char *pkgver, const char *shlib)
 {
-	xbps_object_iterator_t iter;
-	xbps_object_t obj;
-	bool found = false;
+	xbps_array_t unsorted, shrequires;
+	xbps_dictionary_t pkgd;
+	const char *tract;
+	char *pkgname;
 
-	if (!xbps_dictionary_count(xhp->pkgdb))
-		return found;
+	pkgname = xbps_pkg_name(pkgver);
+	assert(pkgname);
 
-	iter = xbps_dictionary_iterator(xhp->pkgdb);
-	assert(iter);
-
-	while ((obj = xbps_object_iterator_next(iter))) {
-		xbps_array_t shprovides;
-		xbps_dictionary_t pkgd;
-		const char *curpkgver;
-
-		pkgd = xbps_dictionary_get_keysym(xhp->pkgdb, obj);
-		shprovides = xbps_dictionary_get(pkgd, "shlib-provides");
-		if (!shprovides)
-			continue;
-		if (xbps_match_string_in_array(shprovides, shlib)) {
-			/* shlib matched */
-			xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &curpkgver);
-			xbps_dbg_printf(xhp, "[trans] %s requires `%s': "
-			    "matched by `%s' (pkgdb)\n", pkgver, shlib, curpkgver);
-			found = true;
-			break;
-		}
+	unsorted = xbps_dictionary_get(xhp->transd, "unsorted_deps");
+	if ((pkgd = xbps_find_pkg_in_array(unsorted, pkgname)) == NULL) {
+		free(pkgname);
+		return false;
 	}
-	xbps_object_iterator_release(iter);
-	return found;
+	free(pkgname);
+
+	xbps_dictionary_get_cstring_nocopy(pkgd, "transaction", &tract);
+	if (strcmp(tract, "update"))
+		return false;
+
+	shrequires = xbps_dictionary_get(pkgd, "shlib-requires");
+	if (!shrequires)
+		return false;
+
+	return xbps_match_string_in_array(shrequires, shlib);
 }
 
 static bool
-shlib_in_transaction(struct xbps_handle *xhp, const char *pkgver, const char *shlib)
+shlib_matched(struct xbps_handle *xhp, xbps_array_t mshlibs,
+		const char *pkgver, const char *shlib)
 {
-	xbps_array_t unsorted;
+	xbps_array_t revdeps;
+	const char *shlibver;
+	char *pkgname, *shlibname;
+	bool found = true;
 
-	unsorted = xbps_dictionary_get(xhp->transd, "unsorted_deps");
-	for (unsigned int i = 0; i < xbps_array_count(unsorted); i++) {
-		xbps_array_t shprovides;
-		xbps_object_t obj;
+	pkgname = xbps_pkg_name(pkgver);
+	assert(pkgname);
+	revdeps = xbps_pkgdb_get_pkg_revdeps(xhp, pkgname);
+	free(pkgname);
+	if (!revdeps)
+		return true;
 
-		obj = xbps_array_get(unsorted, i);
-		shprovides = xbps_dictionary_get(obj, "shlib-provides");
-		if (!shprovides)
-			continue;
-		if (xbps_match_string_in_array(shprovides, shlib)) {
-			/* shlib matched */
-			const char *curpkgver;
+	shlibver = strchr(shlib, '.');
+	shlibname = strdup(shlib);
+	shlibname[strlen(shlib) - strlen(shlibver)] = '\0';
+	assert(shlibname);
 
-			xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &curpkgver);
-			xbps_dbg_printf(xhp, "[trans] %s requires `%s': "
-			    "matched by `%s' (trans)\n", pkgver, shlib, curpkgver);
-			return true;
+	/* Iterate over its revdeps and match the provided shlib */
+	for (unsigned int i = 0; i < xbps_array_count(revdeps); i++) {
+		xbps_array_t shrequires;
+		xbps_dictionary_t pkgd;
+		const char *rpkgver;
+
+		xbps_array_get_cstring_nocopy(revdeps, i, &rpkgver);
+		pkgd = xbps_pkgdb_get_pkg(xhp, rpkgver);
+		shrequires = xbps_dictionary_get(pkgd, "shlib-requires");
+
+		for (unsigned int x = 0; x < xbps_array_count(shrequires); x++) {
+			const char *rshlib, *rshlibver;
+			char *rshlibname;
+
+			xbps_array_get_cstring_nocopy(shrequires, x, &rshlib);
+			rshlibver = strchr(rshlib, '.');
+			rshlibname = strdup(rshlib);
+			rshlibname[strlen(rshlib) - strlen(rshlibver)] = '\0';
+
+			if ((strcmp(shlibname, rshlibname) == 0) &&
+			    (strcmp(shlibver, rshlibver))) {
+				/*
+				 * The shared library version did not match the
+				 * installed pkg; find out if there's an update
+				 * in the transaction with the matching version.
+				 */
+				if (!shlib_trans_matched(xhp, rpkgver, shlib)) {
+					char *buf;
+					/* shlib not matched */
+					buf = xbps_xasprintf("%s breaks `%s' "
+					    "(needs `%s%s', got '%s')",
+					    pkgver, rpkgver, shlibname,
+					    rshlibver, shlib);
+					xbps_array_add_cstring(mshlibs, buf);
+					free(buf);
+					found = false;
+				}
+			}
+			free(rshlibname);
 		}
 	}
-	return false;
+	free(shlibname);
+
+	return found;
 }
 
+
 bool HIDDEN
 xbps_transaction_shlibs(struct xbps_handle *xhp)
 {
@@ -112,44 +147,48 @@ xbps_transaction_shlibs(struct xbps_handle *xhp)
 
 	mshlibs = xbps_dictionary_get(xhp->transd, "missing_shlibs");
 	unsorted = xbps_dictionary_get(xhp->transd, "unsorted_deps");
+
 	for (unsigned int i = 0; i < xbps_array_count(unsorted); i++) {
-		xbps_array_t shrequires;
-		xbps_object_t obj;
+		xbps_array_t shprovides;
+		xbps_object_t obj, pkgd;
 		const char *pkgver, *tract;
+		char *pkgname;
 
 		obj = xbps_array_get(unsorted, i);
 		/*
-		 * Only process pkgs that are being installed or updated.
+		 * If pkg does not have 'shlib-provides' obj, pass to next one.
+		 */
+		if ((shprovides = xbps_dictionary_get(obj, "shlib-provides")) == NULL)
+			continue;
+		/*
+		 * Only process pkgs that are being updated.
 		 */
 		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &tract);
-		if (strcmp(tract, "install") && strcmp(tract, "update"))
+		if (strcmp(tract, "update"))
 			continue;
+
 		/*
-		 * If pkg does not have 'shlib-requires' obj, pass to next one.
-		 */
-		if ((shrequires = xbps_dictionary_get(obj, "shlib-requires")) == NULL)
-			continue;
-		/*
-		 * Check if all required shlibs are provided by:
-		 * 	- an installed pkg
-		 * 	- a pkg in the transaction
+		 * If there's no change in shlib-provides, pass to next one.
 		 */
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
-		for (unsigned int x = 0; x < xbps_array_count(shrequires); x++) {
+		pkgname = xbps_pkg_name(pkgver);
+		assert(pkgname);
+		pkgd = xbps_pkgdb_get_pkg(xhp, pkgname);
+		assert(pkgd);
+		free(pkgname);
+		if (xbps_array_equals(shprovides, xbps_dictionary_get(pkgd, "shlib-provides")))
+			continue;
+
+		for (unsigned int x = 0; x < xbps_array_count(shprovides); x++) {
 			const char *shlib;
 
-			xbps_array_get_cstring_nocopy(shrequires, x, &shlib);
-			if ((!shlib_in_pkgdb(xhp, pkgver, shlib)) &&
-			    (!shlib_in_transaction(xhp, pkgver, shlib))) {
-				char *buf;
-
-				buf = xbps_xasprintf("%s: needs `%s' shlib, not"
-				    " provided by any pkg!", pkgver, shlib);
-				xbps_dbg_printf(xhp, "%s\n", buf);
-				xbps_array_add_cstring(mshlibs, buf);
-				free(buf);
+			xbps_array_get_cstring_nocopy(shprovides, x, &shlib);
+			/*
+			 * Check that all shlibs provided by this pkg are used by
+			 * its revdeps.
+			 */
+			if (!shlib_matched(xhp, mshlibs, pkgver, shlib))
 				unmatched = true;
-			}
 		}
 	}
 	return unmatched;
diff --git a/tests/xbps/libxbps/common/Kyuafile b/tests/xbps/libxbps/common/Kyuafile
index 0729f169..e605007a 100644
--- a/tests/xbps/libxbps/common/Kyuafile
+++ b/tests/xbps/libxbps/common/Kyuafile
@@ -22,6 +22,7 @@ atf_test_program{name="incorrect_deps_test"}
 atf_test_program{name="vpkg_test"}
 atf_test_program{name="install_test"}
 atf_test_program{name="preserve_files_test"}
+atf_test_program{name="update_shlibs"}
 
 include('config/Kyuafile')
 include('find_pkg_orphans/Kyuafile')
diff --git a/tests/xbps/libxbps/shell/Makefile b/tests/xbps/libxbps/shell/Makefile
index 488a18d8..37eef8f4 100644
--- a/tests/xbps/libxbps/shell/Makefile
+++ b/tests/xbps/libxbps/shell/Makefile
@@ -5,6 +5,7 @@ TESTSHELL = conf_files_test issue6_test issue18_test issue20_test remove_test
 TESTSHELL+= replace_test installmode_test obsoletefiles_test
 TESTSHELL+= issue31_test scripts_test incorrect_deps_test
 TESTSHELL+= vpkg_test install_test preserve_files_test
+TESTSHELL+= update_shlibs
 
 include ../Makefile.inc
 include $(TOPDIR)/mk/test.mk
diff --git a/tests/xbps/libxbps/shell/update_shlibs.sh b/tests/xbps/libxbps/shell/update_shlibs.sh
new file mode 100644
index 00000000..648da447
--- /dev/null
+++ b/tests/xbps/libxbps/shell/update_shlibs.sh
@@ -0,0 +1,125 @@
+#!/usr/bin/env atf-sh
+#
+# 1st case:
+#	- A is updated and contains a soname bump
+#	- A revdeps are broken
+#	- ENOEXEC expected (unresolved shlibs)
+
+atf_test_case shlib_bump
+
+shlib_bump_head() {
+	atf_set "descr" "Tests for pkg updates: update pkg with soname bump"
+}
+
+shlib_bump_body() {
+	mkdir -p repo pkg_A pkg_B
+	cd repo
+	xbps-create -A noarch -n A-1.0_1 -s "A pkg" --shlib-provides "libfoo.so.1" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-1.0_1 -s "B pkg" --dependencies "A>=0" --shlib-requires "libfoo.so.1" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yvd B
+	atf_check_equal $? 0
+
+	cd repo
+	xbps-create -A noarch -n A-2.0_1 -s "A pkg" --shlib-provides "libfoo.so.2" ../pkg_A
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	# returns ENOEXEC if there are unresolved shlibs
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yuvd A
+	atf_check_equal $? 8
+}
+
+# 2nd case:
+#	- A is updated and contains a soname bump
+#	- A revdeps are broken but there are updates in transaction
+
+atf_test_case shlib_bump_revdep_in_trans
+
+shlib_bump_revdep_in_trans_head() {
+	atf_set "descr" "Tests for pkg updates: update pkg with soname bump, updated revdep in trans"
+}
+
+shlib_bump_revdep_in_trans_body() {
+	mkdir -p repo pkg_A pkg_B
+	cd repo
+	xbps-create -A noarch -n A-1.0_1 -s "A pkg" --shlib-provides "libfoo.so.1" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-1.0_1 -s "B pkg" --dependencies "A>=1.0" --shlib-requires "libfoo.so.1" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yvd B
+	atf_check_equal $? 0
+
+	cd repo
+	xbps-create -A noarch -n A-2.0_1 -s "A pkg" --shlib-provides "libfoo.so.2" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-2.0_1 -s "B pkg" --dependencies "A>=0" --shlib-requires "libfoo.so.2" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yuvd
+	atf_check_equal $? 0
+
+	res=$(xbps-query -C empty.conf -r root -p pkgver A)
+	atf_check_equal $res A-2.0_1
+
+	res=$(xbps-query -C empty.conf -r root -p pkgver B)
+	atf_check_equal $res B-2.0_1
+}
+
+# 3rd case:
+#	- A is updated and contains a soname bump
+#	- A revdeps are broken and there are broken updates in transaction
+#	- ENOEXEC expected (unresolved shlibs)
+
+atf_test_case shlib_bump_incomplete_revdep_in_trans
+
+shlib_bump_incomplete_revdep_in_trans_head() {
+	atf_set "descr" "Tests for pkg updates: update pkg with soname bump, updated revdep in trans with unresolved shlibs"
+}
+
+shlib_bump_incomplete_revdep_in_trans_body() {
+	mkdir -p repo pkg_A pkg_B
+	cd repo
+	xbps-create -A noarch -n A-1.0_1 -s "A pkg" --shlib-provides "libfoo.so.1" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-1.0_1 -s "B pkg" --dependencies "A>=1.0" --shlib-requires "libfoo.so.1" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yvd B
+	atf_check_equal $? 0
+
+	cd repo
+	xbps-create -A noarch -n A-2.0_1 -s "A pkg" --shlib-provides "libfoo.so.2" ../pkg_A
+	atf_check_equal $? 0
+	xbps-create -A noarch -n B-1.1_1 -s "B pkg" --dependencies "A>=0" --shlib-requires "libfoo.so.1" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+	cd ..
+
+	xbps-install -C empty.conf -r root --repository=$PWD/repo -yuvd
+	atf_check_equal $? 8
+}
+
+atf_init_test_cases() {
+	atf_add_test_case shlib_bump
+	atf_add_test_case shlib_bump_incomplete_revdep_in_trans
+	atf_add_test_case shlib_bump_revdep_in_trans
+}