tests: new test to verify obsolete files code works in updates and reinstalls.

lib/package_unpack.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2008-2013 Juan Romero Pardines.
+ * Copyright (c) 2008-2014 Juan Romero Pardines.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -162,7 +162,7 @@ unpack_archive(struct xbps_handle *xhp,
 	       const char *fname,
 	       struct archive *ar)
 {
-	xbps_dictionary_t propsd, filesd, old_filesd;
+	xbps_dictionary_t propsd, filesd, metapropsd;
 	xbps_array_t array, obsoletes;
 	xbps_object_t obj;
 	const struct stat *entry_statp;
@@ -172,22 +172,21 @@ unpack_archive(struct xbps_handle *xhp,
 	struct archive_entry *entry;
 	size_t  instbufsiz = 0, rembufsiz = 0;
 	ssize_t entry_size;
-	unsigned int entry_idx = 0;
 	const char *file, *entry_pname, *transact,  *tgtlnk;
 	char *pkgname, *dname, *buf, *buf2, *p, *p2;
 	int ar_rv, rv, entry_type, flags;
 	bool preserve, update, conf_file, file_exists, skip_obsoletes;
-	bool skip_extract, force, metafile, xucd_stats;
+	bool skip_extract, force, xucd_stats;
 	uid_t euid;
 
-	propsd = filesd = old_filesd = NULL;
+	propsd = filesd = metapropsd = NULL;
 	force = preserve = update = conf_file = file_exists = false;
-	skip_obsoletes = metafile = xucd_stats = false;
+	skip_obsoletes = xucd_stats = false;
+	ar_rv = rv = entry_type = flags = 0;
 
 	xbps_dictionary_get_bool(pkg_repod, "preserve", &preserve);
 	xbps_dictionary_get_bool(pkg_repod, "skip-obsoletes", &skip_obsoletes);
-	xbps_dictionary_get_cstring_nocopy(pkg_repod,
+	xbps_dictionary_get_cstring_nocopy(pkg_repod, "transaction", &transact);
-	    "transaction", &transact);
 
 	euid = geteuid();
 
@@ -203,6 +202,102 @@ unpack_archive(struct xbps_handle *xhp,
 	 * Process the archive files.
 	 */
 	flags = set_extract_flags(euid);
+
+	/*
+	 * First get all metadata files on archive in this order:
+	 * 	- INSTALL
+	 * 	- REMOVE
+	 * 	- props.plist
+	 * 	- files.plist
+	 *
+	 * The XBPS package must contain props and files plists, otherwise
+	 * it's not a valid package.
+	 */
+	for (uint8_t i = 0; i < 5; i++) {
+		ar_rv = archive_read_next_header(ar, &entry);
+		if (ar_rv == ARCHIVE_EOF) {
+			rv = 0;
+			goto out;
+		} else if (ar_rv == ARCHIVE_FATAL) {
+			rv = -1;
+			goto out;
+		} else if (ar_rv == ARCHIVE_RETRY) {
+			continue;
+		}
+		entry_pname = archive_entry_pathname(entry);
+		entry_size = archive_entry_size(entry);
+
+		if (strcmp("./INSTALL", entry_pname) == 0) {
+			/*
+			 * Store file in a buffer to execute it later.
+			 */
+			instbufsiz = entry_size;
+			instbuf = malloc(entry_size);
+			assert(instbuf);
+			if (archive_read_data(ar, instbuf, entry_size) != entry_size) {
+				rv = EINVAL;
+				goto out;
+			}
+		} else if (strcmp("./REMOVE", entry_pname) == 0) {
+			/*
+			 * Store file in a buffer to execute it later.
+			 */
+			rembufsiz = entry_size;
+			rembuf = malloc(entry_size);
+			assert(rembuf);
+			if (archive_read_data(ar, rembuf, entry_size) != entry_size) {
+				rv = EINVAL;
+				goto out;
+			}
+		} else if (strcmp("./props.plist", entry_pname) == 0) {
+			propsd = xbps_archive_get_dictionary(ar, entry);
+			if (propsd == NULL) {
+				rv = errno;
+				goto out;
+			}
+		} else if (strcmp("./files.plist", entry_pname) == 0) {
+			filesd = xbps_archive_get_dictionary(ar, entry);
+			if (filesd == NULL) {
+				rv = errno;
+				goto out;
+			}
+		}
+		archive_read_data_skip(ar);
+	}
+	/*
+	 * Bail out if required metadata files are not in archive.
+	 */
+	if (propsd == NULL || filesd == NULL) {
+		xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL, ENODEV, pkgver,
+		    "%s: [unpack] invalid binary package `%s'.", pkgver, fname);
+		rv = ENODEV;
+		goto out;
+	}
+	/*
+	 * Create new metaplist file before unpacking any real file.
+	 */
+	metapropsd = xbps_pkgdb_get_pkg_metadata(xhp, pkgname);
+	rv = create_pkg_metaplist(xhp, pkgname, pkgver,
+	    propsd, filesd, instbuf, instbufsiz, rembuf, rembufsiz);
+	if (rv != 0) {
+		xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL, rv, pkgver,
+		    "%s: [unpack] failed to create metaplist file: %s",
+		    pkgver, strerror(rv));
+		goto out;
+	}
+	/*
+	 * Execute INSTALL "pre" ACTION before unpacking files.
+	 */
+	rv = xbps_pkg_exec_buffer(xhp, instbuf, instbufsiz, pkgver, "pre", update);
+	if (rv != 0) {
+		xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL, rv, pkgver,
+		    "%s: [unpack] INSTALL script failed to execute pre ACTION: %s",
+		    pkgver, strerror(rv));
+		goto out;
+	}
+	/*
+	 * Unpack all files on archive now.
+	 */
 	for (;;) {
 		ar_rv = archive_read_next_header(ar, &entry);
 		if (ar_rv == ARCHIVE_EOF || ar_rv == ARCHIVE_FATAL)
@@ -221,100 +316,6 @@ unpack_archive(struct xbps_handle *xhp,
 			archive_read_data_skip(ar);
 			continue;
 		}
-		if (strcmp("./INSTALL", entry_pname) == 0) {
-			/*
-			 * Store file in a buffer and execute
-			 * the "pre" action from it.
-			 */
-			instbufsiz = entry_size;
-			instbuf = malloc(entry_size);
-			assert(instbuf);
-
-			if (archive_read_data(ar, instbuf, entry_size) !=
-			    entry_size) {
-				rv = EINVAL;
-				goto out;
-			}
-			rv = xbps_pkg_exec_buffer(xhp, instbuf, instbufsiz,
-					pkgver, "pre", update);
-			if (rv != 0) {
-				xbps_set_cb_state(xhp,
-				    XBPS_STATE_UNPACK_FAIL,
-				    rv, pkgver,
-				    "%s: [unpack] INSTALL script failed "
-				    "to execute pre ACTION: %s",
-				    pkgver, strerror(rv));
-				goto out;
-			}
-			continue;
-
-		} else if (strcmp("./REMOVE", entry_pname) == 0) {
-			/* store file in a buffer */
-			rembufsiz = entry_size;
-			rembuf = malloc(entry_size);
-			assert(rembuf);
-			if (archive_read_data(ar, rembuf, entry_size) !=
-			    entry_size) {
-				rv = EINVAL;
-				goto out;
-			}
-			continue;
-
-		} else if (strcmp("./files.plist", entry_pname) == 0) {
-			filesd = xbps_archive_get_dictionary(ar, entry);
-			if (filesd == NULL) {
-				rv = errno;
-				goto out;
-			}
-			continue;
-		} else if (strcmp("./props.plist", entry_pname) == 0) {
-			propsd = xbps_archive_get_dictionary(ar, entry);
-			if (propsd == NULL) {
-				rv = errno;
-				goto out;
-			}
-			continue;
-		}
-		/*
-		 * If XBPS_PKGFILES or XBPS_PKGPROPS weren't found
-		 * in the archive at this phase, skip all data.
-		 */
-		if (propsd == NULL || filesd == NULL) {
-			archive_read_data_skip(ar);
-			/*
-			 * If we have processed 4 entries and the two
-			 * required metadata files weren't found, bail out.
-			 * This is not an XBPS binary package.
-			 */
-			if (entry_idx >= 3) {
-				xbps_set_cb_state(xhp,
-				    XBPS_STATE_UNPACK_FAIL, ENODEV, pkgver,
-				    "%s: [unpack] invalid binary package `%s'.",
-				    pkgver, fname);
-				rv = ENODEV;
-				goto out;
-			}
-
-			entry_idx++;
-			continue;
-		}
-		/*
-		 * XXX: duplicate code.
-		 * Create the metaplist file before unpacking any real file.
-		 */
-		if (!metafile) {
-			rv = create_pkg_metaplist(xhp, pkgname, pkgver,
-			    propsd, filesd, instbuf, instbufsiz,
-			    rembuf, rembufsiz);
-			if (rv != 0) {
-				xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL,
-				    rv, pkgver,
-				    "%s: [unpack] failed to create metaplist file: %s",
-				    pkgver, strerror(rv));
-				goto out;
-			}
-			metafile = true;
-		}
 		/*
 		 * Prepare unpack callback ops.
 		 */
