New code to find pkg obsolete files, with a proper kyua test.

Previous code incorrectly found obsolete files in this case:

(curpkg) - in files: /etc/foo.conf
(newpkg) - in conf_files: /etc/foo.conf

and removed this file because the code couldn't find it in the same
array. The new code fixes this case and compares the whole pkg filelist.

include/xbps_api.h.in

@@ -690,8 +690,8 @@ prop_array_t xbps_find_pkg_orphans(struct xbps_handle *xhp, prop_array_t orphans
  * a new files dictionary.
  *
  * @param[in] xhp The pointer to the xbps_handle struct.
- * @param[in] instd Installed package files dictionary (\a XBPS_PKGFILES).
+ * @param[in] instd Installed package metadata dictionary.
- * @param[in] newd New package files dictionary (provided by a binary package).
+ * @param[in] newd New package metadata dictionary.
  *
  * @return A proplib array of strings with a sorted list of obsolete files.
  */

lib/package_remove_obsoletes.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2009-2012 Juan Romero Pardines.
+ * Copyright (c) 2009-2013 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -32,22 +32,57 @@
 
 #include "xbps_api_impl.h"
 
+static prop_array_t
+merge_filelist(prop_dictionary_t d)
+{
+	prop_array_t a, result;
+	prop_dictionary_t filed;
+	size_t i;
+
+	result = prop_array_create();
+	assert(result);
+
+	if ((a = prop_dictionary_get(d, "files"))) {
+		for (i = 0; i < prop_array_count(a); i++) {
+			filed = prop_array_get(a, i);
+			prop_array_add(result, filed);
+		}
+	}
+	if ((a = prop_dictionary_get(d, "links"))) {
+		for (i = 0; i < prop_array_count(a); i++) {
+			filed = prop_array_get(a, i);
+			prop_array_add(result, filed);
+		}
+	}
+	if ((a = prop_dictionary_get(d, "conf_files"))) {
+		for (i = 0; i < prop_array_count(a); i++) {
+			filed = prop_array_get(a, i);
+			prop_array_add(result, filed);
+		}
+	}
+	if ((a = prop_dictionary_get(d, "dirs"))) {
+		for (i = 0; i < prop_array_count(a); i++) {
+			filed = prop_array_get(a, i);
+			prop_array_add(result, filed);
+		}
+	}
+
+	return result;
+}
+
 prop_array_t
 xbps_find_pkg_obsoletes(struct xbps_handle *xhp,
 			prop_dictionary_t instd,
 			prop_dictionary_t newd)
 {
-	prop_array_t array, array2, obsoletes;
+	prop_array_t instfiles, newfiles, obsoletes;
 	prop_object_t obj, obj2;
 	prop_string_t oldstr, newstr;
 	size_t i, x;
-	const char *array_str = "files";
 	const char *oldhash;
 	char *file;
 	int rv = 0;
-	bool found, dodirs, dolinks, docffiles;
+	bool found;
-
-	dodirs = dolinks = docffiles = false;
 
 	assert(prop_object_type(instd) == PROP_TYPE_DICTIONARY);
 	assert(prop_object_type(newd) == PROP_TYPE_DICTIONARY);
@@ -55,19 +90,22 @@ xbps_find_pkg_obsoletes(struct xbps_handle *xhp,
 	obsoletes = prop_array_create();
 	assert(obsoletes);
 
-again:
+	instfiles = merge_filelist(instd);
-	array = prop_dictionary_get(instd, array_str);
+	if (prop_array_count(instfiles) == 0) {
-	if (array == NULL || prop_array_count(array) == 0)
+		/* nothing to check if current pkg does not own any file */
-		goto out1;
+		prop_object_release(instfiles);
+		return obsoletes;
+	}
+	newfiles = merge_filelist(newd);
 
 	/*
 	 * Iterate over files list from installed package.
 	 */
-	for (i = 0; i < prop_array_count(array); i++) {
+	for (i = 0; i < prop_array_count(instfiles); i++) {
 		found = false;
-		obj = prop_array_get(array, i);
+		obj = prop_array_get(instfiles, i);
-		if (prop_object_type(obj) != PROP_TYPE_DICTIONARY)
+		assert(prop_object_type(obj) == PROP_TYPE_DICTIONARY);
-			continue;
+
 		oldstr = prop_dictionary_get(obj, "file");
 		if (oldstr == NULL)
 			continue;
@@ -75,10 +113,10 @@ again:
 		file = xbps_xasprintf(".%s",
 		    prop_string_cstring_nocopy(oldstr));
 
-		if ((strcmp(array_str, "files") == 0) ||
+		oldhash = NULL;
-		    (strcmp(array_str, "conf_files") == 0)) {
+		prop_dictionary_get_cstring_nocopy(obj,
-			prop_dictionary_get_cstring_nocopy(obj,
+		    "sha256", &oldhash);
-			    "sha256", &oldhash);
+		if (oldhash) {
 			rv = xbps_file_hash_check(file, oldhash);
 			if (rv == ENOENT || rv == ERANGE) {
 				/*
@@ -89,19 +127,19 @@ again:
 				continue;
 			}
 		}
-		array2 = prop_dictionary_get(newd, array_str);
+		/*
-		if (array2 && prop_array_count(array2)) {
+		 * Check if current file is available in new pkg filelist.
-			for (x = 0; x < prop_array_count(array2); x++) {
+		 */
-				obj2 = prop_array_get(array2, x);
+		for (x = 0; x < prop_array_count(newfiles); x++) {
-				newstr = prop_dictionary_get(obj2, "file");
+			obj2 = prop_array_get(newfiles, x);
-				assert(newstr);
+			newstr = prop_dictionary_get(obj2, "file");
-				/*
+			assert(newstr);
-				 * Skip files with same path.
+			/*
-				 */
+			 * Skip files with same path.
-				if (prop_string_equals(oldstr, newstr)) {
+			 */
-					found = true;
+			if (prop_string_equals(oldstr, newstr)) {
-					break;
+				found = true;
-				}
+				break;
 			}
 		}
 		if (found) {
@@ -126,26 +164,12 @@ again:
 		/*
 		 * Obsolete found, add onto the array.
 		 */
-		xbps_dbg_printf(xhp, "found obsolete: %s (%s)\n",
+		xbps_dbg_printf(xhp, "found obsolete: %s\n", file);
-		    file, array_str);
-
 		prop_array_add_cstring(obsoletes, file);
 		free(file);
 	}
-out1:
+	prop_object_release(instfiles);
-	if (!dolinks) {
+	prop_object_release(newfiles);
-		dolinks = true;
-		array_str = "links";
-		goto again;
-	} else if (!docffiles) {
-		docffiles = true;
-		array_str = "conf_files";
-		goto again;
-	} else if (!dodirs) {
-		dodirs = true;
-		array_str = "dirs";
-		goto again;
-	}
 
 	return obsoletes;
 }

tests/libxbps/Makefile

@@ -7,5 +7,6 @@ SUBDIRS += pkgpattern_match
 SUBDIRS += plist_match
 SUBDIRS += plist_match_virtual
 SUBDIRS += util
+SUBDIRS += find_pkg_obsoletes
 
 include ../../mk/subdir.mk

tests/libxbps/common/Kyuafile

@@ -5,11 +5,6 @@ test_suite("libxbps")
 atf_test_program{name="util_test"}
 atf_test_program{name="cmpver_test"}
 atf_test_program{name="pkgpattern_match_test"}
-atf_test_program{name="plist_find_dictionary_test"}
-atf_test_program{name="plist_find_array_test"}
 atf_test_program{name="plist_match_test"}
 atf_test_program{name="plist_match_virtual_test"}
-atf_test_program{name="plist_remove_test"}
+atf_test_program{name="find_pkg_obsoletes_test"}
-atf_test_program{name="plist_array_replace_test"}
-
-include("find_pkg/Kyuafile")

tests/libxbps/find_pkg_obsoletes/Makefile

@@ -0,0 +1,7 @@
+TOPDIR = ../../..
+-include $(TOPDIR)/config.mk
+
+TEST = find_pkg_obsoletes_test
+
+include ../Makefile.inc
+include $(TOPDIR)/mk/test.mk

tests/libxbps/find_pkg_obsoletes/main.c

@@ -0,0 +1,128 @@
+/*-
+ * Copyright (c) 2013 Juan Romero Pardines.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *-
+ */
+#include <atf-c.h>
+#include <xbps_api.h>
+
+static void
+append_file(prop_dictionary_t d, const char *key, const char *fpath)
+{
+	prop_array_t a;
+	prop_dictionary_t filed;
+
+	filed = prop_dictionary_create();
+	a = prop_dictionary_get(d, key);
+	if (a == NULL)  {
+		a = prop_array_create();
+		prop_dictionary_set(d, key, a);
+	}
+
+	prop_dictionary_set_cstring_nocopy(filed, "file", fpath);
+	prop_array_add(a, filed);
+}
+
+static prop_dictionary_t
+create_dict(const char *key, const char *fpath)
+{
+	prop_array_t a;
+	prop_dictionary_t d, filed;
+
+	d = prop_dictionary_create();
+	filed = prop_dictionary_create();
+	a = prop_array_create();
+
+	prop_dictionary_set_cstring_nocopy(filed, "file", fpath);
+	prop_dictionary_set_cstring_nocopy(filed, "sha256", "kjaskajsk");
+	prop_array_add(a, filed);
+	prop_dictionary_set(d, key, a);
+
+	return d;
+}
+
+ATF_TC(find_pkg_obsoletes_test);
+ATF_TC_HEAD(find_pkg_obsoletes_test, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Test xbps_find_pkg_obsoletes");
+}
+
+ATF_TC_BODY(find_pkg_obsoletes_test, tc)
+{
+	struct xbps_handle xh;
+	prop_array_t res;
+	prop_dictionary_t d1, d2;
+
+	memset(&xh, 0, sizeof(xh));
+	xh.rootdir = "/tmp";
+	xh.conffile = "/tmp/unexistent.conf";
+	ATF_REQUIRE_EQ(xbps_init(&xh), 0);
+
+        d1 = create_dict("files", "/etc/foo.conf");
+	d2 = create_dict("conf_files", "/etc/foo.conf");
+
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 0);
+
+	res = xbps_find_pkg_obsoletes(&xh, d2, d1);
+	ATF_REQUIRE_EQ(prop_array_count(res), 0);
+
+	append_file(d1, "files", "file");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 1);
+
+	append_file(d1, "conf_files", "conf_file");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 2);
+
+	append_file(d1, "links", "link");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 3);
+
+	append_file(d1, "dirs", "dir");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 4);
+
+	append_file(d2, "files", "file");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 3);
+
+	append_file(d2, "conf_files", "conf_file");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 2);
+
+	append_file(d2, "links", "link");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 1);
+
+	append_file(d2, "dirs", "dir");
+	res = xbps_find_pkg_obsoletes(&xh, d1, d2);
+	ATF_REQUIRE_EQ(prop_array_count(res), 0);
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+	ATF_TP_ADD_TC(tp, find_pkg_obsoletes_test);
+
+	return atf_no_error();
+}