remote: add new --fetch option for set-url
RFC PATCH 1/1 remote: add new --fetch option for set-url
git remote set-url knew about the '--push' option to update just the pushurl, but it does not have a similar option for "update fetch URL and leave whatever was in place for the push URL".
This patch adds support for a '--fetch' option which implements that use case in a backwards compatible way: if neither --push nor --fetch are given, then the push URL is modified too if it was not set before. This is the case since the push URL is implicitly based on the fetch URL.
The documentation has also been updated and a missing '--push' option is added to the 'git remote -h' command.
Tests are also added to verify the documented behavior.
---
(please cc me, I am not subscribed)
Hi,
Now and then I hit this issue where I want to change the remote where sources are fetched from without having to touch the push URL. Example:
git remote add gh [email protected]:Lekensteyn/git.git
# Avoid needing SSH for pulling from a repo, so change fetch URL git remote set-url https://github.com/Lekensteyn/git.git
# Hmm, the fetch URL got changed too, let's fix that. git remote add --push gh [email protected]:Lekensteyn/git.git
After this patch, I can use:
git remote add gh [email protected]:Lekensteyn/git.git git remote set-url --fetch https://github.com/Lekensteyn/git.git
rather than being forced into a specific order:
git remote add gh https://github.com/Lekensteyn/git.git
git remote set-url --push gh [email protected]:Lekensteyn/git.git
The functionality is implemented, but it might need a refactoring to reduce duplicate code. Translation strings also need to be updated (the current 'git remote set-url' strings are also not up to date).
Kind regards.
Peter
--- Documentation/git-remote.txt | 12 +++-- builtin/remote.c | 106 +++++++++++++++++++++++++++++-------------- t/t5505-remote.sh | 53 ++++++++++++++++++++++ 3 files changed, 133 insertions(+), 38 deletions(-)
Documentation/git-remote.txt +8 -4
@@ -15,9 +15,9 @@ SYNOPSIS 'git remote remove' <name> 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>) 'git remote set-branches' [--add] <name> <branch>... -'git remote set-url' [--push] <name> <newurl> [<oldurl>] -'git remote set-url --add' [--push] <name> <newurl> -'git remote set-url --delete' [--push] <name> <url> +'git remote set-url' [--fetch] [--push] <name> <newurl> [<oldurl>] +'git remote set-url' [--fetch] [--push] '--add' <name> <newurl> +'git remote set-url' [--fetch] [--push] '--delete' <name> <url> 'git remote' [-v | --verbose] 'show' [-n] <name>... 'git remote prune' [-n | --dry-run] <name>... 'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
@@ -134,7 +134,11 @@ 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 '--push', push URLs are manipulated. ++ +With '--fetch', fetch URLs are manipulated. If neither '--push' nor '--fetch' +are given, then (1) '--fetch' is assumed if no push URL is set in the +configuration or (2) '--fetch --push' otherwise. + With '--add', instead of changing some URL, new URL is added. +
builtin/remote.c +72 -34
154 lines not on this page · raw message
t/t5505-remote.sh +53 -0
59 lines not on this page · raw message
Signature · 1 line
2.1.2
Quoted 3 lines · Peter Wu
git remote set-url knew about the '--push' option to update just the pushurl, but it does not have a similar option for "update fetch URL and leave whatever was in place for the push URL".
Isn't that what:
git remote set-url foo new-fetch-url
does already? It affects only the "url" setting, which is the de-facto fetch setting (it is _also_ the push setting if there is no pushurl defined).
You gave this example:
Quoted 5 lines
git remote add gh [email protected]:Lekensteyn/git.git
# Avoid needing SSH for pulling from a repo, so change fetch URL git remote set-url https://github.com/Lekensteyn/git.git
# Hmm, the fetch URL got changed too, let's fix that. git remote add --push gh [email protected]:Lekensteyn/git.git
But here you do not have a pushurl defined in the first place. So I guess this is really just a shortcut for swapping the two, like:
git remote set-url --push gh $(git config remote.gh.url) git remote set-url gh new-fetch-url
I dunno. I guess that is more convenient, but it seems like a lot of code for a very marginal use case. But more importantly, I'm a little worried that the presence of --fetch creates confusion about what set-url without a --fetch or --push does. That is, it implies to me that:
git remote add gh old-url git remote set-url gh --push push-url git remote set-url gh new-url
would replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
-Peff
Quoted 6 lines · On Wednesday 19 November 2014 14:08:00 Jeff King wrote:
Quoted 3 lines · Peter Wu
git remote set-url knew about the '--push' option to update just the pushurl, but it does not have a similar option for "update fetch URL and leave whatever was in place for the push URL".
Quoted 7 lines
Isn't that what:
git remote set-url foo new-fetch-url
does already? It affects only the "url" setting, which is the de-facto fetch setting (it is _also_ the push setting if there is no pushurl defined).
If pushurl is not set, then this will implicitly change the fetch URL which is unwanted and fixed in this patch by introducing a --fetch option.
Quoted 8 lines
You gave this example:
Quoted 5 lines
git remote add gh [email protected]:Lekensteyn/git.git
# Avoid needing SSH for pulling from a repo, so change fetch URL git remote set-url https://github.com/Lekensteyn/git.git
# Hmm, the fetch URL got changed too, let's fix that. git remote add --push gh [email protected]:Lekensteyn/git.git
Quoted 5 lines
But here you do not have a pushurl defined in the first place. So I guess this is really just a shortcut for swapping the two, like:
git remote set-url --push gh $(git config remote.gh.url) git remote set-url gh new-fetch-url
Indeed, and not having a push URL is not uncommon (that is, always the case when a new remote is added or just cloned). Compare the above against this one:
git remote set-url --fetch new-fetch-url
This is less verbose and is much more intuitive.
Quoted 6 lines
I dunno. I guess that is more convenient, but it seems like a lot of code for a very marginal use case. But more importantly, I'm a little worried that the presence of --fetch creates confusion about what set-url without a --fetch or --push does. That is, it implies to me that:
Quoted 8 lines
git remote add gh old-url git remote set-url gh --push push-url git remote set-url gh new-urlwould replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
Step three currently only replaces the fetch URL as an explicit push URL (remote.gh.pushurl) is set in step two (and therefore remote.gh.url does not become the implicit push URL).
This might be a bug, but since it has been so long this way I was worried that people actually rely on this behavior. The following sets both url and pushurl (even if the URLs are equal):
git remote set-url --push --fetch gh pushurl
The patch is not tiny, but also not overly large either even if it has similar pieces of code duplicated. Care has been taken of to keep backwards compatibility which has also increased the size.
By the way, in the old code there was a memleak as strbuf was not released at the end of the function, isn't it? A new patch can be found on the bottom, it fixes an issue in the tests (no change in other parts).
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 1 line
From
6be164f68b4f625ac90f998ae4073e031ebed8baMon Sep 17 00:00:00 2001
From: Peter Wu <[email protected]>
Date: Wed, 19 Nov 2014 15:57:40 +0100
Subject: [PATCHv2] remote: add new --fetch option for set-url
git remote set-url knew about the '--push' option to update just the pushurl, but it does not have a similar option for "update fetch URL and leave whatever was in place for the push URL".
This patch adds support for a '--fetch' option which implements that use case in a backwards compatible way: if neither --push nor --fetch are given, then the push URL is modified too if it was not set before. This is the case since the push URL is implicitly based on the fetch URL.
The documentation has also been updated and a missing '--push' option is added to the 'git remote -h' command.
Tests are also added to verify the documented behavior.
--- Documentation/git-remote.txt | 12 +++-- builtin/remote.c | 106 +++++++++++++++++++++++++++++-------------- t/t5505-remote.sh | 54 ++++++++++++++++++++++ 3 files changed, 134 insertions(+), 38 deletions(-)
Documentation/git-remote.txt +8 -4
@@ -15,9 +15,9 @@ SYNOPSIS 'git remote remove' <name> 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>) 'git remote set-branches' [--add] <name> <branch>... -'git remote set-url' [--push] <name> <newurl> [<oldurl>] -'git remote set-url --add' [--push] <name> <newurl> -'git remote set-url --delete' [--push] <name> <url> +'git remote set-url' [--fetch] [--push] <name> <newurl> [<oldurl>] +'git remote set-url' [--fetch] [--push] '--add' <name> <newurl> +'git remote set-url' [--fetch] [--push] '--delete' <name> <url> 'git remote' [-v | --verbose] 'show' [-n] <name>... 'git remote prune' [-n | --dry-run] <name>... 'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
@@ -134,7 +134,11 @@ 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 '--push', push URLs are manipulated. ++ +With '--fetch', fetch URLs are manipulated. If neither '--push' nor '--fetch' +are given, then (1) '--fetch' is assumed if no push URL is set in the +configuration or (2) '--fetch --push' otherwise. + With '--add', instead of changing some URL, new URL is added. +
builtin/remote.c +72 -34
154 lines not on this page · raw message
t/t5505-remote.sh +54 -0
60 lines not on this page · raw message
Signature · 1 line
2.1.2
Quoted 10 lines · Peter Wu
Quoted 2 lines
git remote set-url --push gh $(git config remote.gh.url) git remote set-url gh new-fetch-urlIndeed, and not having a push URL is not uncommon (that is, always the case when a new remote is added or just cloned). Compare the above against this one:
git remote set-url --fetch new-fetch-urlThis is less verbose and is much more intuitive.
I agree your suggestion is a nicer way to do this. I'm just not sure that this swapping is all that common an operation. If there were no cost, I wouldn't mind. But I'm mostly concerned about the funny, non-intuitive behavior of "git remote set-url foo" that is created by this.
Quoted 11 lines
Quoted 4 lines
would replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
Step three currently only replaces the fetch URL as an explicit push URL (remote.gh.pushurl) is set in step two (and therefore remote.gh.url does not become the implicit push URL).
This might be a bug, but since it has been so long this way I was worried that people actually rely on this behavior.
I don't think this is a bug. I think that "git fetch set-url" without "--push" is a de-facto "--fetch" already. Which makes sense, as there isn't a "--fetch" now (and the "--push" variant and "pushurl" grew after the fact, so the "url" option serves double-duty as both the single url and the "fetch" half).
And that's what makes the proposed interface funny. Omitting "--fetch" is already a de-facto "--fetch", and sometimes the two behave the same, and sometimes differently. Calling the option "--keep-push" would be a more accurate description, but that is rather clunky.
Quoted 3 lines
The patch is not tiny, but also not overly large either even if it has similar pieces of code duplicated. Care has been taken of to keep backwards compatibility which has also increased the size.
Yeah, sorry, I shouldn't have even mentioned patch size. I appreciate your attention to backwards-compatibility, and the patch should be as large as it needs to safely implement the desired behavior. It's really the resulting interface that I'm a bit negative on (which can be related to code size, in the sense that a simple interface can often be implemented simply, but here we have to deal with historical corner cases).
So I dunno. I do not think what you are proposing is the end of the world, but it would be nice to resolve the inconsistency I mentioned. Perhaps we can see what others think.
Quoted 2 lines
By the way, in the old code there was a memleak as strbuf was not released at the end of the function, isn't it?
Yes. We are often quite lazy about such leaks in functions that are known to return straight to the program exit, but it does not hurt to clean up here.
-Peff
Quoted 10 lines · On Wednesday 19 November 2014 15:17:21 Jeff King wrote:
Quoted 3 lines · Peter Wu
git remote set-url --fetch new-fetch-urlThis is less verbose and is much more intuitive.
I agree your suggestion is a nicer way to do this. I'm just not sure that this swapping is all that common an operation. If there were no cost, I wouldn't mind. But I'm mostly concerned about the funny, non-intuitive behavior of "git remote set-url foo" that is created by this.
It is not about swapping, but setting a new fetch while keeping the push URL intact. Likely not common, but nice to have for the times it is needed. I am sure there are other features which are not used a lot, but still exist ;)
Quoted 18 lines
Quoted 11 lines
Quoted 4 lines
would replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
Step three currently only replaces the fetch URL as an explicit push URL (remote.gh.pushurl) is set in step two (and therefore remote.gh.url does not become the implicit push URL).
This might be a bug, but since it has been so long this way I was worried that people actually rely on this behavior.
I don't think this is a bug. I think that "git fetch set-url" without "--push" is a de-facto "--fetch" already. Which makes sense, as there isn't a "--fetch" now (and the "--push" variant and "pushurl" grew after the fact, so the "url" option serves double-duty as both the single url and the "fetch" half).
Quoted 4 lines
And that's what makes the proposed interface funny. Omitting "--fetch" is already a de-facto "--fetch", and sometimes the two behave the same, and sometimes differently. Calling the option "--keep-push" would be a more accurate description, but that is rather clunky.
Before this patch I did not even know that "git remote set-url name url" would have different user-visible behavior depending on whether a pushurl is set or not. In my opinion, the proposed functionality does not make the interface more confusing. Instead, the new option establish a behavior which is consistent with the existing '--push' name.
(Aside, I intended to name this option "--pull" which seemed more natural given the opposite direction "--push", but decided to stay consistent with the "git remote show" terminology.)
I think that your confusion is caused by the meaning of '--push' and '--fetch'. These options form a group and are not as independent as "--add". Something like "--change=[push|fetch|all]" would describe the functionality better:
git remote set-url --change=fetch gh fetchurl
But then the "--push" option becomes redundant.
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 5 lines · Jeff King
But here you do not have a pushurl defined in the first place. So I guess this is really just a shortcut for swapping the two, like:
git remote set-url --push gh $(git config remote.gh.url) git remote set-url gh new-fetch-url
It seems that this swapping is only necessary because the repository is set up in this way:
$ browser github.com
... fork upstream to your own publishing repository ...
$ git clone <your publish repo>
... oops, I am set up to fetch from myself ...
$ git remote set-url --push mine <url for your publish repo>
$ git remote set-url <url for your upstream repo>If you are fetching from somebody else and then pushing into your own publishing repository (i.e. fork of that upstream), why isn't the sequence of event like this, instead?
$ git clone $upstream
$ browser github.com
... fork upstream to your own publishing repository ...
$ git remote set-url --push mine <url for your publish repo>Isn't this one of those bad workflows encouraged by GitHub, for which you guys have to be punished ;-)?
Quoted 22 lines · Junio C Hamano
Quoted 5 lines · Jeff King
But here you do not have a pushurl defined in the first place. So I guess this is really just a shortcut for swapping the two, like:
git remote set-url --push gh $(git config remote.gh.url) git remote set-url gh new-fetch-urlIt seems that this swapping is only necessary because the repository is set up in this way:
$ browser github.com ... fork upstream to your own publishing repository ... $ git clone <your publish repo> ... oops, I am set up to fetch from myself ... $ git remote set-url --push mine <url for your publish repo> $ git remote set-url <url for your upstream repo>If you are fetching from somebody else and then pushing into your own publishing repository (i.e. fork of that upstream), why isn't the sequence of event like this, instead?
Quoted 7 lines
$ git clone $upstream $ browser github.com ... fork upstream to your own publishing repository ... $ git remote set-url --push mine <url for your publish repo>Isn't this one of those bad workflows encouraged by GitHub, for which you guys have to be punished ;-)?
I re-read the original and Peter is really accessing the same repository, only over different transports. Cloning via ssh:// but later deciding to push via ssh:// but to fetch via https:// (or vice versa; the important point is that clone was done over a transport that is "wrong" for future fetching).
Of course, if you cloned via a wrong transport that is not suitable for whatever reason for later fetching, you would need to have a way to "swap", so the observation I made in the message I am following up on does not apply to this thread at all. Please scratch all of the above.
Coming back to the topic, how common would this "oops, I cloned via a wrong transport" be? I am not opposed to giving a recovery method for gotcha that does not happen very often, but if such an addition adds undue confusion factor for people who use "set-url" for more common cases, that would be a bad trade-off.
Quoted 6 lines · On Wednesday 19 November 2014 13:18:56 Junio C Hamano wrote:
Quoted 5 lines · Junio C Hamano
Jeff King <[email protected]> writes:
If you are fetching from somebody else and then pushing into your own publishing repository (i.e. fork of that upstream), why isn't the sequence of event like this, instead?
Quoted 7 lines
Quoted 7 lines
$ git clone $upstream $ browser github.com ... fork upstream to your own publishing repository ... $ git remote set-url --push mine <url for your publish repo>Isn't this one of those bad workflows encouraged by GitHub, for which you guys have to be punished ;-)?
For "forks", it usually goes like this:
git clone $upstream
... realizes that is has a bug which I want to fix ... ... creates a new repo ...
git remote rename origin upstream
git remote add origin git@$personal_repo
# "--fetch" is what I need
git remote add --fetch https://$personal_repo
I often start by entering/copying the ssh URL which is what I need for pushing. Later ssh-agent forget about my key and I realize that push works fine over https, so would like to set that... only to observe that is not possible in an straightforward way through 'git remote'.
Quoted 5 lines
Coming back to the topic, how common would this "oops, I cloned via a wrong transport" be? I am not opposed to giving a recovery method for gotcha that does not happen very often, but if such an addition adds undue confusion factor for people who use "set-url" for more common cases, that would be a bad trade-off.
Well, people rarely need to use 'git remote' except when, well, they need to modify the remotes. Where does the confusion come from? I might be biased now that I know the internals. Maybe the https/ssh case above needs to be mentioned in the documentation? What do you think of the updated documentation by the way?
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
ping?
I asked around and the people who know of git remote fell in these two categories:
- those who know of this "bug" and then first set the fetch URL and then the push URL.
- those who did not expect the current behavior.
The command 'git remote set-url NAME URL' reads as "set the URL(s) for remote NAME to URL". Currently it means "set the fetch (and push) URL of remote NAME to URL" (depending on whether pushurl is set).
I propose to add the option --fetch next to --push with the meaning "set the fetch/push URL of remote NAME to URL". Then --fetch --push means "set the fetch and push URL of remote NAME to URL". In a future git version, this could be made the default option to avoid surprises (which would be backwards incompatible though).
As for the changelog entry,
The "git remote set-url" command now allows you to change just the fetch URL without modifying the push URL using the new --fetch option. For symmetry with the --push option.
("symmetry" in the eyes of the user, not how it is implemented in the git config.)
Opinions?
Quoted 41 lines · On Wednesday 19 November 2014 22:28:35 Peter Wu wrote:
Quoted 6 lines · On Wednesday 19 November 2014 13:18:56 Junio C Hamano wrote:
Quoted 5 lines · Junio C Hamano
Jeff King <[email protected]> writes:
If you are fetching from somebody else and then pushing into your own publishing repository (i.e. fork of that upstream), why isn't the sequence of event like this, instead?Quoted 7 lines
Quoted 7 lines
$ git clone $upstream $ browser github.com ... fork upstream to your own publishing repository ... $ git remote set-url --push mine <url for your publish repo>Isn't this one of those bad workflows encouraged by GitHub, for which you guys have to be punished ;-)?
For "forks", it usually goes like this:
git clone $upstream
... realizes that is has a bug which I want to fix ... ... creates a new repo ...
git remote rename origin upstream
git remote add origin git@$personal_repo
# "--fetch" is what I need
git remote add --fetch https://$personal_repoI often start by entering/copying the ssh URL which is what I need for pushing. Later ssh-agent forget about my key and I realize that push works fine over https, so would like to set that... only to observe that is not possible in an straightforward way through 'git remote'.
Quoted 5 lines
Coming back to the topic, how common would this "oops, I cloned via a wrong transport" be? I am not opposed to giving a recovery method for gotcha that does not happen very often, but if such an addition adds undue confusion factor for people who use "set-url" for more common cases, that would be a bad trade-off.
Well, people rarely need to use 'git remote' except when, well, they need to modify the remotes. Where does the confusion come from? I might be biased now that I know the internals. Maybe the https/ssh case above needs to be mentioned in the documentation? What do you think of the updated documentation by the way?
Quoted 3 lines · Peter Wu
I propose to add the option --fetch next to --push with the meaning "set the fetch/push URL of remote NAME to URL". Then --fetch --push means "set the fetch and push URL of remote NAME to URL".
What would (and "should") the configuration look like after you did this?
git remote set-url nick $url1
git remote set-url nick --push $url2
git remote set-url nick $url3
Whatever happens without your patch after the above is what the current users (i.e. those who do not use the --fetch option) expect, so if the behaviour does not change with your patch, then there is one less incompatibilities to worry about.
A new option "--fetch" introducing a different behaviour is perfectly fine; existing users who are not using it will not be harmed by sudden behaviour change.
Quoted 3 lines
In a future git
version, this could be made the default option to avoid surprises (which would be backwards incompatible though).
I am not sure what you mean "by default". If you mean "set both if remote.nick.pushurl does not exist but otherwise update only remote.nick.url", then the sequence
git remote set-url nick $url1
git remote set-url nick --push $url2
git remote set-url nick $url3
would retain the current behaviour, so it probably is OK.
If you mean to always set remote.nick.url and remote.nick.pushurl pointing at the same value when neither --fetch nor --push is given, That would make the sequence behave quite different from what people would expect, and you would need to devise a transition plan to first start warning when the user did something that will behave differently between the current version and the future version without changing the behaviour, then switch the behaviour but keep warning and finally remove the warning, or something like that.
And the above three-command sequence may not be the only case where the change you are proposing may hurt existing users.
Quoted 13 lines · On Monday 24 November 2014 14:04:07 Junio C Hamano wrote:
Quoted 3 lines · Peter Wu
I propose to add the option --fetch next to --push with the meaning "set the fetch/push URL of remote NAME to URL". Then --fetch --push means "set the fetch and push URL of remote NAME to URL".
What would (and "should") the configuration look like after you did this?
git remote set-url nick $url1
git remote set-url nick --push $url2
git remote set-url nick $url3
Quoted 8 lines
Whatever happens without your patch after the above is what the current users (i.e. those who do not use the --fetch option) expect, so if the behaviour does not change with your patch, then there is one less incompatibilities to worry about.
A new option "--fetch" introducing a different behaviour is perfectly fine; existing users who are not using it will not be harmed by sudden behaviour change.
As stated before, I took care to avoid backwards incompatibilities. The command will still work as expected by the users who are aware of this particular behavior. What I am suggesting (and which is independent of the patch) is to make the command have a more consistent behavior. Either it should set the fetch URL, or both the fetch and push URL, but not vary its behavior depending on whether a push URL is set or not. That should make the behavior of the command more consistent.
Quoted 22 lines
Quoted 2 lines
In a future git version, this could be made the default option to avoid surprises (which would be backwards incompatible though).
I am not sure what you mean "by default". If you mean "set both if remote.nick.pushurl does not exist but otherwise update only remote.nick.url", then the sequence
git remote set-url nick $url1
git remote set-url nick --push $url2
git remote set-url nick $url3would retain the current behaviour, so it probably is OK.
If you mean to always set remote.nick.url and remote.nick.pushurl pointing at the same value when neither --fetch nor --push is given, That would make the sequence behave quite different from what people would expect, and you would need to devise a transition plan to first start warning when the user did something that will behave differently between the current version and the future version without changing the behaviour, then switch the behaviour but keep warning and finally remove the warning, or something like that.
Quoted 2 lines
And the above three-command sequence may not be the only case where the change you are proposing may hurt existing users.
The "default" refers to the behavior of "git remote set-url" in absence of "--push" and "--fetch" options. A transition period is expected (if this idea is put forward). Since nobody seems to be bitten by this option, I am not sure if it really adds much value to make this change though.
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 7 lines · Peter Wu
Quoted 3 lines
A new option "--fetch" introducing a different behaviour is perfectly fine; existing users who are not using it will not be harmed by sudden behaviour change.
As stated before, I took care to avoid backwards incompatibilities. The command will still work as expected by the users who are aware of this particular behavior.
Right. My original complaint was only that "--fetch" is not as orthogonal to "--push" (and an optionless set-url) as it could be. I think the alternatives for going forward are basically:
1. Name it something besides --fetch (but that's rather clunky).
2. Migrate to new behavior, which is what is being discussed here. Probably needs a transition period?
3. Live with it. Probably address the weirdness in the documentation.
4. Do nothing, drop the patch.
I think I'd be OK with (3), with an appropriate documentation update.
-Peff
Quoted 10 lines · On Monday 24 November 2014 17:22:44 Jeff King wrote:
Quoted 7 lines · Peter Wu
Quoted 3 lines
A new option "--fetch" introducing a different behaviour is perfectly fine; existing users who are not using it will not be harmed by sudden behaviour change.
As stated before, I took care to avoid backwards incompatibilities. The command will still work as expected by the users who are aware of this particular behavior.
Quoted 5 lines
Right. My original complaint was only that "--fetch" is not as orthogonal to "--push" (and an optionless set-url) as it could be. I think the alternatives for going forward are basically:
1. Name it something besides --fetch (but that's rather clunky).
It is not orthogonal to --push in the config, but the behavior exposed to the user is orthogonal unless I am missing something?
I can understand that --fetch sounds a bit weird, what about this natural translation:
"git remote: set the URL (only the fetch one) for NAME to URL" git remote set-url --only=fetch NAME URL
"git remote: set the URL (only the push one) for NAME to URL" git remote set-url --only=push NAME URL
(obsoletes --push)
"git remote: set the URL (both) for NAME to URL"
git remote set-url --only=both NAME URL
(it would be nice if --only=both (weird!) can be removed in the future such that the option is more natural)
"git remote: set the URL for NAME to URL"
git remote set-url NAME URL
(current behavior: YOU git guru knows what I do right?)
Quoted 2 lines
2. Migrate to new behavior, which is what is being discussed here. Probably needs a transition period?
A transition period would also help to solicit feedback.
Quoted 5 lines
3. Live with it. Probably address the weirdness in the documentation.
4. Do nothing, drop the patch.
I think I'd be OK with (3), with an appropriate documentation update.
I prefer 1 for now as it avoids the extra manual action I have to take when changing URLs.
Peter
Quoted 8 lines · Peter Wu
Quoted 5 lines
Right. My original complaint was only that "--fetch" is not as orthogonal to "--push" (and an optionless set-url) as it could be. I think the alternatives for going forward are basically:
1. Name it something besides --fetch (but that's rather clunky).
It is not orthogonal to --push in the config, but the behavior exposed to the user is orthogonal unless I am missing something?
My complaint is that you have three possible options to provide: --push, --fetch, or no option at all. And "--fetch" sometimes behaves like no option, and sometimes not. Which is the confusing/non-orthogonal part.
Quoted 10 lines
I can understand that --fetch sounds a bit weird, what about this natural translation:
"git remote: set the URL (only the fetch one) for NAME to URL" git remote set-url --only=fetch NAME URL
"git remote: set the URL (only the push one) for NAME to URL" git remote set-url --only=push NAME URL
(obsoletes --push)
Quoted 8 lines
"git remote: set the URL (both) for NAME to URL"
git remote set-url --only=both NAME URL
(it would be nice if --only=both (weird!) can be removed in the future such that the option is more natural)"git remote: set the URL for NAME to URL"
git remote set-url NAME URL
(current behavior: YOU git guru knows what I do right?)
Yeah, I think that addresses my concern (because it explicitly leaves no-option as a historical curiosity, and not as an implicit version of "--both").
Quoted 8 lines
Quoted 5 lines
3. Live with it. Probably address the weirdness in the documentation.
4. Do nothing, drop the patch.
I think I'd be OK with (3), with an appropriate documentation update.
I prefer 1 for now as it avoids the extra manual action I have to take when changing URLs.
I'm not sure if I was clear on (3), but "live with it" was "live with your original patch". Which I think you would also be happy with.
-Peff
Quoted 16 lines · On Monday 24 November 2014 17:54:57 Jeff King wrote:
Quoted 15 lines · Peter Wu
I can understand that --fetch sounds a bit weird, what about this natural translation:
"git remote: set the URL (only the fetch one) for NAME to URL" git remote set-url --only=fetch NAME URL
"git remote: set the URL (only the push one) for NAME to URL" git remote set-url --only=push NAME URL
(obsoletes --push)"git remote: set the URL (both) for NAME to URL"
git remote set-url --only=both NAME URL
(it would be nice if --only=both (weird!) can be removed in the future such that the option is more natural)
Quoted 7 lines
Quoted 3 lines
"git remote: set the URL for NAME to URL"
git remote set-url NAME URL
(current behavior: YOU git guru knows what I do right?)Yeah, I think that addresses my concern (because it explicitly leaves no-option as a historical curiosity, and not as an implicit version of "--both").
Ok, I will make a clear note about the default (without --only) behavior having weird behavior for historical reasons. Are you really OK with --only=both? It sounds a bit odd (mathematically speaking it is correct as fetch and push are both partitions that form the whole set if you ignore the historical behavior).
Quoted 11 lines
Quoted 8 lines
Quoted 5 lines
3. Live with it. Probably address the weirdness in the documentation.
4. Do nothing, drop the patch.
I think I'd be OK with (3), with an appropriate documentation update.
I prefer 1 for now as it avoids the extra manual action I have to take when changing URLs.
I'm not sure if I was clear on (3), but "live with it" was "live with your original patch". Which I think you would also be happy with.
Oh yes, I misunderstood this one ;)
What about the translations? Should I send a separate patch for that or can I update all translations at once?
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 25 lines · Peter Wu
Quoted 16 lines · On Monday 24 November 2014 17:54:57 Jeff King wrote:
Quoted 15 lines · Peter Wu
I can understand that --fetch sounds a bit weird, what about this natural translation:
"git remote: set the URL (only the fetch one) for NAME to URL" git remote set-url --only=fetch NAME URL
"git remote: set the URL (only the push one) for NAME to URL" git remote set-url --only=push NAME URL
(obsoletes --push)"git remote: set the URL (both) for NAME to URL"
git remote set-url --only=both NAME URL
(it would be nice if --only=both (weird!) can be removed in the future such that the option is more natural)Quoted 7 lines
Quoted 3 lines
"git remote: set the URL for NAME to URL"
git remote set-url NAME URL
(current behavior: YOU git guru knows what I do right?)Yeah, I think that addresses my concern (because it explicitly leaves no-option as a historical curiosity, and not as an implicit version of "--both").
Quoted 5 lines
Ok, I will make a clear note about the default (without --only) behavior having weird behavior for historical reasons. Are you really OK with --only=both? It sounds a bit odd (mathematically speaking it is correct as fetch and push are both partitions that form the whole set if you ignore the historical behavior).
Maybe "--operation={push,fetch,both}" would be less odd (though "--operation" is rather clunky, I could not think of a better word). It is the conjunction of "--only" an "both" that makes little sense.
However, I think what removed the confusion for me in your --only=both proposal was the presence of a "both" option, since it made it more clear that is not what no-option means. So what about just "--push", "--fetch", and "--both"? Explain the current behavior of no-options in the documentation as a historical oddity.
That also gives us an easy path forward for changing the behavior. During the transition period, people should use --push, --fetch, or --both. Using no-options provides a warning. After a settling period, the no-option behavior will switch to one of those (presumably --both), and drop the warning.
You do not have to do the migration path if you don't want to. Adding "--fetch" and "--both" scratches your itch and sets us up to migrate later.
Quoted 2 lines
What about the translations? Should I send a separate patch for that or can I update all translations at once?
You do not have to update the translations. When we near a release, the l10n coordinator will run "make pot" to update po/git.pot with the strings marked for translation, and then the translators will write translations for the new strings. You are of course welcome to help with the translation effort at that stage. :)
Details are in po/README.
-Peff
Quoted 5 lines · On Monday 24 November 2014 23:08:26 Jeff King wrote:
However, I think what removed the confusion for me in your --only=both proposal was the presence of a "both" option, since it made it more clear that is not what no-option means. So what about just "--push", "--fetch", and "--both"? Explain the current behavior of no-options in the documentation as a historical oddity.
Ok, this sounds even better. I have dropped the --only part and made the options --push, --fetch and --both disjoint (overriding each other). A patch will follow soon. Maybe it should warn when you try to specify both options though.
Quoted 9 lines
That also gives us an easy path forward for changing the behavior. During the transition period, people should use --push, --fetch, or --both. Using no-options provides a warning. After a settling period, the no-option behavior will switch to one of those (presumably --both), and drop the warning.
You do not have to do the migration path if you don't want to. Adding "--fetch" and "--both" scratches your itch and sets us up to migrate later.
I have documented the historic behavior and mentioned that it is /possible/ that the option --both becomes default in the future.
Quoted 10 lines
Quoted 2 lines
What about the translations? Should I send a separate patch for that or can I update all translations at once?
You do not have to update the translations. When we near a release, the l10n coordinator will run "make pot" to update po/git.pot with the strings marked for translation, and then the translators will write translations for the new strings. You are of course welcome to help with the translation effort at that stage.
Details are in po/README.
Well, it is not necessary the translations, but the format of them. The format
"git remote set-url --delete <name> <url>" has changed to "git remote set-url [--both | --fetch | --push] --delete <name> <url>" for example. The old strings are still usable, so I wonder whether I can make it easier for the i10n maintainer to recognize this change?
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 4 lines · Jeff King
However, I think what removed the confusion for me in your --only=both proposal was the presence of a "both" option, since it made it more clear that is not what no-option means. So what about just "--push", "--fetch", and "--both"?
I think that is the set that is most sensible, at least syntactically, among the ones I heard so far in this thread.
However, one thing still makes me wonder....
After doing "set-url --fetch" and nothing else, because the user never said "--both" or "--push", does the user get a configuration where "git push" fails? Or does "set-url --fetch" still gives us remote.nick.url and causes "git push" to also go there?
If that is the case, then did addition of "--fetch" achieve anything to reduce confusion?
After doing "set-url --push" and nothing else, I suspect that having remote.nick.pushURL alone without remote.nick.URL will make "git fetch" to fail, which would be in line with my expectation. I just expected anything we do in the name of symmetry or consistency would work the same/symmetric way, I cannot see how "set-url --fetch" would work to make its effect symmetric to the "set-url --push" one.
Puzzled...
Quoted 10 lines · Junio C Hamano
Quoted 4 lines · Jeff King
However, I think what removed the confusion for me in your --only=both proposal was the presence of a "both" option, since it made it more clear that is not what no-option means. So what about just "--push", "--fetch", and "--both"?
I think that is the set that is most sensible, at least syntactically, among the ones I heard so far in this thread.
Quoted 6 lines
However, one thing still makes me wonder....
After doing "set-url --fetch" and nothing else, because the user never said "--both" or "--push", does the user get a configuration where "git push" fails? Or does "set-url --fetch" still gives us remote.nick.url and causes "git push" to also go there?
I think the latter. And that makes sense to me. Push falls back to fetch at runtime. And you never set a push url, so that's what happens.
But it is not symmetric. We do not fall back fetch to push. We _could_, but that is a separate issue (one for git-fetch, and not git-remote). And I do not think it is one anybody is particularly asking for.
We could also stop making push fall back to fetch. But I think people would find that irritating.
I dunno. I think there has always been an implicit "subordinate" relationship between fetch and push URLs, with fetch being the "main" one. Maybe that is so ingrained in me at this point that I do not see a problem with the asymmetry.
-Peff
A message that is not in the archive (sent off-list or not yet received): CAPc5daWh4hnKsTMpaW-TvCmVDfU+rzCezrAHcLgXDG6RVvzXHA@mail.gmail.com
Quoted 3 lines · On Monday 24 November 2014 21:19:16 Junio C Hamano wrote:
Quoted 2 lines · Jeff King
We could also stop making push fall back to fetch. But I think people would find that irritating.
The common case is probably having the same fetch and push URL, so I think that this should not be changed.
Quoted 14 lines
Quoted 4 lines
I dunno. I think there has always been an implicit "subordinate" relationship between fetch and push URLs, with fetch being the "main" one. Maybe that is so ingrained in me at this point that I do not see a problem with the asymmetry.
I actually do not have problem with asymmetry/subordinate relationship myself, but then why are we adding --fetch to complement --push in the first place?
Or perhaps I am misunderstanding the suggested semantics of --both. That "subordinate" relationship really means that remote.nick.URL is the one that is used for both directions when pushURL is not set.
Quoted 7 lines
I misunderstood that --both would add identical value to both remote.nick.URL and remote.nick.pushURL, but that would break the implicit subordinate relationship. Is the suggested semantics of "set-url --both" to first delete remote.nick.pushURL if exists and then to set remote.nick.URL?
If that is what is being proposed, then I think it makes sense.
Yes, your last understanding is correct. For this feature, try to think as the user who does not know about the configuration implementation. That is why the --fetch option was proposed, earlier it did not make sense to me why a --push option exists, but a --fetch option is missing.
Option '--both' will drop the push URL and result in an implicit fallback to the fetch URL. It becomes slightly more hairy in the presence of URL sets (using --add and --delete), but I have also tried to make that act sensibly).
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
From: "Peter Wu" <[email protected]>
Quoted 7 lines
Ok, I will make a clear note about the default (without --only) behavior having weird behavior for historical reasons. Are you really OK with --only=both? It sounds a bit odd (mathematically speaking it is correct
as fetch and push are both partitions that form the whole set if you ignore the historical behavior).
How about :
s/--only/--direction/
or some suitable abbreviation (--dirn ?)
--
Philip
Quoted 9 lines · On Saturday 29 November 2014 13:31:18 Philip Oakley wrote:
From: "Peter Wu" <[email protected]>
Quoted 8 lines
Ok, I will make a clear note about the default (without --only) behavior
having weird behavior for historical reasons. Are you really OK with --only=both? It sounds a bit odd (mathematically speaking it is correct
as fetch and push are both partitions that form the whole set if you ignore the historical behavior).
Quoted 5 lines
How about :
s/--only/--direction/
or some suitable abbreviation (--dirn ?)
In the next version of the patch I went for three separate options, --fetch, --push and --both:
http://article.gmane.org/gmane.comp.version-control.git/260213
(Juno, Jeff: ping?)
The option --direction=<fetch|push|both> is a bit longer and --dirn can be mistaken for "directory N".
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 16 lines · Peter Wu
Quoted 9 lines · On Saturday 29 November 2014 13:31:18 Philip Oakley wrote:
From: "Peter Wu" <[email protected]>
Quoted 8 lines
Ok, I will make a clear note about the default (without --only) behavior
having weird behavior for historical reasons. Are you really OK with --only=both? It sounds a bit odd (mathematically speaking it is correct
as fetch and push are both partitions that form the whole set if you ignore the historical behavior).Quoted 5 lines
How about :
s/--only/--direction/
or some suitable abbreviation (--dirn ?)
Quoted 7 lines
In the next version of the patch I went for three separate options, --fetch, --push and --both:
http://article.gmane.org/gmane.comp.version-control.git/260213
(Juno, Jeff: ping?)The option --direction=<fetch|push|both> is a bit longer and --dirn can be mistaken for "directory N".
If we have to have three variants, --{fetch,push,both} would be the easiest to understand among the possibilities listed above, I would think.
Quoted 19 lines · Jeff King
My complaint is that you have three possible options to provide: --push, --fetch, or no option at all. And "--fetch" sometimes behaves like no option, and sometimes not. Which is the confusing/non-orthogonal part.
Quoted 15 lines
I can understand that --fetch sounds a bit weird, what about this natural translation:
"git remote: set the URL (only the fetch one) for NAME to URL" git remote set-url --only=fetch NAME URL
"git remote: set the URL (only the push one) for NAME to URL" git remote set-url --only=push NAME URL
(obsoletes --push)"git remote: set the URL (both) for NAME to URL"
git remote set-url --only=both NAME URL
(it would be nice if --only=both (weird!) can be removed in the future such that the option is more natural)
Quoted 7 lines
Quoted 3 lines
"git remote: set the URL for NAME to URL"
git remote set-url NAME URL
(current behavior: YOU git guru knows what I do right?)Yeah, I think that addresses my concern (because it explicitly leaves no-option as a historical curiosity, and not as an implicit version of "--both").
Fine by me, too.
Quoted 6 lines · Jeff King
I dunno. I guess that is more convenient, but it seems like a lot of code for a very marginal use case. But more importantly, I'm a little worried that the presence of --fetch creates confusion about what set-url without a --fetch or --push does. That is, it implies to me that:
Quoted 8 lines
git remote add gh old-url git remote set-url gh --push push-url git remote set-url gh new-urlwould replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
Yes, the semantics the updated code gives feel very strange. I wouldn't be able to write a three-line summary in the release notes to advertise what good this new feature brings to users myself.
Quoted 12 lines · On Wednesday 19 November 2014 12:29:47 Junio C Hamano wrote:
Quoted 10 lines · Jeff King
I dunno. I guess that is more convenient, but it seems like a lot of code for a very marginal use case. But more importantly, I'm a little worried that the presence of --fetch creates confusion about what set-url without a --fetch or --push does. That is, it implies to me that:
git remote add gh old-url git remote set-url gh --push push-url git remote set-url gh new-url
Quoted 8 lines
Quoted 4 lines
would replace both the "url" _and_ "pushurl" values in the third step, since we did not specify --fetch. But it is in fact identical whether you run it with "--fetch" or not. That is, it creates a weirdly non-orthogonal interface.
Yes, the semantics the updated code gives feel very strange. I wouldn't be able to write a three-line summary in the release notes to advertise what good this new feature brings to users myself.
What about:
"git remote set-url" learned a new "--fetch" option which can be used to change the fetch URL while leaving the push URL intact. Useful to keep a ssh URL for push and change the fetch URL to https.
which is effectively the functionality I am using it for.
Signature · 3 lines
Kind regards, Peter https://lekensteyn.nl
Quoted 6 lines · Peter Wu
On Wednesday 19 November 2014 12:29:47 Junio C Hamano wrote:
...Quoted 3 lines
Yes, the semantics the updated code gives feel very strange. I wouldn't be able to write a three-line summary in the release notes to advertise what good this new feature brings to users myself.
Quoted 7 lines
What about:
"git remote set-url" learned a new "--fetch" option which can be used to change the fetch URL while leaving the push URL intact. Useful to keep a ssh URL for push and change the fetch URL to https.
which is effectively the functionality I am using it for.
I do not think that clarifies the confusion-factor Peff has been pointing out in this thread, unfortunately. At least not to me.