On 12/07/13 10:32, Dave Reisner wrote: > On Thu, Jul 11, 2013 at 11:42:38PM +0100, Ashley Whetter wrote: >> Hey everyone, >> >> I've attached (and pasted below) a couple of possible patches that fix >> FS#36097 (Wrong exit code for `pacman -Sg`). > > Please start using git-send-email in the future if you expect these to > be reviewed and applied properly: > > https://www.archlinux.org/pacman/submitting-patches.html > >> From 045b58ada05747c6743e8bf68e7e7d8156016eee Mon Sep 17 00:00:00 2001 >> From: Ashley Whetter <[email protected]> >> Date: Fri, 12 Jul 2013 00:15:15 +0100 >> Subject: [PATCH] Sg flag now returns correct return value for errors >> >> sync_group now returns non-zero if no groups are found >> in the (non-empty) list of groups to search for
If we are making this consistent with -Ss, we should return 1 any time a group is not found. e.g. in Arch: pacman -Sg base afsljasdf would return 1 as the group "afsljasdf" does not exist (hopefully!) >> Fixes FS#36097 >> >> Signed-off-by: Ashley Whetter <[email protected]> >> --- >> src/pacman/sync.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/src/pacman/sync.c b/src/pacman/sync.c >> index fc1314b..af26ae4 100644 >> --- a/src/pacman/sync.c >> +++ b/src/pacman/sync.c >> @@ -367,6 +367,7 @@ static int sync_search(alpm_list_t *syncs, >> alpm_list_t *targets) >> static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) >> { >> alpm_list_t *i, *j, *k, *s = NULL; >> + int found_group = 0; >> >> if(targets) { >> for(i = targets; i; i = alpm_list_next(i)) { >> @@ -376,6 +377,7 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> alpm_group_t *grp = alpm_db_get_group(db, grpname); >> >> if(grp) { >> + found_group = 1; >> /* get names of packages in group */ >> for(k = grp->packages; k; k = alpm_list_next(k)) { >> if(!config->quiet) { >> @@ -396,6 +398,8 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> alpm_group_t *grp = j->data; >> >> if(level > 1) { >> + found_group |= (int)(intptr_t) grp->packages; > > This cast doesn't buy you anything. Just compare the pointer to NULL... > >> + >> for(k = grp->packages; k; k = alpm_list_next(k)) { >> printf("%s %s\n", grp->name, >> alpm_pkg_get_name(k->data)); >> @@ -403,6 +407,7 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> } else { >> /* print grp names only, no package names */ >> if(!alpm_list_find_str (s, grp->name)) { >> + found_group = 1; >> s = alpm_list_add (s, grp->name); >> printf("%s\n", grp->name); >> } >> @@ -412,7 +417,7 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> alpm_list_free(s); >> } >> >> - return 0; >> + return (found_group == 0); >> } >> >> static int sync_info(alpm_list_t *syncs, alpm_list_t *targets) >> -- >> 1.8.3.2 >> >> From 2b36585f5aeca8adbe15fc4609ff4bc5536af798 Mon Sep 17 00:00:00 2001 >> From: Ashley Whetter <[email protected]> >> Date: Fri, 12 Jul 2013 00:35:55 +0100 >> Subject: [PATCH] Sg flag now returns correct return value for errors >> MIME-Version: 1.0 >> Content-Type: text/plain; charset=UTF-8 >> Content-Transfer-Encoding: 8bit >> >> sync_group now returns non-zero if no groups are found >> in the (non-empty) list of groups to search for >> >> Fixes FS#36097 >> >> Signed-off-by: Ashley Whetter <[email protected]> >> --- >> src/pacman/sync.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/src/pacman/sync.c b/src/pacman/sync.c >> index fc1314b..9d2dbd6 100644 >> --- a/src/pacman/sync.c >> +++ b/src/pacman/sync.c >> @@ -367,6 +367,7 @@ static int sync_search(alpm_list_t *syncs, >> alpm_list_t *targets) >> static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) >> { >> alpm_list_t *i, *j, *k, *s = NULL; >> + int found = 0; >> >> if(targets) { >> for(i = targets; i; i = alpm_list_next(i)) { >> @@ -376,6 +377,7 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> alpm_group_t *grp = alpm_db_get_group(db, grpname); >> >> if(grp) { >> + found = 1; >> /* get names of packages in group */ >> for(k = grp->packages; k; k = alpm_list_next(k)) { >> if(!config->quiet) { >> @@ -410,9 +412,13 @@ static int sync_group(int level, alpm_list_t >> *syncs, alpm_list_t *targets) >> } >> } >> alpm_list_free(s); >> + >> + /* Even if there are no groups we have still done everything we should >> + * have */ >> + found = 1; >> } >> >> - return 0; >> + return (found == 0); >> } >> >> static int sync_info(alpm_list_t *syncs, alpm_list_t *targets) >> -- >> 1.8.3.2 >> >> >> With the first patch, the new double cast line is a bit weird but >> genuinely flags whether or not we found any groups. >> The second patch seems a bit more sane to me in terms of the >> functionality. It just marks there to be a group found if the user >> didn't search for a specific group, regardless of whether any groups >> were actually found. I think this makes sense because in the highly >> unlikely scenario that there are no groups and the user didn't search >> for any specifically, pacman has still done everything it should have >> done so shouldn't return an error. >> >> Regards, >> Ashley > > > >> > > > >
