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

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Apr 28, 2009, 07:44 UTC
Message-ID: <49F6B3EF.8010503@drmicha.warpmail.net>
In-Reply-To: <7vab61s0aq.fsf@gitster.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 28.04.2009 09:30:

Show 20 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> 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 <git@drmicha.warpmail.net>
>> ---
>> 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.

Show 5 quoted lines

> 
> 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