@@ -491,7 +492,6 @@ unpack_archive(struct xbps_handle *xhp,
 			    "mode to %s.\n", pkgver, entry_pname,
 			    archive_entry_strmode(entry));
 		}
-
 		if (!update && conf_file && file_exists && !skip_extract) {
 			/*
 			 * If installing new package preserve old configuration
@@ -506,7 +506,6 @@ unpack_archive(struct xbps_handle *xhp,
 			    "Renamed old configuration file "
 			    "`%s' to `%s.old'.", entry_pname, entry_pname);
 		}
-
 		if (!force && skip_extract) {
 			archive_read_data_skip(ar);
 			continue;
@@ -532,22 +531,6 @@ unpack_archive(struct xbps_handle *xhp,
 			}
 		}
 	}
-	/*
-	 * XXX: duplicate code.
-	 * Create the metaplist file if it wasn't created before.
-	 */
-	if (propsd && filesd && !metafile) {
-		rv = create_pkg_metaplist(xhp, pkgname, pkgver,
-		    propsd, filesd, instbuf, instbufsiz,
-		    rembuf, rembufsiz);
-		if (rv != 0) {
-			xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL,
-			    rv, pkgver,
-			    "%s: [unpack] failed to create metaplist file: %s",
-			    pkgver, strerror(rv));
-			goto out;
-		}
-	}
 	/*
 	 * If there was any error extracting files from archive, error out.
 	 */
