[pacman-dev] [PATCH] pacman: list all unknown targets on removal operation
On a removal operation, pacman currently reports an error for the package that is not found in the database and then exists. Adjust so that all unknown packages are reported. Before:
pacman -R foo bar error: 'foo': target not found
After:
pacman -R foo bar error: 'foo': target not found error: 'bar': target not found
Signed-off-by: Allan McRae <allan@archlinux.org> --- src/pacman/remove.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/pacman/remove.c b/src/pacman/remove.c index f56c1ec..b6da7c3 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -104,10 +104,13 @@ int pacman_remove(alpm_list_t *targets) } if(remove_target(targ) == -1) { retval = 1; - goto cleanup; } } + if(retval == 1) { + goto cleanup; + } + /* Step 2: prepare the transaction based on its type, targets and flags */ if(alpm_trans_prepare(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); -- 1.7.8
On 10/12/11 20:10, Allan McRae wrote:
On a removal operation, pacman currently reports an error for the package that is not found in the database and then exists. Adjust so that all unknown packages are reported.
Before:
pacman -R foo bar error: 'foo': target not found
After:
pacman -R foo bar error: 'foo': target not found error: 'bar': target not found
Signed-off-by: Allan McRae <allan@archlinux.org> ---
BTW, I would like something similar for -S operations too: # pacman -S foo bar error: target not found: foo However, I can not see how to do that (without passing some value around to lots of functions) while avoiding the potential group selection dialog if we did something like: # pacman -S foo base bar Any pointers would be appreciated. Allan
On Sat, Dec 10, 2011 at 5:12 AM, Allan McRae <allan@archlinux.org> wrote:
On 10/12/11 20:10, Allan McRae wrote:
On a removal operation, pacman currently reports an error for the package that is not found in the database and then exists. Adjust so that all unknown packages are reported.
Before:
pacman -R foo bar error: 'foo': target not found
After:
pacman -R foo bar error: 'foo': target not found error: 'bar': target not found
Signed-off-by: Allan McRae <allan@archlinux.org> ---
BTW, I would like something similar for -S operations too:
# pacman -S foo bar error: target not found: foo
However, I can not see how to do that (without passing some value around to lots of functions) while avoiding the potential group selection dialog if we did something like:
# pacman -S foo base bar
Any pointers would be appreciated.
-U would be nice too; might be similar to your -S case mentioned above. $ sudo ./src/pacman/pacman -U foo bar baz Password: loading packages... error: 'foo': could not find or read package I'll take a quick look at this to see what I can come up with. -Dan
If an early target fails, we stopped processing the rest of the list. We should continue all the way through and show relevant errors for each target if possible, and error out only at the end. We do process all targets to check for URLs first and will error out if some could not be processed; we then do a second loop and try to load each target specified on the command line. This mirrors a patch by Allan to do the same for removal operations. Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/upgrade.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c index 0ca6fec..650af6b 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -41,6 +41,7 @@ */ int pacman_upgrade(alpm_list_t *targets) { + int retval = 0; alpm_list_t *i; alpm_siglevel_t level = alpm_option_get_default_siglevel(config->handle); @@ -57,7 +58,7 @@ int pacman_upgrade(alpm_list_t *targets) if(str == NULL) { pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", (char *)i->data, alpm_strerror(alpm_errno(config->handle))); - return 1; + retval = 1; } else { free(i->data); i->data = str; @@ -65,6 +66,10 @@ int pacman_upgrade(alpm_list_t *targets) } } + if(retval) { + return retval; + } + /* Step 1: create a new transaction */ if(trans_init(config->flags, 1) == -1) { return 1; @@ -79,19 +84,24 @@ int pacman_upgrade(alpm_list_t *targets) if(alpm_pkg_load(config->handle, targ, 1, level, &pkg) != 0) { pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", targ, alpm_strerror(alpm_errno(config->handle))); - trans_release(); - return 1; + retval = 1; + continue; } if(alpm_add_pkg(config->handle, pkg) == -1) { pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", targ, alpm_strerror(alpm_errno(config->handle))); alpm_pkg_free(pkg); - trans_release(); - return 1; + retval = 1; + continue; } config->explicit_adds = alpm_list_add(config->explicit_adds, pkg); } + if(retval) { + trans_release(); + return retval; + } + /* now that targets are resolved, we can hand it all off to the sync code */ return sync_prepare_execute(); } -- 1.7.8
If someone specifies a bogus line such as pacman -S baz adsf/boo base-devel we are better off trying to process all targets and showing all relevant errors before exiting. This is easier in -U and -R operations where we aren't dealing with groups, but here we attempt to skip group selection once we know a target has errored to avoid cluttering the output and hiding the real problem. Signed-off-by: Dan McGee <dan@archlinux.org> --- Allan, I see what you mean, this isn't pretty. :) I'm still OK with doing this if everyone else is, however. Feedback? -Dan src/pacman/sync.c | 40 +++++++++++++++++++++++++++------------- 1 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 1003a42..9015b08 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -616,7 +616,7 @@ static int process_pkg(alpm_pkg_t *pkg) return 0; } -static int process_group(alpm_list_t *dbs, const char *group) +static int process_group(alpm_list_t *dbs, const char *group, int error) { int ret = 0; alpm_list_t *i; @@ -628,6 +628,12 @@ static int process_group(alpm_list_t *dbs, const char *group) return 1; } + if(error) { + /* we already know another target errored. there is no reason to prompt the + * user here; we already validated the group name so just move on since we + * won't actually be installing anything anyway. */ + goto cleanup; + } if(config->print == 0) { printf(_(":: There are %d members in group %s:\n"), count, @@ -666,12 +672,14 @@ static int process_group(alpm_list_t *dbs, const char *group) } } } + cleanup: alpm_list_free(pkgs); return ret; } -static int process_targname(alpm_list_t *dblist, const char *targname) +static int process_targname(alpm_list_t *dblist, const char *targname, + int error) { alpm_pkg_t *pkg = alpm_find_dbs_satisfier(config->handle, dblist, targname); @@ -685,20 +693,20 @@ static int process_targname(alpm_list_t *dblist, const char *targname) return process_pkg(pkg); } /* fallback on group */ - return process_group(dblist, targname); + return process_group(dblist, targname, error); } -static int process_target(const char *target) +static int process_target(const char *target, int error) { /* process targets */ char *targstring = strdup(target); char *targname = strchr(targstring, '/'); - char *dbname = NULL; int ret = 0; - alpm_list_t *dblist = NULL; + alpm_list_t *dblist; if(targname && targname != targstring) { - alpm_db_t *db = NULL; + alpm_db_t *db; + const char *dbname; *targname = '\0'; targname++; @@ -710,14 +718,15 @@ static int process_target(const char *target) ret = 1; goto cleanup; } - dblist = alpm_list_add(dblist, db); - ret = process_targname(dblist, targname); + dblist = alpm_list_add(NULL, db); + ret = process_targname(dblist, targname, error); alpm_list_free(dblist); } else { targname = targstring; dblist = alpm_option_get_syncdbs(config->handle); - ret = process_targname(dblist, targname); + ret = process_targname(dblist, targname, error); } + cleanup: free(targstring); if(ret && access(target, R_OK) == 0) { @@ -730,6 +739,7 @@ cleanup: static int sync_trans(alpm_list_t *targets) { + int retval = 0; alpm_list_t *i; /* Step 1: create a new transaction... */ @@ -740,12 +750,16 @@ static int sync_trans(alpm_list_t *targets) /* process targets */ for(i = targets; i; i = alpm_list_next(i)) { const char *targ = i->data; - if(process_target(targ) == 1) { - trans_release(); - return 1; + if(process_target(targ, retval) == 1) { + retval = 1; } } + if(retval) { + trans_release(); + return retval; + } + if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); alpm_logaction(config->handle, "starting full system upgrade\n"); -- 1.7.8
participants (3)
-
Allan McRae
-
Dan McGee
-
Dan McGee