Fix bug 9395 by allowing pacman to remove unresolvable packages from a transaction
Hi, forgive me if I'm doing this wrong, I'm new to git and this is the
first time I've tried to supply a git patch to this list. My patch is
attached to this email - is that the correct way to send it to the list?
Some notes on this patch:
- It is based on the current git sources instead of the release version
of the code as was my previous patch on this same topic
- It takes into consideration the following feedback from the list on my
previous patch:
- Fixed static function names (static void _alpm_xxx -> static void xxx)
- Removed the prompt that asks the user if they want to un-ignore
packages that have upgrades for the current transaction. The reasoning
is that if the user didn't want to ignore the package, they wouldn't
have put them in their IgnorePkg list, so why ask them during an
upgrade? Instead just expect the user to remove packages from IgnorePkg
that they don't actually want to ignore.
- Removed the NoIgnorePrompt configuration option that I had
previously added since instead I'm just removing that prompt altogether
- Fixed the issue where if a top-level package is depended upon by
unresolvable top-level packages, it would be removed from the
transaction, whereas really it should be retained
- Fixed the infinite loop when there are dependency loops
- Added a few new test cases to pactest: ignore001,ignore002, and
ignore003, which test some new ignore package functionality
- Changed the expected results of some existing test cases
(provision020, provision022, sync021, sync1008, and sync300) because
with the new functionality, pacman succeeds in cases where it used to
fail (simply ignoring packages which cannot be upgraded instead of
issuing an error) even though the results are the same, and also,
because the new behavior means that there is no way to sync a group that
has a package that is ignored in it (it used to work if the user said
'yes' to the prompt that I have removed, now there is no such prompt and
thus no opportunity to un-ignore the offending package)
- And one VERY IMPORTANT note: this change means that the release
version of libalpm needs to be increased, so it should be libalpm.so.4
instead of libalpm.so.3. This is because a new callback prompt was
added and an old one removed. The latter shouldn't affect existing
front-end (they just won't get asked a question that they used to get
asked), but the former will likely break any front-end by sending a
callback with a new "Transaction Conversation" identifier.
Comments welcome!
Thanks,
Bryan
From 4a9236bb54031eb4f447279ce40d86d0c6db7da5 Mon Sep 17 00:00:00 2001
From: Bryan Ischo <bryan@ischo.com>
Date: Mon, 12 Jan 2009 17:05:02 +1300
Subject: [PATCH] Changed behavior of transactions to optionally remove packages which cannot be resolved
This fixes incident 9395, by providing for a call-out from the transaction resolve step to the front-end, asking the user if they would like to ignore any packages which could not be upgraded due to unresolvable dependencies. In most cases, such dependencies are due to packages in IgnorePkg/IgnoreGroup on which packages to upgrade depend. Simply removing the offending packages from the transaction rather than failing the transaction allows such transactions to proceed and do the work that can be done. This makes managing packages with IgnorePkg/IgnoreGroup much easier. Also removed the prompt which asks the user if they'd like to not ignore packages in IgnorePkg/IgnoreGroup, because this is an unnecessary prompt - if the user wants to do this, they can do it in their /etc/pacman.conf, they don't need pacman to prompt them about it every time they do an upgrade. Finally, changed the expected behavior of some tests, to match the new transaction behavior.
+typedef struct __pkginfo_t
+{
+ /* The package for which this info is being kept */
+ pmpkg_t *pkg;
+ /* 0 if this package has been determined to be unresolvable, meaning that
+ it has dependencies that cannot be resolved, nonzero otherwise */
+ int unresolvable;
+ /* 0 if this package was not pulled, nonzero if this package was pulled */
+ int pulled;
+ /* Packages that are immediately dependent on this package. */
+ alpm_list_t *dependents;
+ /* This marker is used to detect when a dependency cycle exists */
+ int marker;
+} pkginfo_t;
+
+
+static pkginfo_t *findinfo(alpm_list_t *list, pmpkg_t *pkg)
+{
+ alpm_list_t *i;
+
+ for (i = list; i; i = i->next) {
+ pkginfo_t *info = (pkginfo_t *) i->data;
+ if (info->pkg == pkg) {
+ return info;
+ }
+ }
+
+ return NULL;
+}
+
+
+static void mark_unresolvable(alpm_list_t *list, pkginfo_t *info)
+{
+ alpm_list_t *i;
+
+ if (info->unresolvable) {
+ return;
+ }
+
+ info->unresolvable = 1;
+
+ for (i = info->dependents; i; i = i->next) {
+ mark_unresolvable(list, findinfo(list, ((pmpkg_t *) i->data)));
+ }
+}
+
+static void info_free(pkginfo_t *info)
+{
+ alpm_list_free(info->dependents);
+ free(info);
+}
+
+
+static int is_needed(alpm_list_t *infolist, pkginfo_t *info, int marker)
+{
+ if (info->unresolvable) {
+ /* Obviously if it's already been marked unresolvable, it is not
+ needed */
+ return(0);
+ } else if (!info->pulled) {
+ /* If it's top-level (not pulled), then it's needed */
+ return(1);
+ } else {
+ /* Now, if all of the top-level packages which depend on it are
+ unresolvable, then it is unneeded */
+ alpm_list_t *i;
+ for (i = info->dependents; i; i = i->next) {
+ pmpkg_t *deppkg = (pmpkg_t *) i->data;
+ if (info->marker == marker) {
+ /* This means that a dependency loop has been detected; we've
+ already marked this package meaning that it's already been
+ seen this time through. So ignore this dependency. */
+ continue;
+ }
+
+ info->marker = marker;
+
+ if (!is_needed(infolist, findinfo(infolist, deppkg), marker)) {
+ return(0);
+ }
+ }
+
+ return(1);
+ }
+}
+
+
/* populates list with packages that need to be installed to satisfy all
* dependencies of packages in list
*
* @param remove contains packages elected for removal
*/
-int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, alpm_list_t *list,
- alpm_list_t *remove, alpm_list_t **data)
+int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, alpm_list_t **list,
+ alpm_list_t **pulled, alpm_list_t *remove, alpm_list_t **data)
{
+ int marker = 0;
+ int ret = 0;
alpm_list_t *i, *j;
alpm_list_t *targ;
alpm_list_t *deps = NULL;
+ alpm_list_t *info = NULL;
+ alpm_list_t *unresolvable = NULL;
if(!(trans->flags & PM_TRANS_FLAG_NODEPS)) {
- /* store a pointer to the last original target so we can tell what was
- * pulled by resolvedeps */
- alpm_list_t *pulled = alpm_list_last(list);
+ /* resolvedeps returns a pointer to the first element of the
+ * list which is a pulled element, and all elements after that
+ * are pulled as well */
+ alpm_list_t *pulled;
/* Resolve targets dependencies */
EVENT(trans, PM_TRANS_EVT_RESOLVEDEPS_START, NULL, NULL);
_alpm_log(PM_LOG_DEBUG, "resolving target's dependencies
");
@@ -432,13 +433,13 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
}
}
- if(_alpm_resolvedeps(db_local, dbs_sync, list, remove, data) == -1) {
+ if(_alpm_resolvedeps(db_local, dbs_sync, &list, &pulled, remove, data) == -1) {
/* pm_errno is set by resolvedeps */
ret = -1;
goto cleanup;
}
- for(i = pulled->next; i; i = i->next) {
+ for(i = pulled; i; i = i->next) {
pmpkg_t *spkg = i->data;
pmsyncpkg_t *sync = _alpm_sync_new(PM_PKG_REASON_DEPEND, spkg, NULL);
if(sync == NULL) {
diff --git a/pactest/README b/pactest/README
index 5d4e47f..c7d00f7 100644
--- a/pactest/README
+++ b/pactest/README
@@ -70,12 +70,12 @@ Usage
pactest will run the suite of tests defined by the "--test" parameter.
-This example will run tests from the "test" directory.
+This example will run all tests from the "tests" directory.
Note: several "--test" options can be passed to pactest.
-Use the ""help" option to get the full list of parameters:
+Use the "help" option to get the full list of parameters:
./pactest.py --help
@@ -103,15 +103,11 @@ Example:
------
A dictionary that holds the data used in the pacman configuration file.
-It has 3 keys, each one of them pointing at a list of strings:
- - noupgrade
- - noextract
- - ignorepkg
-self.addrule("PACMAN_RETCODE=1")
+self.addrule("PACMAN_RETCODE=0")
self.addrule("!PKG_EXIST=pkg1")
self.addrule("!PKG_EXIST=pkg2")
diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 9f1cca8..e478bf5 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -247,17 +247,6 @@ void cb_trans_conv(pmtransconv_t event, void *data1, void *data2,
void *data3, int *response)
{
switch(event) {
- case PM_TRANS_CONV_INSTALL_IGNOREPKG:
- if(data2) {
- /* TODO we take this route based on data2 being not null? WTF */
- *response = yesno(_(":: %s requires installing %s from IgnorePkg/IgnoreGroup. Install anyway?"),
- alpm_pkg_get_name(data2),
- alpm_pkg_get_name(data1));
- } else {
- *response = yesno(_(":: %s is in IgnorePkg/IgnoreGroup. Install anyway?"),
- alpm_pkg_get_name(data1));
- }
- break;
case PM_TRANS_CONV_REMOVE_HOLDPKG:
*response = yesno(_(":: %s is designated as a HoldPkg. Remove anyway?"),
alpm_pkg_get_name(data1));
@@ -274,6 +263,31 @@ void cb_trans_conv(pmtransconv_t event, void *data1, void *data2,
(char *)data2,
(char *)data2);
break;
+ case PM_TRANS_CONV_REMOVE_PKGS:
+ {
+ /* Allocate a buffer big enough to hold all of the
+ package names */
+ char *packagenames;
+ alpm_list_t *unresolved = (alpm_list_t *) data1;
+ alpm_list_t *i;
+ int len = 1, /* for trailing */ where = 0, count = 0;
+ for (i = unresolved; i; i = i->next) {
+ count += 1;
+ len += 3 /* for , comma, and
*/ +
+ strlen(alpm_pkg_get_name(i->data));
+ }
+ packagenames = (char *) malloc(len);
+ for (i = unresolved; i; i = i->next) {
+ where += snprintf(&(packagenames[where]), len - where, " %s%s
",
+ alpm_pkg_get_name(i->data), (i->next) ? "," : "");
+ }
+ *response = yesno(_(":: the following package%s cannot be upgraded due to unresolvable "
+ "dependencies:
%s
Do you want to skip %s package%s for this upgrade?"),
+ (count > 1) ? "s" : "", packagenames, (count > 1) ? "these" : "this",
+ (count > 1) ? "s" : "");
+ free(packagenames);
+ }
+ break;
case PM_TRANS_CONV_LOCAL_NEWER:
if(!config->op_s_downloadonly) {
*response = yesno(_(":: %s-%s: local version is newer. Upgrade anyway?"),
--
1.6.1
_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
01-13-2009, 07:07 PM
Bryan Ischo
Fix bug 9395 by allowing pacman to remove unresolvable packages from a transaction
Xavier wrote:
On Tue, Jan 13, 2009 at 11:07 AM, Bryan Ischo
<bji-keyword-pacman.3644cb@www.ischo.com> wrote:
I'm ready to send my patches as 2 patches:
- One with everything except the removal of the "would you like to not
ignore this package that you put in your IgnorePkg/IgnoreGroup list" dialog
- One to subsequently remove said dialog
Short and simple patches are much more likely to be accepted
So I would do the short and simple patches first, and then basing the
more complex ones on them.
That way, you have more chances that at least a part of your work get merged.
I have to be honest and admit I didn't find the motivation to review
your changes. I do like the idea, but it's still a huge code addition
( lib/libalpm/deps.c | 259
+++++++++++++++++++++++++++++++++++------)
and I was hoping there would be a better and simpler way to integrate
this in the existing code.
Maybe the idea Nagy made several times before, to compute our
dependency graph once and then store it and re-use it, would help
here. But this is a huge change too
______________________________________________
OK, I'll do my best to split my patches up. But most of what I am
trying to do cannot be split up. If the number of added lines in deps.c
scares you, please understand that it's just the addition of one new
internal data structure, a few helper methods, and some moderate
re-arrangement of one existing function. If you'd apply the patch
locally, you'd be able to see how reasonably self-contained it all is.
Anyway, I'll make one more attempt and getting the patches into a form
that you'll accept.
Thanks,
Bryan
_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
Fix bug 9395 by allowing pacman to remove unresolvable packages from a transaction
