Re: [BUG] Possible bug in `remote set-url --add --push`

Jardel Weyrich [off-list ref] writes:

I believe `remote set-url --add --push` has a bug. Performed tests
with v1.8.0.1 and v1.8.1 (Mac OS X).

Quoting the relevant part of the documentation:

quoted

set-url
    Changes URL remote points to. Sets first URL remote points to matching regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If <oldurl> doesn’t match any URL, error occurs and nothing is changed.

    With --push, push URLs are manipulated instead of fetch URLs.
    With --add, instead of changing some URL, new URL is added.
    With --delete, instead of changing some URL, all URLs matching regex <url> are deleted. Trying to delete all non-push URLs is an error.

Here are some steps to reproduce:

1. Show the remote URLs

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test (push)

2. Add a new push URL for origin

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
    /Volumes/sandbox/test_clone2

3. Check what happened

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test_clone2 (push)

The original pushurl was replaced with the additional one, instead
of being left and the new one getting added.  That looks certainly
wrong.

However, the result of applying the attached patch (either to
v1.7.12 or v1.8.1) still passes the test and I do not think it is
doing anything differently from what you described above.

What do you get from

	git config -l | grep '^remote\.origin'

in steps 1. and 3. in your procedure?  This question is trying to
tell if your bug is in "git remote -v" or in "git remote set-url".

-- >8 --
From 0f6cbc67db926e97707ae732b02e790b4604508e Mon Sep 17 00:00:00 2001
From: Junio C Hamano <redacted>
Date: Fri, 11 Jan 2013 23:04:16 -0800
Subject: [PATCH] t5505: adding one pushurl from jweyrich

Signed-off-by: Junio C Hamano <redacted>
---
 t/t5505-remote.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index c03ffdd..b31c5bb 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh

@@ -901,6 +901,25 @@ test_expect_success 'remote set-url --push --add aaa' '
 	cmp expect actual
 '
 
+test_expect_success 'remote set-url --push --add' '
+	git config remote.jweyrich.url /Volumes/sandbox/test &&
+	git config remote.jweyrich.pushurl /Volumes/sandbox/test &&
+	git config remote.jweyrich.fetch "refs/heads/*:refs/remotes/jweyrich/*" &&
+
+	added=/Volumes/sandbox/test_clone2 &&
+	{
+		git config -l | grep "^remote\.jweyrich\." &&
+		echo "remote.jweyrich.pushurl=$added"
+	} | sort >expect &&
+
+	git remote set-url --add --push jweyrich "$added" &&
+	git config -l | grep "^remote\.jweyrich\." | sort >actual &&
+
+	test_cmp expect actual &&
+
+	git remote -v | grep "^jweyrich" # this is just for debugging
+'
+
 test_expect_success 'remote set-url --push bar aaa' '
 	git remote set-url --push someremote bar aaa &&
 	echo foo >expect &&

-- 
1.8.1.421.g6236851

Step 1:

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test (push)

jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*

Step 3:

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add
--push origin /Volumes/sandbox/test_clone2
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2/ (push)

jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.pushurl=/Volumes/sandbox/test_clone2/


