GNU bug report logs - #9987
RFE: 'groups' command ADD command switches "-0", and "-1"

Previous Next

Package: coreutils;

Reported by: Linda Walsh <coreutils <at> tlinx.org>

Date: Mon, 7 Nov 2011 22:31:01 UTC

Severity: normal

Merged with 12083

Done: Bernhard Voelker <mail <at> bernhard-voelker.de>

Bug is archived. No further changes may be made.

Full log


Message #61 received at 9987 <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 9987 <at> debbugs.gnu.org
Subject: Re: bug#9987: [PATCH] groups,id: add -0, --null option
Date: Thu, 19 Sep 2013 09:56:48 +0200 (CEST)
> On September 19, 2013 at 3:31 AM Pádraig Brady <P <at> draigBrady.com> wrote:
> 
> On 09/19/2013 12:24 AM, Bernhard Voelker wrote:
> > diff --git a/NEWS b/NEWS

> > +  id accepts a new option: --zero (-z) to separate the output entries by
> > +  a NUL instead of a white space character.
> 
> s/separate/delimit/

done.

> > diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> > +@item -z
> > +@itemx --zero
> > +@opindex -z
> > +@opindex --zero
> > +Separate output items with NUL characters.
> 
> s/Separate/Delimit/

Actually I copied that sentence from other sections.
I guess it's wrong there, too.  And probaby it makes
sense to replace it by a macro.

> > +@example
> > +$ id -Gn --zero
> > +users <NUL> devs
> 
> users <NUL> devs <NUL>

Good point!
 
> > diff --git a/tests/misc/id-zero.sh b/tests/misc/id-zero.sh
> 
> > +id root || fail=1
> 
> Is 'root' a guaranteed name?
> misc/chroot-credentials.sh since v7.4-16-gc45c51f assumes so.
> I'm still not convinced (and will look at addressing that separately).

Cygwin lacks a 'root' entry - but it also lacks getent ...

> > +# Create a nice list of users (ignoring errors) ...
> > +getent passwd | head -n 100 | cut -d: -f1 > users
> 
> test -s users || skip_ 'Failed to read the passwd database with getent'

... so this test is good; added.

> Also why so many (100). The test is a bit slow and you would get
> the same coverage from a couple of entries?

Actually that was to protect against "very-expensive" behavior on
bigger NIS installation. On normal installations, I haven't seen much
more than 30-40 users yet anyway.

The point was to get an entry with only 1 and with 2 or more groups.
I wouldn't go that far and add require_membership_in_two_groups_,
so maybe finding such entries with something like this would be better
(btw: can we rely on xargs to be there)?

  getent passwd | head -n50 | cut -d: -f1 \
    | xargs groups \
    | awk '{ x[NF] = $1; }
           END { for (u in x) print x[u]; }'

Anyway, I moved the tr(1) out of the loop, so we're saving a few
pipe creations.

> > +# id(1) should refuse --zero in default format.
> > +id --zero > out 2>err && fail=1
> > +compare /dev/null out || fail=1
> 
> > +grep 'option --zero not permitted in default format' err || fail=1
> 
> Better to:
> echo 'option ...' > exp-err
> compare exp-err err || fail=1

true, changed.
 
> > +printf "\n" > exp || framework_failure_
> 
> Minor point, but it's better to use '' quotes when not interpolating,
> here and throughout the test.

true, thanks.

> > +cp /dev/null  out || framework_failure_
> 
> :> out

yep, done.

Thanks for the review.
I'll provide a new version of the patch later (or tomorrow).

Have a nice day,
Berny




This bug report was last modified 11 years and 299 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.