lib/transaction_files.c: allow replacing file with alternative

bin/xbps-uchroot/main.c

@@ -135,7 +135,7 @@ walk_dir(const char *path,
 	struct stat sb;
 	const char *p;
 	char tmp_path[PATH_MAX] = {0};
-	int rv, i;
+	int rv = 0, i;
 
 	i = scandir(path, &list, NULL, alphasort);
 	if (i == -1) {

include/xbps_api_impl.h

@@ -160,5 +160,7 @@ xbps_array_t HIDDEN xbps_get_pkg_fulldeptree(struct xbps_handle *,
 struct xbps_repo HIDDEN *xbps_regget_repo(struct xbps_handle *,
 		const char *);
 int HIDDEN xbps_conf_init(struct xbps_handle *);
+int HIDDEN xbps_parse_alternative(char *alternative, const char *rootdir,
+		char **linkpath, char **dir, char **target);
 
 #endif /* !_XBPS_API_IMPL_H_ */

lib/package_alternatives.c

@@ -1,6 +1,7 @@
 /*-
  * Copyright (c) 2015-2019 Juan Romero Pardines.
  * Copyright (c) 2019 Duncan Overbruck.
+ * Copyright (c) 2020 Piotr Wójcik
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -153,28 +154,19 @@ create_symlinks(struct xbps_handle *xhp, xbps_array_t a, const char *grname)
 {
 	int rv;
 	unsigned int i, n;
-	char *alternative, *tok1, *tok2, *linkpath, *target, *dir, *p;
+	char *alternative, *target, *dir, *p, *linkpath;
 
 	n = xbps_array_count(a);
 
 	for (i = 0; i < n; i++) {
 		alternative = xbps_string_cstring(xbps_array_get(a, i));
+		target = linkpath = NULL;
 
-		if (!(tok1 = strtok(alternative, ":")) ||
-		    !(tok2 = strtok(NULL, ":"))) {
+		if (xbps_parse_alternative(alternative, xhp->rootdir, &linkpath, &dir, &target)) {
 			free(alternative);
 			return -1;
 		}
 
-		target = strdup(tok2);
-		dir = dirname(tok2);
-
-		/* add target dir to relative links */
-		if (tok1[0] != '/')
-			linkpath = xbps_xasprintf("%s/%s/%s", xhp->rootdir, dir, tok1);
-		else
-			linkpath = xbps_xasprintf("%s/%s", xhp->rootdir, tok1);
-
 		/* create target directory, necessary for dangling symlinks */
 		dir = xbps_xasprintf("%s/%s", xhp->rootdir, dir);
 		if (strcmp(dir, ".") && xbps_mkpath(dir, 0755) && errno != EEXIST) {
@@ -202,7 +194,7 @@ create_symlinks(struct xbps_handle *xhp, xbps_array_t a, const char *grname)
 
 		xbps_set_cb_state(xhp, XBPS_STATE_ALTGROUP_LINK_ADDED, 0, NULL,
 		    "Creating '%s' alternatives group symlink: %s -> %s",
-		    grname, tok1, target);
+		    grname, linkpath + strlen(xhp->rootdir), target);
 
 		if (target[0] == '/') {
 			p = relpath(linkpath + strlen(xhp->rootdir), target);
@@ -524,6 +516,46 @@ remove_obsoletes(struct xbps_handle *xhp, const char *pkgname, const char *pkgve
 	xbps_object_release(allkeys);
 }
 
+
+/**
+ * Computes path, parent directory and target of alternative link, based on
+ * record from index and rootdir.
+ *
+ * @param[in] alternative Alternative information from index - colon-delimited
+ * pair of relative or absolute link path and absolute target. Is passed to
+ * strtok.
+ * @param[in] rootdir Rootdir path.
+ * @param[out] linkpath Path of defined link. Starts with rootdir if it isn't
+ * NULL or empty. On success, returns memory to be freed.
+ * @param[out] dir Parent directory of linkpath. Not to be freed.
+ * @param[out] target Link target, without rootdir. On success, returns memory to
+ * be freed.
+ *
+ * @return nonzero on error, zero on success.
+ **/
+int HIDDEN
+xbps_parse_alternative(char *alternative, const char *rootdir, char **linkpath, char **dir, char **target)
+{
+	char *tok1, *tok2;
+	if (!alternative ||
+	    !(tok1 = strtok(alternative, ":")) ||
+	    !(tok2 = strtok(NULL, ":"))) {
+		return -1;
+	}
+
+	*target = strdup(tok2);
+	*dir = dirname(tok2);
+
+	/* add target dir to relative links */
+	*linkpath = xbps_xasprintf("%s%s%s%s%s",
+	    ((rootdir && *rootdir) ? rootdir : ""),
+	    ((rootdir && *rootdir) ? "/" : ""),
+	    ((tok1[0] != '/') ? *dir : ""),
+	    ((tok1[0] != '/') ? "/" : ""),
+	    tok1);
+	return 0;
+}
+
 int
 xbps_alternatives_register(struct xbps_handle *xhp, xbps_dictionary_t pkg_repod)
 {

lib/transaction_files.c

@@ -1,6 +1,7 @@
 /*-
  * Copyright (c) 2019 Juan Romero Pardines.
  * Copyright (c) 2019 Duncan Overbruck <[email protected]>.
+ * Copyright (c) 2020 Piotr Wójcik
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -38,6 +39,7 @@ enum type {
 	TYPE_DIR,
 	TYPE_FILE,
 	TYPE_CONFFILE,
+	TYPE_ALTERNATIVE,
 };
 
 struct item {
@@ -117,11 +119,12 @@ static const char *
 typestr(enum type typ)
 {
 	switch (typ) {
-	case TYPE_LINK:     return "symlink";
-	case TYPE_DIR:      return "directory";
-	case TYPE_FILE:     return "file";
-	case TYPE_CONFFILE: return "configuration file";
-	default:            return NULL;
+	case TYPE_LINK:        return "symlink";
+	case TYPE_DIR:         return "directory";
+	case TYPE_FILE:        return "file";
+	case TYPE_CONFFILE:    return "configuration file";
+	case TYPE_ALTERNATIVE: return "alternative symlink";
+	default:               return NULL;
 	}
 }
 
@@ -235,6 +238,12 @@ collect_obsoletes(struct xbps_handle *xhp)
 		}
 
 		if (item->new.type == 0) {
+			if (item->old.type == TYPE_ALTERNATIVE) {
+			    /*
+			     * Leave removing alternative to dedicated phase.
+			     */
+			    continue;
+			}
 			/*
 			 * File was removed and is not provided by any
 			 * new package.
@@ -444,6 +453,11 @@ collect_file(struct xbps_handle *xhp, const char *file, size_t size,
 			 * Multiple packages removing the same directory.
 			 */
 			return 0;
+		} else if (type == TYPE_ALTERNATIVE && item->old.type == TYPE_ALTERNATIVE) {
+			/*
+			 * Multiple packages removing the same alternative.
+			 */
+			return 0;
 		} else {
 			/*
 			 * Multiple packages removing the same file.
@@ -484,15 +498,20 @@ collect_file(struct xbps_handle *xhp, const char *file, size_t size,
 			 * Multiple packages creating the same directory.
 			 */
 			return 0;
+		} else if (type == TYPE_ALTERNATIVE && item->new.type == TYPE_ALTERNATIVE) {
+			/*
+			 * Multiple packages creating the same alternative.
+			 */
+			return 0;
 		} else {
 			/*
 			 * Multiple packages creating the same file.
 			 * This should never happen in a transaction.
 			 */
 			xbps_set_cb_state(xhp, XBPS_STATE_FILES_FAIL,
 			    EEXIST, pkgver,
-			    "%s: file `%s' already installed by package %s.",
-			    pkgver, file, item->new.pkgver);
+			    "%s: %s `%s' already installed by package %s as %s.",
+			    pkgver, typestr(type), file, item->new.pkgver, typestr(item->new.type));
 			if (xhp->flags & XBPS_FLAG_IGNORE_FILE_CONFLICTS)
 				return 0;
 
@@ -548,7 +567,7 @@ collect_file(struct xbps_handle *xhp, const char *file, size_t size,
 }
 
 static int
-collect_files(struct xbps_handle *xhp, xbps_dictionary_t d,
+collect_files(struct xbps_handle *xhp, xbps_dictionary_t d, xbps_dictionary_t alternatives,
 			const char *pkgname, const char *pkgver, unsigned int idx,
 			bool update, bool removepkg, bool preserve, bool removefile)
 {
@@ -632,6 +651,40 @@ collect_files(struct xbps_handle *xhp, xbps_dictionary_t d,
 			}
 		}
 	}
+	if (alternatives) {
+		xbps_object_iterator_t iter = NULL;
+		xbps_dictionary_keysym_t keysym = NULL;
+
+		iter = xbps_dictionary_iterator(alternatives);
+		assert(iter);
+
+		while ((keysym = xbps_object_iterator_next(iter))) {
+			a = xbps_dictionary_get_keysym(alternatives, keysym);
+			for (i = 0; i < xbps_array_count(a); i++) {
+				char *alternative, *link = NULL, *dir = NULL, *target = NULL;
+
+				alternative = xbps_string_cstring(xbps_array_get(a, i));
+				if (xbps_parse_alternative(alternative, NULL, &link, &dir, &target)) {
+					free(alternative);
+					continue;
+				}
+
+				rv = collect_file(xhp, link, 0, pkgname, pkgver, idx, NULL,
+				    TYPE_ALTERNATIVE, update, removepkg, preserve, removefile, NULL);
+				free(alternative);
+				free(link);
+				free(target);
+				if (rv == EEXIST) {
+					error = true;
+					continue;
+				} else if (rv != 0) {
+					xbps_object_iterator_release(iter);
+					goto out;
+				}
+			}
+		}
+		xbps_object_iterator_release(iter);
+	}
 
 out:
 	if (error)
@@ -644,14 +697,14 @@ static int
 collect_binpkg_files(struct xbps_handle *xhp, xbps_dictionary_t pkg_repod,
 		unsigned int idx, bool update)
 {
-	xbps_dictionary_t filesd;
+	xbps_dictionary_t filesd = NULL, propsd = NULL, alternativesd = NULL;
 	struct archive *ar = NULL;
 	struct archive_entry *entry;
 	struct stat st;
 	const char *pkgver, *pkgname;
 	char *bpkg;
 	/* size_t entry_size; */
-	int rv = 0, pkg_fd = -1;
+	int rv = 0, pkg_fd = -1, found = 0;
 
 	xbps_dictionary_get_cstring_nocopy(pkg_repod, "pkgver", &pkgver);
 	assert(pkgver);
@@ -720,15 +773,26 @@ collect_binpkg_files(struct xbps_handle *xhp, xbps_dictionary_t pkg_repod,
 				rv = EINVAL;
 				goto out;
 			}
-			rv = collect_files(xhp, filesd, pkgname, pkgver, idx,
+			found++;
+		}
+		else if ((strcmp("./props.plist", entry_pname)) == 0) {
+			propsd = xbps_archive_get_dictionary(ar, entry);
+			alternativesd = xbps_dictionary_get(propsd, "alternatives");
+			found++;
+		}
+		if (found == 2) {
+			rv = collect_files(xhp, filesd, alternativesd, pkgname, pkgver, idx,
 			    update, false, false, false);
-			xbps_object_release(filesd);
 			goto out;
 		}
 		archive_read_data_skip(ar);
 	}
 
 out:
+	if (filesd)
+	    xbps_object_release(filesd);
+	if (propsd)
+	    xbps_object_release(propsd);
 	if (pkg_fd != -1)
 		close(pkg_fd);
 	if (ar)
@@ -763,7 +827,7 @@ cleanup(void)
 int HIDDEN
 xbps_transaction_files(struct xbps_handle *xhp, xbps_object_iterator_t iter)
 {
-	xbps_dictionary_t pkgd, filesd;
+	xbps_dictionary_t pkgd, filesd, alternativesd;
 	xbps_object_t obj;
 	xbps_trans_type_t ttype;
 	const char *pkgver, *pkgname;
@@ -827,11 +891,13 @@ xbps_transaction_files(struct xbps_handle *xhp, xbps_object_iterator_t iter)
 			if (filesd == NULL) {
 				continue;
 			}
+			alternativesd = xbps_pkgdb_get_pkg(xhp, pkgname);
+			alternativesd = xbps_dictionary_get(alternativesd, "alternatives");
 
 			assert(oldpkgver);
 			xbps_set_cb_state(xhp, XBPS_STATE_FILES, 0, oldpkgver,
 			    "%s: collecting files...", oldpkgver);
-			rv = collect_files(xhp, filesd, pkgname, pkgver, idx,
+			rv = collect_files(xhp, filesd, alternativesd, pkgname, pkgver, idx,
 			    update, removepkg, preserve, true);
 			if (rv != 0)
 				goto out;

tests/xbps/libxbps/shell/obsoletefiles_test.sh

@@ -892,7 +892,6 @@ atf_test_case multiple_obsoletes_with_alternatives_unordered
 
 multiple_obsoletes_with_alternatives_unordered_head() {
 	atf_set "descr" "Multiple packages add alternative unordered"
-	atf_expect_fail "not fixed yet"
 }
 
 multiple_obsoletes_with_alternatives_unordered_body() {

tests/xbps/xbps-alternatives/main_test.sh

@@ -847,8 +847,6 @@ replace_file_with_alternative_head() {
 	atf_set "descr" "xbps-alternatives: replace file with an alternative"
 }
 replace_file_with_alternative_body() {
-	atf_expect_fail "https://github.com/void-linux/xbps/pull/185"
-
 	mkdir -p repo pkg_A_old/usr/bin pkg_A_new/usr/bin pkg_B_old/usr/bin \
 	pkg_B_new/usr/bin
 	printf 'A' > pkg_A_old/usr/bin/pkg-a-file
@@ -895,8 +893,8 @@ replace_file_with_alternative_body() {
 
 	test -L root/usr/bin/file
 	atf_check_equal $? 0
-	test "$(readlink -f root/usr/bin/file)" = "pkg-b-file"
-	atf_check_equal $? 0
+	lnk=$(readlink -f root/usr/bin/file)
+	atf_check_equal $lnk "$PWD/root/usr/bin/pkg-a-file"
 }
 
 atf_init_test_cases() {