Re: [bug] git checkout lies about number of ahead commits when switching from detached HEAD

[Jeff King <peff@peff.net>]

On Sat, Mar 19, 2011 at 06:28:52PM -0400, Jeff King wrote:

> On Sat, Mar 19, 2011 at 10:53:13PM +0100, Piotr Krukowiecki wrote:
> 
> > It says "by 0 commits" when going back from detached HEAD to
> > master branch:
> > 
> > 
> > $ git checkout HEAD^
> > 
> > $ git checkout master
> > Previous HEAD position was af4c62a... Merge branch 'maint'
> > Switched to branch 'master'
> > Your branch is ahead of 'origin/master' by 0 commits.
> >                                         ^^^^^^^^^^^^
> > 
> > $ git status
> > # On branch master
> > # Your branch is ahead of 'origin/master' by 1 commit.
> >                                           ^^^^^^^^^^^
> 
> Hmm. My guess is that the new "check for connectivity when leaving
> detached HEAD" test is polluting the commit flags for the ahead/behind
> test.
> 
> [bisect bisect bisect]
> 
> Yep, it bisects to 8e2dc6a (commit: give final warning when reattaching
> HEAD to leave commits behind, 2011-02-18). We probably need to clean the
> uninteresting flags between the two traversals.

This patch fixes it:

diff --git a/remote.c b/remote.c
index ca42a12..92b7428 100644
--- a/remote.c
+++ b/remote.c
@@ -1560,6 +1560,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
 	strcpy(symmetric + 40, "...");
 	strcpy(symmetric + 43, sha1_to_hex(theirs->object.sha1));
 
+	clear_commit_marks(ours, -1);
+	clear_commit_marks(theirs, -1);
 	init_revisions(&revs, NULL);
 	setup_revisions(rev_argc, rev_argv, &revs, NULL);
 	prepare_revision_walk(&revs);

but I'm not quite sure if this is the right place. Is it the
responsibility of the checkout-orphan-warning code to clean up after
itself, or is it the responsibility of a revision walker to clean up
before itself?

In the former case, things can be a bit tricky because you have to
remember the tips you started from to call clear_commit_marks(), which
means saving away the revs.pending list after setup_revisions but before
prepare_revision_walk. It might be worth having an alternate
clear_commit_marks() that didn't actually walk the tree but just cleared
the marks from every commit we've loaded, giving other walkers a total
clean slate.

If the latter case (clean up beforehand), should prepare_revision_walk
actually be clearing any existing marks?

I think I favor the prepare_revision_walk approach; in most cases
cleaning up afterwards is just a waste (since there usually isn't a
second walk). But I don't know if there is code that actually depends on
the intermediate results of a previous walk.

-Peff