Subject: Re: [PATCH] Fix behavior with non-committish upstream references

From: Michael J Gruber <[email-masked]>
Date: Tue, 28 Apr 2009 09:44:47 +0200

Junio C Hamano venit, vidit, dixit 28.04.2009 09:30:
> Michael J Gruber <[email-masked]> writes:
> 
>> stat_tracking_info() assumes that upstream references (as specified by
>> --track or set up automatically) are commits. By calling lookup_commit()
>> on them, create_objects() creates objects for them with type commit no
>> matter what their real type is; this disturbs lookup_tag() later on the
>> call sequence, leading to git status, git branch -v  and git checkout
>> erroring out.
>>
>> Fix this by using lookup_commit_reference() instead so that (annotated)
>> tags can be used as upstream references.
>>
>> Signed-off-by: Michael J Gruber <[email-masked]>
>> ---
>> I'm sorry I won't be able to write a test any more today. Please let me
>> whether it's okay without a test.
> 
> I am sorry, but I simply do not see much point in this.  I think you meant
> by the title "non-commit upstream ref", as a tag that eventually peels to
> a commit is a committish.  Because a tag is meant to be immutable, forking

Ooops, of course you're right. I meant "not of type commit" but
resolvable to a commit, so in fact a non-commit committish.

> from that mean your further "merges from upstream" won't do anything, so
> the current behaviour of returning without saying anything sounds like the
> right thing to do, even though I strongly suspect that it behaves this way
> by accident not by design.

The current behavior is different: git branch/checkout allow you to
specify a tag as upstream, but error out and die when that info is used
(when stat_tracking_info() is called) with a mysterious message, saying
"A is a commit, not a tag", when in fact A is a tag (see Simon's report,
or try "git branch --track tmp v1.6.3-rc3" and run "git branch -v"),
because the entry in hash_obj is messed up.

So I claim that the current behavior is buggy, both on the user visible
side as well as in the internals (setting up wrong obj). The two
possible solutions are:

- Disallow non-commit upstreams. (But people can set anything using git
config.)
- Make stat_tracking_info() work with non-commit committish upstreams.

The latter is what my patch does, without changing behavior for commit
type upstreams.

> 
> Admittedly, I do not "fork and keep up-to-date with an upstream" that
> often, so I am in no way making a final decision here.  It would be
> healthy for interested people to discuss this patch, but I'd appreciate it
> if it happens after 1.6.3 final.

There is one huge advantage when your upstream is a tag: You may be
"ahead", but you'll never be "behind" ;)

Seriously, I find this useful as a way of developing topics on top of a
certain release and seeing the info in git branch -vv, git status etc.

In any case, erroring out and claiming that a tag is not a tag is a bug
which should be fixed one way or the other (which is why I thought it's
okay during rc, being a bug fix).

Michael