Re: [PATCH v3 3/3] repack -ad: prune the list of shallow commits

[Junio C Hamano <gitster@pobox.com>]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/builtin/repack.c b/builtin/repack.c
> index c6a7943d5..9217fc832 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  		if (!po_args.quiet && isatty(2))
>  			opts |= PRUNE_PACKED_VERBOSE;
>  		prune_packed_objects(opts);
> +
> +		if (!keep_unreachable &&
> +		    (!(pack_everything & LOOSEN_UNREACHABLE) ||
> +		     unpack_unreachable) &&
> +		    is_repository_shallow(the_repository))
> +			prune_shallow(0, 1);

The logic looks correct (and the reasoning behind it, which was
explained in the log message, was sound).  prune_shallow(0, 1)
however is not easily understandable.

There are only two callsites of this function after these three
patches, and the areas of the code that have these calls are in
relatively quiescent parts in the whole tree, so it shouldn't be too
distracting to do an update to make the function take a flag word,
so that we can make callsites more immediately obvious, i.e.

	prune_shallow(PRUNE_SHALLOW_QUICK)

It of course can be left as a low-hanging fruit loose-end.

> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> index 2d0031703..777c9d1dc 100755
> --- a/t/t5537-fetch-shallow.sh
> +++ b/t/t5537-fetch-shallow.sh
> @@ -186,7 +186,7 @@ EOF
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure '.git/shallow is edited by repack' '
> +test_expect_success '.git/shallow is edited by repack' '
>  	git init shallow-server &&
>  	test_commit -C shallow-server A &&
>  	test_commit -C shallow-server B &&