Re: [PATCH] branch: make --set-upstream saner without an explicit starting point

From: Jeff King <peff@peff.net>
Date: Jul 5, 2012, 09:42 UTC
Message-ID: <20120705094213.GA29740@sigill.intra.peff.net>
In-Reply-To: <1341480589-1890-1-git-send-email-cmn@elego.de>

On Thu, Jul 05, 2012 at 11:29:49AM +0200, Carlos Martín Nieto wrote:

Show 13 quoted lines

> The branch command assumes HEAD as the starting point if none is
> specified. This causes --set-upstream to behave unexpectedly if the
> user types
> 
>     git branch --set-upstream origin/master
> 
> git-branch will assume a second argument of HEAD and create config
> entries for a local branch origin/master to track the current
> branch. This is rarely, if ever, what the user wants to do.
> 
> Catch invocations with --set-upstream and only one branch so the
> command above sets up the current branch to track origin's master
> branch.

I have been tempted to write this patch several times but was afraid that somebody was relying on the existing behavior. I think the behavior you propose is much saner.

Show 13 quoted lines

> +# The unsets at the end is to leave the master config as we found it,
> +# so later tests don't get confused
> +
> +test_expect_success 'set upstream with implicit HEAD as branch to modify' \
> +    'git config remote.local.url . &&
> +     git config remote.local.fetch refs/heads/master:refs/remotes/local/master &&
> +     (git show-ref -q refs/remotes/local/master || git fetch local) &&
> +     git branch --set-upstream local/master &&
> +     test $(git config branch.master.remote) = local &&
> +     test $(git config branch.master.merge) = refs/heads/master
> +     git config --unset branch.master.remote &&
> +     git config --unset branch.master.merge
> +'

The unsets will not run if the test fails. Use test_when_finished to insert cleanup, or better yet use test_config which handles this case automagically (you are not setting them initially, but perhaps you should set them to some known value initially to make sure that your command changes them as expected).

I don't understand the point of the show-ref call, though. Isn't the fetch idempotent, and you can just run it always?

-Peff