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 <awhetter.2011@my.bristol.ac.uk> 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 <awhetter.2011@my.bristol.ac.uk> --- 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 <awhetter.2011@my.bristol.ac.uk> 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 <awhetter.2011@my.bristol.ac.uk> --- 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