rename a remote does not update pushdefault (v1.9.5)

rename a remote does not update pushdefault (v1.9.5)

[Alexander Duytschaever]

When defining pushdefault and renaming the associated remote, the pushdefault setting becomes invalid. Instead, it should follow the renamed remote if that was designated as pushdefault.

Test procedure:

1. Create/cd empty directory
2. `git init .`
3. `git remote add abc def`
4. Observe that `git remote` now shows 'abc'
5. `git config default.pushdefault abc`
6. Observe that `git config --list` shows default.pushdefault=abc
7. `git remote rename abc xyz`

BUG: observe that pushdefault still refers to 'abc', while it should now refer to 'xyz'.

Rgds

(Initially (wrongly) filed as TGit bug: https://code.google.com/p/tortoisegit/issues/detail?id=2438)

Re: rename a remote does not update pushdefault (v1.9.5)

[Junio C Hamano]

Alexander Duytschaever <Alexander.Duytschaever@UGent.be> writes:

> When defining pushdefault and renaming the associated remote, the
> pushdefault setting becomes invalid. Instead, it should follow the
> renamed remote if that was designated as pushdefault.
>
> Test procedure:
>
> 1. Create/cd empty directory
> 2. `git init .`
> 3. `git remote add abc def`
> 4. Observe that `git remote` now shows 'abc'
> 5. `git config default.pushdefault abc`
> 6. Observe that `git config --list` shows default.pushdefault=abc

I do not think we have default.pushdefault.  Perhaps you meant
remote.pushdefault, but even then, I do not think we usually set
that up by default.

> 7. `git remote rename abc xyz`
>
> BUG: observe that pushdefault still refers to 'abc', while it should now refer to 'xyz'.

Again,  by default remote.pushDefault is not set up by us, so there
is nothing to comment here.

> (Initially (wrongly) filed as TGit bug:

Perhaps TGit is setting remote.pushDefault?  If so, I would say a
bug (if there is a bug, which I highly doubt; see below) belongs
there.

Having said all that, if you did this instead, you will see that
remote.pushdefault will not change:

    $ git init
    $ git remote add abc def
    $ git config remote.pushdefault abc
    $ git config remote.pushdefault
    abc
    $ git remote rename abc xyz
    $ git config remote.pushdefault
    abc

and I can see this argued both ways.

1. Imagine you so far had three remotes A, B and DEFAULT defined,
   among which the last one was the default push destination, like
   this:

       [remote]
           pushDefault = DEFAULT
       [remote "A"]
           url = http://a.xz/project
       [remote "B"]
           url = http://b.xz/project
       [remote "DEFAULT"]
           url = http://c.xz/project

   and for whatever reason, you decided c.xz should not be the default
   place for you to push to, but instead you want to make b.xz the
   default.  It is conceivable that you would expect this to work:

       $ git remote rename DEFAULT C
       $ git remote rename B DEFAULT
    
   to give you this:

       [remote]
           pushDefault = DEFAULT
       [remote "A"]
           url = http://a.xz/project
       [remote "DEFAULT"]
           url = http://b.xz/project
       [remote "C"]
           url = http://c.xz/project

   And for that use case, remaing remote.pushDefault is absolutely the
   wrong thing to do.  After the "rename DEFAULT C", remote.pushDefault
   must not be moved to C, because the next "rename B DEFAULT" will not
   be able to guess that remote.pushDefault wants to become DEFAULT.

2. On the other hand, starting from the same "A, B and DEFAULT"
   state, you may be trying to rename DEFAULT everywhere to C to
   come to this instead:

       [remote]
           pushDefault = C
       [remote "A"]
           url = http://a.xz/project
       [remote "B"]
           url = http://b.xz/project
       [remote "C"]
           url = http://c.xz/project

   And for that use case, not remaing remote.pushDefault will appear to
   be a bug, as remote.pushDefault will be left as DEFAULT.

Whichever way you implement "remote rename", you will make half of
the users happy while making other half unhappy.  One use case will
be happier if remote.pushdefault is left intact; the other use case
will be happier if remote.pushdefault is updated.  There are two
sides to this coin.

I think the implementation took the most straight-forward path to
say "we rename everything inside remote.C.* section and adjust the
refspecs for remote-tracking branches because that is what appear in
that section"; which allows the first use case and the second use
case would be just a single "git config remote.pushDefault C" away.

If you change what the command does this late in the game, then not
only you will upset people who have been happy with the current
behaviour (mostly those who used the first scenario), but you will
not really help people who may have wished that remote.pushDefault
to be adjusted either, because they are already used to do the final
"git config remote.pushDefault C" anyway.

So...

Re: rename a remote does not update pushdefault (v1.9.5)

[Jeff King]

On Mon, Mar 30, 2015 at 12:39:17PM -0700, Junio C Hamano wrote:

> Whichever way you implement "remote rename", you will make half of
> the users happy while making other half unhappy.  One use case will
> be happier if remote.pushdefault is left intact; the other use case
> will be happier if remote.pushdefault is updated.  There are two
> sides to this coin.
> 
> I think the implementation took the most straight-forward path to
> say "we rename everything inside remote.C.* section and adjust the
> refspecs for remote-tracking branches because that is what appear in
> that section"; which allows the first use case and the second use
> case would be just a single "git config remote.pushDefault C" away.

I had a similar thought. But note that we do update "branch.*.remote"
that points to the renamed remote. So it seems inconsistent that we do
not similarly update "branch.*.pushremote". And if we update that, it
seems inconsistent that we do not update "remote.pushdefault".

In other words, we should probably choose to update all references or
none, but we are currently somewhere in between. Of course, the fact
that the code has been in this limbo for so long makes it doubly
awkward, as we do not know what people expect (and what they simply
consider a bug).

So I don't know what the right answer is.

I did take a peek at the code. I don't think updating these variables
would be too hard, but there needs to be some refactoring around
remote.c's pushremote_name.  After calling read_config(), we do not have
a value that exactly corresponds to remote.pushdefault; we overwrite it
with the branch-specific pushremote if there is one.

-Peff