From d6076553715a9f7f519547f4a51a40ad08e4acd1 Mon Sep 17 00:00:00 2001 From: Juan RP Date: Tue, 3 Feb 2015 10:20:13 +0100 Subject: [PATCH] libxbps: globally check for unresolved reverse dependencies. Close #46. See NEWS for more information. --- NEWS | 5 ++ bin/xbps-remove/main.c | 39 +++--------- include/xbps.h.in | 10 +++- lib/transaction_dictionary.c | 21 +++++-- lib/transaction_ops.c | 21 +------ lib/transaction_revdeps.c | 79 +++++++++++++++++-------- tests/xbps/libxbps/shell/remove_test.sh | 29 +++++++++ 7 files changed, 121 insertions(+), 83 deletions(-) diff --git a/NEWS b/NEWS index be5e3081..d04e9ff9 100644 --- a/NEWS +++ b/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: diff --git a/bin/xbps-remove/main.c b/bin/xbps-remove/main.c index 53db3cc1..143884a5 100644 --- a/bin/xbps-remove/main.c +++ b/bin/xbps-remove/main.c @@ -48,7 +48,8 @@ usage(bool fail) " -C --config Path to confdir (xbps.d)\n" " -c --cachedir 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, - bool recursive) +remove_pkg(struct xbps_handle *xhp, const char *pkgname, 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 orphans, reqby_force; + bool yes, drun, recursive, clean_cache, orphans; int maxcols; rootdir = cachedir = confdir = NULL; flags = rv = 0; - drun = recursive = ignore_revdeps = clean_cache = false; - reqby_force = yes = orphans = false; + drun = recursive = clean_cache = 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); - if (rv == 0) - continue; - else if (rv != EEXIST) { + rv = remove_pkg(&xh, argv[i], recursive); + if (rv != 0) { 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); } diff --git a/include/xbps.h.in b/include/xbps.h.in index 27735ea2..aebe315b 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 "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. diff --git a/lib/transaction_dictionary.c b/lib/transaction_dictionary.c index 2be598b2..7c5b2067 100644 --- a/lib/transaction_dictionary.c +++ b/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)) - return ENODEV; - + if (xbps_array_count(array)) { + 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"))) - return ENOEXEC; + xbps_dictionary_get(xhp->transd, "missing_shlibs"))) { + 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. */ diff --git a/lib/transaction_ops.c b/lib/transaction_ops.c index 60a802ea..b2e030f6 100644 --- a/lib/transaction_ops.c +++ b/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; } diff --git a/lib/transaction_revdeps.c b/lib/transaction_revdeps.c index 7883c725..393e6828 100644 --- a/lib/transaction_revdeps.c +++ b/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_dictionary_t revpkgd; - xbps_object_t obj; - const char *pkgver, *curdep, *revpkgver, *curpkgver, *tract; - char *pkgname, *curdepname, *curpkgname, *str; + xbps_array_t mdeps; + + mdeps = xbps_dictionary_get(xhp->transd, "missing_deps"); 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) - continue; + free(curpkgname); - 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, NULL)) { + if (xbps_find_pkg_in_array(pkgs, pkgname, "update")) { free(pkgname); continue; } free(pkgname); - mdeps = xbps_dictionary_get(xhp->transd, "missing_deps"); - 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); + broken_pkg(mdeps, curpkgver, pkgver); } } diff --git a/tests/xbps/libxbps/shell/remove_test.sh b/tests/xbps/libxbps/shell/remove_test.sh index 60324d86..a160a67e 100644 --- a/tests/xbps/libxbps/shell/remove_test.sh +++ b/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