Re: [PATCH v2] reachable.c: add HEAD to reachability starting commits

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 2, 2014, 17:47 UTC
Message-ID: <xmqq4mwp3oeb.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1409519804-15960-1-git-send-email-max@max630.net>

Max Kirillov <max@max630.net> writes:

Show 25 quoted lines

> HEAD is not explicitly used as a starting commit for
> calculating reachability, so if it's detached and reflogs
> are disabled it may be pruned.
>
> Add tests which demonstrate it. Test 'prune: prune former HEAD after checking
> out branch' also reverts changes to repository.
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
> Inserted test into existing script.
>  reachable.c      |  3 +++
>  t/t5304-prune.sh | 21 +++++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/reachable.c b/reachable.c
> index 654a8c5..6f6835b 100644
> --- a/reachable.c
> +++ b/reachable.c
> @@ -229,6 +229,9 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
>  	/* Add all external refs */
>  	for_each_ref(add_one_ref, revs);
>  
> +	/* detached HEAD is not included in the list above */
> +	head_ref(add_one_ref, revs);
> +

Unlike the change to rev-parse, which I haven't decided if I like it, this is definitely the right thing to do.

Thanks for catching.

Show 15 quoted lines

>  	/* Add all reflog info */
>  	if (mark_reflog)
>  		for_each_reflog(add_one_reflog, revs);
> diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
> index 377d3d3..77cf064 100755
> --- a/t/t5304-prune.sh
> +++ b/t/t5304-prune.sh
> @@ -104,6 +104,27 @@ test_expect_success 'prune: prune unreachable heads' '
>  
>  '
>  
> +test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> +
> +	git config core.logAllRefUpdates false &&
> +	test ! -e .git/logs &&

This test is somewhat iffy; if you had it just before "git prune", I would understand that it matches what the title of the test claims, though.

Show 11 quoted lines

> +	git checkout --detach --quiet &&
> +	git commit --allow-empty -m "detached commit" &&
> +	git prune -n >prune_actual &&
> +	: >prune_expected &&
> +	test_cmp prune_actual prune_expected
> +
> +'
> +
> +test_expect_success 'prune: prune former HEAD after checking out branch' '
> +
> +	head_sha1=`git rev-parse HEAD` &&

Favor $(...) over `...`.

> +	git checkout --quiet master &&
> +	git prune -v >prune_actual &&
> +	grep -q "$head_sha1" prune_actual

Please don't use "grep -q"; unless running the test with "-v", we wouldn't have to see the output anyway.

Show 6 quoted lines

> +
> +'
> +
>  test_expect_success 'prune: do not prune heads listed as an argument' '
>  
>  	: > file2 &&