From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 19 Jun 2020 19:20:38 GMT
Subject: Re: [PATCH 1/3] fast-export: allow dumping the refname mapping
Message-ID: <xmqqv9jmhng9.fsf@gitster.c.googlers.com>
In-Reply-To: <20200619161816.GA9205@flurp.local>

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Jun 19, 2020 at 12:01 PM Jeff King <peff@peff.net> wrote:
>> On Fri, Jun 19, 2020 at 11:51:06AM -0400, Eric Sunshine wrote:
>> > That is, have the caller do this instead:
>> >
>> >   if (anonymized_refnames_handle)
>> >     dump_anon(anonymized_refnames_handle, ...);
>>
>> <shrug> The names were long enough that I thought it was more clear not
>> to repeat myself. [...]
>
> Indeed, it's a minor point and subjective. Certainly not worth a
> re-roll or even worrying about it.

It probably is subjective but fwiw I too find yours easier to
follow.

>> > This hard-coded "6" seems rather fragile, causing the test to break if
>> > other refs are ever added or removed. Perhaps just count the refs
>> > dynamically?
>> >
>> >   git show-ref >refs &&
>> >   nrefs=$(wc -l refs) &&
>> >   # Note that master is not anonymized, and so not included
>> >   # in the mapping.
>> >   nrefs=$((nrefs - 1))
>> >   test_line_count = $nrefs refs.out &&
>> >
>> That's exactly what I wrote originally, but it failed on macos due to
>> extra spaces in the "wc" output.
>
> Hmph, that shouldn't have failed. Did you quote the $(wc -l refs)
> invocation? Quoting it would cause it to fail.

> +	git show-ref >refs &&
> +	nrefs=$(wc -l <refs) &&

Yup, I've seen that workaround for macs too many times and it should
work well.

>  	# Note that master is not anonymized, and so not included
>  	# in the mapping.
> -	test_line_count = 6 refs.out &&
> +	nrefs=$((nrefs - 1)) &&
> +	test_line_count = $nrefs refs.out &&
>  	grep "^refs/heads/other refs/heads/" refs.out
>  '