Re: [PATCH 2/2] ls-files: add --sparse option

On Fri, Dec 10 2021, Derrick Stolee wrote:

On 12/9/2021 7:50 AM, Ævar Arnfjörð Bjarmason wrote:

quoted

On Wed, Dec 08 2021, Derrick Stolee wrote:

quoted

On 12/8/2021 1:36 PM, Elijah Newren wrote:

quoted

On Wed, Dec 8, 2021 at 10:23 AM Derrick Stolee [off-list ref] wrote:

quoted

On 12/8/2021 12:04 PM, Elijah Newren wrote:

quoted

This actually looks quite nice, though the magic '16' is kind of
annoying.  Could we get rid of that -- perhaps using something to rip
out the diff header, or using comm instead?

What I really want is "remove the first two lines of this file"

Is `tail -n +3` portable?  Looks like we have five uses of tail -n +N
in the testsuite, so it should be okay to use.

Ah, that's the magic incantation. Sounds good.

quoted

quoted

but perhaps "tail -n $(wc -l expect)" would suffice to avoid a
magic number?

That works too.

If the "-n +X" syntax works, then I'll opt for that.

I think it should be per
https://pubs.opengroup.org/onlinepubs/009695299/utilities/tail.html

But isn't that "diff -u" non-portable, per GIT_TEST_CMP. I.e. sometimes
we'll fall back on "cmp" etc.

You're talking about this hunk, right?

if test -z "$GIT_TEST_CMP"
then
	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"
	then
		GIT_TEST_CMP="$DIFF -c"
	else
		GIT_TEST_CMP="$DIFF -u"
	fi
fi

This only switches from "diff -u" to "diff -c" if the
GIT_TEST_CMP_USE_COPIED_CONTEXT variable is set, but it is not set
by default. Thus, we are using "diff -u" by default throughout.

Please let me know if I'm misreading this.

Yes and no. Yes it's not on by default, but the reason (well, maybe not
*the* reason, but definitely one reason) we have GIT_TEST_CMP is because
-u isn't portable.

It's not in POSIX, and not just in the theoretical sense, e.g. when I
run tests on HP/UX it'll yell at me about not knowing about -u until I
adjust GIT_TEST_CMP.

Thus you can't use "diff -u" as you did upthread without breaking the
test on those platforms, and you can't execute "$GIT_TEST_CMP" either
because it might use -c, and then your "skip the first three lines"
might not hold.

So even if we avoid using "git" for the actual test helpers themselves
usually using it in this case is probably fine, and a neat way out of
this particular hassle without worrying about portability.

So I don't know if we want a diff here, but if so I think it would be
neat to wrap that in say a "test_cmp_diff" which we'd just use as:

	test_cmp_diff a b <<-\EOF
        <unified diff here>
	EOF

Where under the hood we'd just run:

	git diff --no-index a b >actual
	cat >expect &&
	[ munge diff header here? ] &&
	test_cmp expect actual