@@ -560,21 +543,22 @@ unpack_archive(struct xbps_handle *xhp,
 	}
 	/*
 	 * Skip checking for obsolete files on:
-	 * 	- New package installation.
 	 * 	- Package with "preserve" keyword.
 	 * 	- Package with "skip-obsoletes" keyword.
 	 */
-	if (skip_obsoletes || preserve || !update)
+	if (skip_obsoletes || preserve) {
+		xbps_dbg_printf(xhp, "%s: skipping obsoletes\n", pkgver);
 		goto out;
+	}
 	/*
 	 * Check and remove obsolete files on:
+	 * 	- Package reinstall.
 	 * 	- Package upgrade.
 	 */
-	old_filesd = xbps_pkgdb_get_pkg_metadata(xhp, pkgname);
+	if (metapropsd == NULL || !xbps_dictionary_count(metapropsd))
-	if (old_filesd == NULL)
 		goto out;
 
-	obsoletes = xbps_find_pkg_obsoletes(xhp, old_filesd, filesd);
+	obsoletes = xbps_find_pkg_obsoletes(xhp, metapropsd, filesd);
 	for (unsigned int i = 0; i < xbps_array_count(obsoletes); i++) {
 		obj = xbps_array_get(obsoletes, i);
 		file = xbps_string_cstring_nocopy(obj);
@@ -591,7 +575,7 @@ unpack_archive(struct xbps_handle *xhp,
 		    0, pkgver, "%s: removed obsolete entry: %s", pkgver, file);
 		xbps_object_release(obj);
 	}
