[pacman-dev] [PATCH 2/2] Revert "Add -S --recursive operation"

Dan McGee dan at archlinux.org
Wed Feb 15 16:47:59 EST 2012


This reverts commit f3fa77bcf1d792971c314f8c0de255866e89f3f3 along with
making other necessary changes to fully back this (mis)feature out until
we can do it correctly.

The quick summary here is this was not implemented correctly; provides
are not fully taken into account in this logic, and making that happen
exposes a lot of other flaws in this code that are covered up later on
in the dependency resolving process by several other pieces of
convoluted and conditional logic.

Tests have been adjusted accordingly. Some test EXISTS conditions have
been removed as we already know the package is installed locally, and we
also are checking the VERSION condition anyway.

With this commit, we do have some changes in test pass/fail results:

* upgrade078.py: does not pass, this is due to --recursive getting
  removed for -U/-S operations after this commit.
* sync302.py: the version checks have been disabled, so this test
  continues to pass but has been scaled back in scope.
* sync303.py: now passes, was failing before.
* sync304.py: still failing, was failing before.
* sync305.py: now passes, was failing before.
* sync306.py: still passes, was passing before.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 doc/pacman.8.txt                |    7 -------
 lib/libalpm/deps.c              |   29 -----------------------------
 src/pacman/conf.h               |    1 -
 src/pacman/pacman.c             |    8 +-------
 test/pacman/tests/sync302.py    |    8 ++++----
 test/pacman/tests/sync303.py    |    3 ---
 test/pacman/tests/sync304.py    |    2 --
 test/pacman/tests/sync305.py    |    3 ---
 test/pacman/tests/sync306.py    |    1 -
 test/pacman/tests/upgrade078.py |   17 ++++++++---------
 10 files changed, 13 insertions(+), 66 deletions(-)

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
index 275f8ac..1ec5e7c 100644
--- a/doc/pacman.8.txt
+++ b/doc/pacman.8.txt
@@ -411,13 +411,6 @@ system upgrade and install/upgrade the foo package in the same operation.
 *\--needed*::
 	Do not reinstall the targets that are already up to date.
 
-*\--recursive*::
-	Recursively reinstall all dependencies of the targets. This forces upgrades
-	or reinstalls of all dependencies without requiring explicit version
-	requirements. This is most useful in combination with the '\--needed' flag,
-	which will induce a deep dependency upgrade without any unnecessary
-	reinstalls.
-
 
 Handling Config Files[[HCF]]
 ----------------------------
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index 89f6d69..0c0a054 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -723,12 +723,6 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs,
 		return 0;
 	}
 
-	if(handle->trans->flags & ALPM_TRANS_FLAG_RECURSE) {
-		/* removing local packages from the equation causes the entire dep chain to
-		 * get pulled for each target- e.g., pactree -u output */
-		localpkgs = NULL;
-	}
-
 	/* Create a copy of the packages list, so that it can be restored
 	   on error */
 	packages_copy = alpm_list_copy(*packages);
@@ -781,29 +775,6 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs,
 		alpm_list_free(deps);
 	}
 