After that, if I do a commit in test_clone1 and try to push to origin,
it pushes only to the test_clone2 rather than pushing to both test and
test_clone2 (it's a bare repo, sorry for using a misleading name).

Is `remote.<remote_name>.pushurl` required for the primary URL as
well? If not, then git-push is not handling that information as it
should.

On Sat, Jan 12, 2013 at 5:10 AM, Junio C Hamano [off-list ref] wrote:

Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:

Jardel Weyrich [off-list ref] writes:

quoted

I believe `remote set-url --add --push` has a bug. Performed tests
with v1.8.0.1 and v1.8.1 (Mac OS X).

Quoting the relevant part of the documentation:

quoted

set-url

    Changes URL remote points to. Sets first URL remote points to
    matching regex <oldurl> (first URL if no <oldurl> is given) to
    <newurl>. If <oldurl> doesn’t match any URL, error occurs and
    nothing is changed.
    
    With --push, push URLs are manipulated instead of fetch URLs.
    With --add, instead of changing some URL, new URL is added.
    With --delete, instead of changing some URL, all URLs matching regex
    <url> are deleted. Trying to delete all non-push URLs is an error.> 

Here are some steps to reproduce:

1. Show the remote URLs

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test (push)

2. Add a new push URL for origin

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
origin \> 
    /Volumes/sandbox/test_clone2

3. Check what happened

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test_clone2 (push)

The original pushurl was replaced with the additional one, instead
of being left and the new one getting added.  That looks certainly
wrong.

However, the result of applying the attached patch (either to
v1.7.12 or v1.8.1) still passes the test and I do not think it is
doing anything differently from what you described above.

What do you get from

	git config -l | grep '^remote\.origin'

in steps 1. and 3. in your procedure?  This question is trying to
tell if your bug is in "git remote -v" or in "git remote set-url".

I'm not sure, if there is a bug at all. According to man git-push:

	The <pushurl> is used for pushes only. It is optional and defaults to
   <url>.
	(From the section REMOTES -> Named remote in configuration file)

the command:
    git remote add foo git@foo-fetch.org/some.git

will set "remote.foo.url" to "git@foo-fetch.org". Subsequently, fetch and push 
will use git@foo-fetch.org as url.
Fetch will use this url, because "remote.foo.url" explicitly sets this. push 
will use it in absence of a "remote.foo.pushurl".

Now, we're adding a push-url:
    git remote set-url --add --push foo git@foo-push.org/some.git

Relevant parts of config are now looking like:
	[remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-push.org/some.git

Since, pushurl is now given explicitly, git push will use that one (and only 
that one).

If we add another push-url now,
    git remote set-url --add --push foo git@foo-push-also.org/some.git

the next git-push will push to foo-push.org and foo-push-also.org.

Now, using --set-url --delete on both of these urls restores the original 
state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to 
url again.

To me this is exactly what Jardel was observing:

In step 2, Git replaced the original push URL instead of adding a new
one. But it seems to happen only the first time I use `remote set-url
--add --push`. Re-adding the original URL using the same command seems
to work properly.

And FWIW, if I delete (with "set-url --delete") both URLs push, Git
restores the original URL.

Or am I missing something here?

Might be that the "bug" actually is that the expectation was

	git remote add foo git@foo-fetch.org/some.git

should have created a config like:

	[remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-fetch.org/some.git

since that is what "git remote -v" reports.

If that is the case, we might want to amend the output of 'git remote -v' with 
the information that a pushurl is not explicitly given and thus defaults to 
url.

Sascha

On Sat, Jan 12, 2013 at 6:44 AM, Sascha Cunz [off-list ref] wrote:

Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:

quoted

Jardel Weyrich [off-list ref] writes:

quoted

I believe `remote set-url --add --push` has a bug. Performed tests
with v1.8.0.1 and v1.8.1 (Mac OS X).

Quoting the relevant part of the documentation:

quoted

set-url

    Changes URL remote points to. Sets first URL remote points to
    matching regex <oldurl> (first URL if no <oldurl> is given) to
    <newurl>. If <oldurl> doesn’t match any URL, error occurs and
    nothing is changed.

    With --push, push URLs are manipulated instead of fetch URLs.
    With --add, instead of changing some URL, new URL is added.
    With --delete, instead of changing some URL, all URLs matching regex
    <url> are deleted. Trying to delete all non-push URLs is an error.>

Here are some steps to reproduce:

1. Show the remote URLs

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test (push)

2. Add a new push URL for origin

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
origin \>
    /Volumes/sandbox/test_clone2

3. Check what happened

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test_clone2 (push)

The original pushurl was replaced with the additional one, instead
of being left and the new one getting added.  That looks certainly
wrong.

However, the result of applying the attached patch (either to
v1.7.12 or v1.8.1) still passes the test and I do not think it is
doing anything differently from what you described above.

What do you get from

      git config -l | grep '^remote\.origin'

in steps 1. and 3. in your procedure?  This question is trying to
tell if your bug is in "git remote -v" or in "git remote set-url".

I'm not sure, if there is a bug at all. According to man git-push:

        The <pushurl> is used for pushes only. It is optional and defaults to
   <url>.
        (From the section REMOTES -> Named remote in configuration file)

the command:
    git remote add foo git@foo-fetch.org/some.git

will set "remote.foo.url" to "git@foo-fetch.org". Subsequently, fetch and push
will use git@foo-fetch.org as url.
Fetch will use this url, because "remote.foo.url" explicitly sets this. push
will use it in absence of a "remote.foo.pushurl".

Now, we're adding a push-url:
    git remote set-url --add --push foo git@foo-push.org/some.git

Relevant parts of config are now looking like:
        [remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-push.org/some.git

Since, pushurl is now given explicitly, git push will use that one (and only
that one).

If we add another push-url now,
    git remote set-url --add --push foo git@foo-push-also.org/some.git

the next git-push will push to foo-push.org and foo-push-also.org.

Now, using --set-url --delete on both of these urls restores the original
state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to
url again.

To me this is exactly what Jardel was observing:

quoted

In step 2, Git replaced the original push URL instead of adding a new
one. But it seems to happen only the first time I use `remote set-url
--add --push`. Re-adding the original URL using the same command seems
to work properly.

quoted

And FWIW, if I delete (with "set-url --delete") both URLs push, Git
restores the original URL.

Or am I missing something here?

You're right. However, as I quoted earlier, the git-remote man-page states:

       set-url
           Changes URL remote points to. <suppressed>
           With --push, push URLs are manipulated instead of fetch URLs.
           With --add, instead of changing some URL, new URL is added.

It explicitly mentions that it should **add a new URL**.
So when I do `git remote set-url --add --push origin
git://another/repo.git`, I expect git-push to use both the default
push URL and the new one. Or am I misinterpreting the man-page?

Might be that the "bug" actually is that the expectation was

        git remote add foo git@foo-fetch.org/some.git

should have created a config like:

        [remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-fetch.org/some.git

since that is what "git remote -v" reports.

If that is the case, we might want to amend the output of 'git remote -v' with
the information that a pushurl is not explicitly given and thus defaults to
url.

Correct. Adding a remote doesn't automatically generate a pushurl for it.

To me, it seems that git-push checks for the existence of pushurl's in
the config, and if it finds any, it ignores the defaul push URL during
the actual push. In better words, it pushes only to pushurls, if it
finds any, otherwise it pushes to the default URL.

To comply with the statements in the git-remote man-page, git-remote
should add a pushurl configuration containing the default push URL,
along with the one passed to `set-url --add --push`. Or git-push
should _not ignore_ the default URL in the presence of pushurls,
effectively pushing to both. These are the solutions I can think of
right now, supposing I'm correct about the cause(s).

Sascha

Jardel Weyrich venit, vidit, dixit 12.01.2013 10:33:

On Sat, Jan 12, 2013 at 6:44 AM, Sascha Cunz [off-list ref] wrote:

quoted

Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:

quoted

Jardel Weyrich [off-list ref] writes:

quoted

I believe `remote set-url --add --push` has a bug. Performed tests
with v1.8.0.1 and v1.8.1 (Mac OS X).

Quoting the relevant part of the documentation:

quoted

set-url

    Changes URL remote points to. Sets first URL remote points to
    matching regex <oldurl> (first URL if no <oldurl> is given) to
    <newurl>. If <oldurl> doesn’t match any URL, error occurs and
    nothing is changed.

    With --push, push URLs are manipulated instead of fetch URLs.
    With --add, instead of changing some URL, new URL is added.
    With --delete, instead of changing some URL, all URLs matching regex
    <url> are deleted. Trying to delete all non-push URLs is an error.>

Here are some steps to reproduce:

1. Show the remote URLs

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test (push)

2. Add a new push URL for origin

jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
origin \>
    /Volumes/sandbox/test_clone2

3. Check what happened

jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin  /Volumes/sandbox/test (fetch)
origin  /Volumes/sandbox/test_clone2 (push)

The original pushurl was replaced with the additional one, instead
of being left and the new one getting added.  That looks certainly
wrong.

However, the result of applying the attached patch (either to
v1.7.12 or v1.8.1) still passes the test and I do not think it is
doing anything differently from what you described above.

What do you get from

      git config -l | grep '^remote\.origin'

in steps 1. and 3. in your procedure?  This question is trying to
tell if your bug is in "git remote -v" or in "git remote set-url".

I'm not sure, if there is a bug at all. According to man git-push:

        The <pushurl> is used for pushes only. It is optional and defaults to
   <url>.
        (From the section REMOTES -> Named remote in configuration file)

the command:
    git remote add foo git@foo-fetch.org/some.git

will set "remote.foo.url" to "git@foo-fetch.org". Subsequently, fetch and push
will use git@foo-fetch.org as url.
Fetch will use this url, because "remote.foo.url" explicitly sets this. push
will use it in absence of a "remote.foo.pushurl".

Now, we're adding a push-url:
    git remote set-url --add --push foo git@foo-push.org/some.git

Relevant parts of config are now looking like:
        [remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-push.org/some.git

Since, pushurl is now given explicitly, git push will use that one (and only
that one).

If we add another push-url now,
    git remote set-url --add --push foo git@foo-push-also.org/some.git

the next git-push will push to foo-push.org and foo-push-also.org.

Now, using --set-url --delete on both of these urls restores the original
state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to
url again.

To me this is exactly what Jardel was observing:

quoted

In step 2, Git replaced the original push URL instead of adding a new
one. But it seems to happen only the first time I use `remote set-url
--add --push`. Re-adding the original URL using the same command seems
to work properly.

quoted

And FWIW, if I delete (with "set-url --delete") both URLs push, Git
restores the original URL.

Or am I missing something here?

You're right. However, as I quoted earlier, the git-remote man-page states:

       set-url
           Changes URL remote points to. <suppressed>
           With --push, push URLs are manipulated instead of fetch URLs.
           With --add, instead of changing some URL, new URL is added.

It explicitly mentions that it should **add a new URL**.
So when I do `git remote set-url --add --push origin
git://another/repo.git`, I expect git-push to use both the default
push URL and the new one. Or am I misinterpreting the man-page?

quoted

Might be that the "bug" actually is that the expectation was

        git remote add foo git@foo-fetch.org/some.git

should have created a config like:

        [remote "foo"]
        url = git@foo-fetch.org/some.git
        pushurl = git@foo-fetch.org/some.git

since that is what "git remote -v" reports.

If that is the case, we might want to amend the output of 'git remote -v' with
the information that a pushurl is not explicitly given and thus defaults to
url.

Correct. Adding a remote doesn't automatically generate a pushurl for it.

To me, it seems that git-push checks for the existence of pushurl's in
the config, and if it finds any, it ignores the defaul push URL during
the actual push. In better words, it pushes only to pushurls, if it
finds any, otherwise it pushes to the default URL.

To comply with the statements in the git-remote man-page, git-remote
should add a pushurl configuration containing the default push URL,
along with the one passed to `set-url --add --push`. Or git-push
should _not ignore_ the default URL in the presence of pushurls,
effectively pushing to both. These are the solutions I can think of
right now, supposing I'm correct about the cause(s).

quoted

Sascha

All that "set-url --push --add" does is adding a remote.foo.pushurl
entry to the config. If there was none, there will be one after that.

If there is no pushurl entry, "push" takes the url entry instead. This
is the "default URL for push", but not a pushurl entry.

It seems to me that everything works as designed, and that the man page
talk about "push URLs" can be read in two ways, one of which is correct
(and which is obvious if you know the above, i.e. the "config
background") and one of which is incorrect (and which may be obvious if
you read just that man page paragraph).

Michael

Michael J Gruber wrote:

All that "set-url --push --add" does is adding a remote.foo.pushurl
entry to the config. If there was none, there will be one after that.

If there is no pushurl entry, "push" takes the url entry instead. This
is the "default URL for push", but not a pushurl entry.

That is how it is implemented, but it is hard for me with a straight
face to say that is what most users expect.

Wouldn't the least confusing thing be to just error out for "set-url
--push --add" when there is no existing pushurl?  That way, the
operator can use plain "set-url --push" to clarify whether whether he
meant to include the pull URLs in the new pushurl set.

My two cents,
Jonathan