-	xbps_object_release(old_filesd);
+	xbps_object_release(metapropsd);
 
 out:
 	if (xbps_object_type(filesd) == XBPS_TYPE_DICTIONARY)
@@ -690,8 +674,7 @@ xbps_unpack_binary_pkg(struct xbps_handle *xhp, xbps_dictionary_t pkg_repod)
 		goto out;
 	}
 	if ((rv = unpack_archive(xhp, pkg_repod, pkgver, bpkg, ar)) != 0) {
-		xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL,
+		xbps_set_cb_state(xhp, XBPS_STATE_UNPACK_FAIL, rv, pkgver,
-		    rv, pkgver,
 		    "%s: [unpack] failed to unpack files from archive: %s",
 		    pkgver, strerror(rv));
 		goto out;

tests/xbps/libxbps/common/Kyuafile

@@ -15,6 +15,7 @@ atf_test_program{name="conf_files_test"}
 atf_test_program{name="remove_test"}
 atf_test_program{name="replace_test"}
 atf_test_program{name="installmode_test"}
+atf_test_program{name="obsoletefiles_test"}
 
 include('find_pkg_orphans/Kyuafile')
 include('pkgdb/Kyuafile')

tests/xbps/libxbps/shell/Makefile

@@ -2,7 +2,7 @@ TOPDIR = ../../../..
 -include $(TOPDIR)/config.mk
 
 TESTSHELL = conf_files_test issue6_test issue18_test issue20_test remove_test
-TESTSHELL+= replace_test installmode_test
+TESTSHELL+= replace_test installmode_test obsoletefiles_test
 
 include ../Makefile.inc
 include $(TOPDIR)/mk/test.mk

tests/xbps/libxbps/shell/obsoletefiles_test.sh

@@ -0,0 +1,55 @@
+#! /usr/bin/env atf-sh
+
+# 1- find obsolete files on reinstall (and downgrades).
+atf_test_case reinstall_obsoletes
+
+reinstall_obsoletes_update_head() {
+	atf_set "descr" "Test that obsolete files are removed on reinstall"
+}
+
+reinstall_obsoletes_body() {
+	#
+	# Simulate a pkg downgrade and make sure that obsolete files
+	# are found and removed properly.
+	#
+	mkdir some_repo
+	mkdir -p pkg_A/usr/bin pkg_B/usr/sbin
+	touch -f pkg_A/usr/bin/foo pkg_A/usr/bin/blah
+	touch -f pkg_B/usr/sbin/baz
+
+	cd some_repo
+	xbps-create -A noarch -n A-1.1_1 -s "foo pkg" ../pkg_A
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+
+	cd ..
+	xbps-install -r root -C null.conf --repository=$PWD/some_repo -yv A-1.1_1
+	atf_check_equal $? 0
+
+	rm -f some_repo/*
+	cd some_repo
+	xbps-create -A noarch -n A-1.0_1 -s "foo pkg" ../pkg_B
+	atf_check_equal $? 0
+	xbps-rindex -a *.xbps
+	atf_check_equal $? 0
+
+	cd ..
+	xbps-install -r root -C null.conf --repository=$PWD/some_repo -yvf A-1.0_1
+	atf_check_equal $? 0
+
+	rv=0
+	if [ ! -f root/usr/sbin/baz ]; then
+		rv=1
+	fi
+	for f in usr/bin/foo usr/bin/blah; do
+		if [ -f root/$f ]; then
+			rv=1
+		fi
+	done
+	atf_check_equal $rv 0
+}
+
+atf_init_test_cases() {
+	atf_add_test_case reinstall_obsoletes
+}