On Fri, Jun 19, 2020 at 9:25 AM Jeff King <peff@xxxxxxxx> wrote:
> [...]
> Let's make it possible to dump the mapping separate from the output
> stream. This can be used by a bug reporter to modify their reproduction
> recipe without revealing the original names (see the example in the
> documentation).
> [...]
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> @@ -118,6 +120,32 @@ static int has_unshown_parent(struct commit *commit)
> +static void maybe_dump_anon(FILE *out, struct seen_set *seen,
> + const char *orig, const char *anon)
> +{
> + if (!out)
> + return;
> + if (!check_and_mark_seen(seen, orig))
> + fprintf(out, "%s %s\n", orig, anon);
> +}
Nit: For a function which has only a single caller in this patch and
one more caller added in 3/3, I wonder if the, um, convenience of this
function short-circuiting as the very first thing it does outweighs
the loss of clarity of having the callers make the check themselves.
That is, have the caller do this instead:
if (anonymized_refnames_handle)
dump_anon(anonymized_refnames_handle, ...);
> @@ -1213,6 +1249,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> + if (anonymized_refnames_file)
> + anonymized_refnames_handle = xfopen(anonymized_refnames_file, "w");
For completeness, do you want to close this file at some point?
> diff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh
> @@ -46,6 +46,18 @@ test_expect_success 'stream omits tag message' '
> +test_expect_success 'refname mapping can be dumped' '
> + git fast-export --anonymize --all \
> + --dump-anonymized-refnames=refs.out >/dev/null &&
> + # 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.
> + # Note that master is not anonymized, and so not included
> + # in the mapping.
> + test_line_count = 6 refs.out &&
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 &&
and then drop the 'nrefs=$((nrefs - 1))' line and associated comment
in patch 2/3 which removes the "master" special case.
> + grep "^refs/heads/other refs/heads/" refs.out
> +'
