Re: [PATCH v2] t4129: don't fail if setgid is set in the test directory

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Jan 9, 2021, 14:19 UTC
Message-ID: <CAHd-oW4Yus5E5U0dykUBfwYM7dryz94YZW_9OFUSZNA-6pp3UA@mail.gmail.com>
In-Reply-To: <xmqqlfd5obvx.fsf@gitster.c.googlers.com>

On Wed, Jan 6, 2021 at 8:59 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

>
> Matheus Tavares <matheus.bernardino@usp.br> writes:
>
> > +# Get the modebits from a file or directory, ignoring the setgid bit (g+s).
> > +# This bit is inherited by subdirectories at their creation. So we remove it
> > +# from the returning string to prevent callers from having to worry about the
> > +# state of the bit in the test directory.
> > +#
>
> We probably do not use "chmod g+s" manually on regular files, so I
> may be being overly "correct", but shouldn't these be done only for
> directories?
>
> >  test_modebits () {
> > -     ls -ld "$1" | sed -e 's|^\(..........\).*|\1|'
> > +     ls -ld "$1" | sed -e 's|^\(..........\).*|\1|' \
> > +                       -e 's|^\(......\)S|\1-|' -e 's|^\(......\)s|\1x|'
>
> That is,
>
>                           -e 's|^\(d.....\)S|\1-|' -e 's|^\(d.....\)s|\1x|'
>
> instead of applying the rule to any filetype.

Yeah, you're right. I ended up applying the rule on regular files as well just for standardization. That is, if some day, for some reason, a test script decides to use "chmod g+s" on a regular file and a directory, test_modebits would treat them equally to avoid any confusion. But I guess it's very unlikely that we will ever need to set the setgid bit on a test, anyway...

> Will queue as-is, as the distinction probably would not matter in
> practice.
>
> Thanks.