Re: [PATCH 1/2] Clean up reflog unreachability pruning decision

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

Linus Torvalds <torvalds@linux-foundation.org> writes:

> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Tue, 31 Mar 2009 09:45:22 -0700
>
> This clarifies the pruning rules for unreachable commits by having a 
> separate helpder function for the unreachability decision.
>
> It's preparation for actual bigger changes to come to speed up the
> decision when the reachability calculations become a bottleneck.
>
> In the process it also _does_ change behavior, although in a way that I 
> think is much saner than the old behavior (which was in my opinion not 
> designed, just a result of how the tests were written). It now will prune 
> reflog entries that are older than that 'prune_unreacable' time _and_ that 
> have commit references that can't be even looked up.

> Of course, "--stale-fix" also does that, and does it regardless of the age 
> of the reflog entry is, but I really think this is the right thing to do. 
> If we can't even look it up, we should consider it to be unreachable.
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  builtin-reflog.c |   32 ++++++++++++++++++++++++++------
>  1 files changed, 26 insertions(+), 6 deletions(-)
>
> Note the behavioural change. I think it's sane and "ObviouslyCorrect(tm)", 
> but maybe somebody disagrees.

I did not recall initially when I was discussing this with you last night
but I think this "no commit? not prune" is intentional.  The codepath is
not about logs/heads but logs/*anything*, and we allow pointers to non
commit objects (like v2.6.11-tree).

Also even if we limit ourselves to commits, @{-N} notation can be used to
see the history of branch switching, and that does not need any underlying
objects.  I do not think it is interesting to be able to see which branch
you were on 30 days abo, though ;-)


> diff --git a/builtin-reflog.c b/builtin-reflog.c
> index d95f515..0355ce6 100644
> --- a/builtin-reflog.c
> +++ b/builtin-reflog.c
> @@ -209,6 +209,31 @@ static int keep_entry(struct commit **it, unsigned char *sha1)
>  	return 1;
>  }
>  
> +static int unreachable(struct expire_reflog_cb *cb, struct commit *commit, unsigned char *sha1)
> +{
> +	/*
> +	 * We may or may not have the commit yet - if not, look it
> +	 * up using the supplied sha1.
> +	 */
> +	if (!commit) {
> +		if (is_null_sha1(sha1))
> +			return 0;
> +
> +		commit = lookup_commit_reference_gently(sha1, 1);
> +
> +		/* We can't even look it up - consider it unreachable */
> +		if (!commit)
> +			return 1;

	/* If it is not a commit, keep it. */
        if (!commit)
        	return 0;

> +	}
> +
> +	/* Reachable from the current reflog top? Don't prune */

That's "tip of the ref", not necessarily "reflog top".

> +	if (in_merge_bases(commit, &cb->ref_commit, 1))
> +		return 0;
> +
> +	/* We can't reach it - prune it. */
> +	return 1;
> +}
> +
>  static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
>  		const char *email, unsigned long timestamp, int tz,
>  		const char *message, void *cb_data)
> @@ -230,12 +255,7 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
>  	if (timestamp < cb->cmd->expire_unreachable) {
>  		if (!cb->ref_commit)
>  			goto prune;
> -		if (!old && !is_null_sha1(osha1))
> -			old = lookup_commit_reference_gently(osha1, 1);
> -		if (!new && !is_null_sha1(nsha1))
> -			new = lookup_commit_reference_gently(nsha1, 1);
> -		if ((old && !in_merge_bases(old, &cb->ref_commit, 1)) ||
> -		    (new && !in_merge_bases(new, &cb->ref_commit, 1)))
> +		if (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))
>  			goto prune;
>  	}
>  
> -- 
> 1.6.2.1.404.gb0085.dirty