Re: [PATCH] sparse-checkout: improve OS ls compatibility

From: Ed Maste <emaste@freebsd.org>
Date: Dec 19, 2019, 20:56 UTC
Message-ID: <CAPyFy2BubWbyq6tQmHYxquikn2+uHz+48VSfQ308BYiuE=SSWQ@mail.gmail.com>
In-Reply-To: <xmqqpngkb2ye.fsf@gitster-ct.c.googlers.com>

On Thu, 19 Dec 2019 at 13:11, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

>
> Hmph, my honest me is very tempted to say
>
>  (1) don't run your tests as 'root', as that would break many tests
>      with prerequisite SANITY

It looks like this is just Cirrus-CI's default without explicit configuration or scripting to use an unprivileged user. Certainly running the build and test as root is generally not a good idea, but it is a purpose-built throwaway VM and so doesn't matter much. Anyhow, it certainly needs to be addressed to avoid skipping the SANITY tests.

>  (2) fix your "ls" to behave

Well, given that hidden dot files were ostensibly a bug and BSD ls has had this behaviour for over 40 years it's far too late to change. I can see the rationale for showing all files for root, even though I dislike the behaviour changing between privileged and unprivileged users.

Show 6 quoted lines

> but if you want to list paths that match shell glob *, this would do
>
>         (cd "$1" && printf "%s\n *)
>
> without any loop (other than the one printf gives us implicitly for
> free), wouldn't it?

Yes.

Show 9 quoted lines

> Note that the helper function's name no longer reflects what it does
> with such a change, so it needs to be renamed.  Together with style
> fix, perhaps
>
>         ls_no_dot () {
>                 (cd "$1" && printf "%s\n *)
>         }
>
> is what we want,

I believe the tests should pass or be skipped when run as root, so I think we should either require (something like) SANITY for these tests, or make the change above. I'm happy with either option; I'll send a v2 based on the approach above for consideration.