-	if(handle->trans->flags & ALPM_TRANS_FLAG_NEEDED) {
-		/* remove any deps that were pulled that match installed version */
-		/* odd loop syntax so we can modify the list as we iterate */
-		i = *packages;
-		while(i) {
-			alpm_pkg_t *tpkg = i->data;
-			alpm_pkg_t *local = _alpm_db_get_pkgfromcache(
-					handle->db_local, tpkg->name);
-			if(local && _alpm_pkg_compare_versions(tpkg, local) == 0) {
-				/* with the NEEDED flag, packages up to date are not reinstalled */
-				_alpm_log(handle, ALPM_LOG_DEBUG,
-						"not adding dep %s-%s as it is not needed, same version\n",
-						local->name, local->version);
-				j = i;
-				i = i->next;
-				*packages = alpm_list_remove_item(*packages, j);
-				free(j);
-			} else {
-				i = i->next;
-			}
-		}
-	}
-
 	if(ret != 0) {
 		alpm_list_free(*packages);
 		*packages = packages_copy;
diff --git a/src/pacman/conf.h b/src/pacman/conf.h
index 9e14925..42f2529 100644
--- a/src/pacman/conf.h
+++ b/src/pacman/conf.h
@@ -127,7 +127,6 @@ enum {
 	OP_ARCH,
 	OP_PRINTFORMAT,
 	OP_GPGDIR,
-	OP_RECURSIVE,
 	OP_DBONLY
 };
 
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 381e0b7..c0c3bb8 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -135,7 +135,6 @@ static void usage(int op, const char * const myname)
 		} else if(op == PM_OP_UPGRADE) {
 			printf("%s:  %s {-U --upgrade} [%s] <%s>\n", str_usg, myname, str_opt, str_file);
 			addlist(_("      --needed         do not reinstall up to date packages\n"));
-			addlist(_("      --recursive      reinstall all dependencies of target packages\n"));
 			printf("%s:\n", str_opt);
 		} else if(op == PM_OP_QUERY) {
 			printf("%s:  %s {-Q --query} [%s] [%s]\n", str_usg, myname, str_opt, str_pkg);
@@ -167,7 +166,6 @@ static void usage(int op, const char * const myname)
 			addlist(_("  -w, --downloadonly   download packages but do not install/upgrade anything\n"));
 			addlist(_("  -y, --refresh        download fresh package databases from the server\n"));
 			addlist(_("      --needed         do not reinstall up to date packages\n"));
-			addlist(_("      --recursive      reinstall all dependencies of target packages\n"));
 		} else if(op == PM_OP_DATABASE) {
 			printf("%s:  %s {-D --database} <%s> <%s>\n", str_usg, myname, str_opt, str_pkg);
 			printf("%s:\n", str_opt);
@@ -512,9 +510,6 @@ static int parsearg_remove(int opt)
 		case 'c': config->flags |= ALPM_TRANS_FLAG_CASCADE; break;
 		case 'n': config->flags |= ALPM_TRANS_FLAG_NOSAVE; break;
 		case 's':
-		case OP_RECURSIVE:
-			/* 's' is the legacy flag here, but since recursive is used in -S without
-			 * a shortopt, we need to do funky tricks */
 			if(config->flags & ALPM_TRANS_FLAG_RECURSE) {
 				config->flags |= ALPM_TRANS_FLAG_RECURSEALL;
 			} else {
@@ -537,7 +532,6 @@ static int parsearg_upgrade(int opt)
 		case OP_ASDEPS: config->flags |= ALPM_TRANS_FLAG_ALLDEPS; break;
 		case OP_ASEXPLICIT: config->flags |= ALPM_TRANS_FLAG_ALLEXPLICIT; break;
 		case OP_NEEDED: config->flags |= ALPM_TRANS_FLAG_NEEDED; break;
-		case OP_RECURSIVE: config->flags |= ALPM_TRANS_FLAG_RECURSE; break;
 		case OP_IGNORE:
 			parsearg_util_addlist(&(config->ignorepkg));
 			break;
@@ -612,6 +606,7 @@ static int parseargs(int argc, char *argv[])
 		{"print",      no_argument,       0, 'p'},
 		{"quiet",      no_argument,       0, 'q'},
 		{"root",       required_argument, 0, 'r'},
+		{"recursive",  no_argument,       0, 's'},
 		{"search",     no_argument,       0, 's'},
 		{"unrequired", no_argument,       0, 't'},
 		{"upgrades",   no_argument,       0, 'u'},
@@ -637,7 +632,6 @@ static int parseargs(int argc, char *argv[])
 		{"arch",       required_argument, 0, OP_ARCH},
 		{"print-format", required_argument, 0, OP_PRINTFORMAT},
 		{"gpgdir",     required_argument, 0, OP_GPGDIR},
-		{"recursive",  no_argument,       0, OP_RECURSIVE},
 		{"dbonly",     no_argument,       0, OP_DBONLY},
 		{0, 0, 0, 0}
 	};
diff --git a/test/pacman/tests/sync302.py b/test/pacman/tests/sync302.py
index b44aaed..78e45c3 100644
--- a/test/pacman/tests/sync302.py
+++ b/test/pacman/tests/sync302.py
@@ -38,12 +38,12 @@
 self.args = "-Su"
 
 self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pacman")
 self.addrule("PKG_VERSION=pacman|1.0-2")
 self.addrule("PKG_EXIST=glibc")
-self.addrule("PKG_VERSION=glibc|2.15-1")
 self.addrule("PKG_EXIST=curl")
-self.addrule("PKG_VERSION=curl|7.22-1")
 self.addrule("PKG_EXIST=libarchive")
-self.addrule("PKG_VERSION=libarchive|2.8.5-1")
+# TODO: when SyncFirst recursive comes back, re-enable these
+#self.addrule("PKG_VERSION=glibc|2.15-1")
+#self.addrule("PKG_VERSION=curl|7.22-1")
+#self.addrule("PKG_VERSION=libarchive|2.8.5-1")
 self.addrule("PKG_EXIST=zlib")
diff --git a/test/pacman/tests/sync303.py b/test/pacman/tests/sync303.py
index b717dd2..9d7bab5 100644
--- a/test/pacman/tests/sync303.py
+++ b/test/pacman/tests/sync303.py
@@ -29,10 +29,7 @@
 self.args = "-Su"
 
 self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pacman")
 self.addrule("PKG_VERSION=pacman|1.0-2")
 self.addrule("PKG_EXIST=glibc-awesome")
 self.addrule("PKG_VERSION=glibc-awesome|2.13-2")
 self.addrule("PKG_EXIST=zlib")
-
-self.expectfailure = True
diff --git a/test/pacman/tests/sync304.py b/test/pacman/tests/sync304.py
index 4ac1a01..18058c9 100644
--- a/test/pacman/tests/sync304.py
+++ b/test/pacman/tests/sync304.py
@@ -19,9 +19,7 @@
 self.args = "-Su"
 
 self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pacman")
 self.addrule("PKG_VERSION=pacman|4.0.1-1")
-self.addrule("PKG_EXIST=pyalpm")
 self.addrule("PKG_VERSION=pyalpm|2-1")
 
 self.expectfailure = True
diff --git a/test/pacman/tests/sync305.py b/test/pacman/tests/sync305.py
index 24fcee4..62005b5 100644
--- a/test/pacman/tests/sync305.py
+++ b/test/pacman/tests/sync305.py
@@ -61,7 +61,4 @@
 
 self.args = "-Su"
 self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pacman")
 self.addrule("PKG_VERSION=pacman|4.0.1-2")
-
-self.expectfailure = True
diff --git a/test/pacman/tests/sync306.py b/test/pacman/tests/sync306.py
index ca7a547..c7401d0 100644
--- a/test/pacman/tests/sync306.py
+++ b/test/pacman/tests/sync306.py
@@ -59,5 +59,4 @@
 
 self.args = "-S pacman"
 self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pacman")
 self.addrule("PKG_VERSION=pacman|4.0.1-2")
diff --git a/test/pacman/tests/upgrade078.py b/test/pacman/tests/upgrade078.py
index 718d587..8ef3245 100644
--- a/test/pacman/tests/upgrade078.py
+++ b/test/pacman/tests/upgrade078.py
@@ -20,15 +20,15 @@
 self.addpkg2db("local", expat_lpkg)
 
 # Sync db
-curl_sync = pmpkg("curl", "7.21.7-1")
-self.addpkg2db("sync", curl_sync)
+perl_sync = pmpkg("perl", "5.14.1-3")
+perl_sync.depends = ["glibc"]
+self.addpkg2db("sync", perl_sync)
 
 glibc_sync = pmpkg("glibc", "2.1.4-4")
 self.addpkg2db("sync", glibc_sync)
 
-perl_sync = pmpkg("perl", "5.14.1-3")
-perl_sync.depends = ["glibc"]
-self.addpkg2db("sync", perl_sync)
+curl_sync = pmpkg("curl", "7.21.7-1")
+self.addpkg2db("sync", curl_sync)
 
 expat_sync = pmpkg("expat", "2.0.1-6")
 self.addpkg2db("sync", expat_sync)
@@ -46,11 +46,10 @@
 self.addrule("PKG_DEPENDS=perl|glibc")
 self.addrule("PKG_EXIST=git")
 self.addrule("PKG_VERSION=git|1.7.6-1")
-self.addrule("PKG_EXIST=curl")
 self.addrule("PKG_VERSION=curl|7.21.7-1")
-self.addrule("PKG_EXIST=glibc")
 self.addrule("PKG_VERSION=glibc|2.1.4-4")
-self.addrule("PKG_EXIST=perl")
 self.addrule("PKG_VERSION=perl|5.14.1-3")
-self.addrule("PKG_EXIST=expat")
 self.addrule("PKG_VERSION=expat|2.0.1-6")
+
+# --recursive operation was removed for now
+self.expectfailure = True
-- 
1.7.9



More information about the pacman-dev mailing list