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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 19, 2019, 18:11 UTC
Message-ID: <xmqqpngkb2ye.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191219024518.GA3411@dcvr>

Eric Wong <e@80x24.org> writes:

Show 17 quoted lines

> But using a shell for loop seems doable, here, since there
> doesn't seem to be wonky characters.  I've done this in the past
> when I had to fix a system without "ls".
>
> This goes on top of your patch:
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 3a3eafa653..a431d05643 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -6,7 +6,7 @@ test_description='sparse checkout builtin tests'
>  
>  ls_no_git()
>  {
> -	ls -1 "$1" | grep -v .git
> +	( cd "$1" && for i in *; do echo "$i"; done )
>  }

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

 (2) fix your "ls" to behave

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?

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, if somebody wants to keep using a broken /bin/ls?