Re: [RFC] add test cases for the --repo option to git push

[Michael J Gruber <git@drmicha.warpmail.net>]

Junio C Hamano venit, vidit, dixit 26.02.2009 18:09:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> First of all: I define good/bad as matching the documentation.
> 
> Ok, I was primarily working from this:
> 
> commit bcc785f611dc6084be75999a3b6bafcc950e21d6
> Author: Linus Torvalds <torvalds@osdl.org>
> Date:   Mon Oct 30 08:28:59 2006 -0800
> 
>     git push: add verbose flag and allow overriding of default target repository
>     
>     This adds a command line flag "-v" to enable a more verbose mode, and
>     "--repo=" to override the default target repository for "git push" (which
>     otherwise always defaults to "origin").
>     
>     This, together with the patch to allow dashes in config variable names,
>     allows me to do
>     
>     	[alias]
>     		push-all = push -v --repo=all
>     
>     in my user-global config file, and then I can (for any project I maintain)
>     add to the project-local config file
>     
>     	[remote "all"]
>     		url=one.target.repo:/directory
>     		url=another.target:/pub/somewhere/else
>     
>     and now "git push-all" just updates all the target repositories, and shows
>     me what it does - regardless of which repo I am in.
>     
>     Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>     Signed-off-by: Junio C Hamano <junkio@cox.net>
> 
> If documentation does not match it, we need to figure out why J6t with
> bf07cc5 (git-push.txt: Describe --repo option in more detail, 2008-10-07)
> needed to update the documentation.
> 
> It could be that the behaviour changed (perhaps by accident, perhaps by
> design) after Linus introduced --repo with bcc785f (git push: add verbose
> flag and allow overriding of default target repository, 2006-10-30) and
> J6t documented that updated behaviour.  And since then there was another
> behaviour change (again, perhaps by accident, perhaps by design) that made
> you notice the description does not match the behaviour.
> 
> You will see that:
> 
>  (1) bf07cc5 (i.e. J6t's documentation) passes your tests;
> 
>  (2) somewhere between that and v1.6.2-rc2, there is a regression to make
>      your test fail.

I see. Back then I checked whether there was a change to git-push at or
after J6t's doc commit, and there was none, but I didn't test. I'll do now.

> if the above conjecture is true, and we may want to fix that regression to
> match the documentation.
> 
> On the other hand, if bf07cc5 does not pass your tests, it means that the
> documentation update was the cause of the confusion, and it is not the
> behaviour that needs to be fixed.

>From Linus' description it's not clear to me what should happen when
there is no explicit repo argument but the branch is tracking a remote
and there is a --repo option. And I think this case is the only open
question: Should the option win or the tracking config? Code does
option, doc says tracking config.

[Also, I don't see immediately what's wrong with "alias.push-all = push
-v all", in the current situation where option and arg are equivalent,
but I haven't tried.]

I'm cc'ing Linus to make sure J6t's (current) doc describes the original
intent for --repo and my patch isn't stepping on /some/one's toes...

> Sorry, but I do not have time today to look into this.  Could you help?

Of course. I ursurpated someone else's itch here, but now it's mine ;)

I'll bisect. Seems to be the right thing to do while watching soccer
later in the evening...

Michael