Re: [PATCH 1/3] fast-export: allow dumping the refname mapping

From: Eric Sunshine <sunshine@sunshineco.com>
To: Jeff King <peff@peff.net>
Cc: Git List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 1/3] fast-export: allow dumping the refname mapping
Date: Fri, 19 Jun 2020 12:18:16 -0400	[thread overview]
Message-ID: <20200619161816.GA9205@flurp.local> (raw)
In-Reply-To: <20200619160129.GA1843858@coredump.intra.peff.net>

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.

> > 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.

I just applied this atop your patch and it worked fine on Mac OS:

diff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh
index 0c5dd2a4fb..849192b1b0 100755
--- a/t/t9351-fast-export-anonymize.sh
+++ b/t/t9351-fast-export-anonymize.sh
@@ -52,9 +52,12 @@ test_expect_success 'refname mapping can be dumped' '
 	# we make no guarantees of the exact anonymized names,
 	# so just check that we have the right number and
 	# that a sample line looks sane.
+	git show-ref >refs &&
+	nrefs=$(wc -l <refs) &&
 	# 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
 '
