| Message ID | 20240614103122.GG222445@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9badf97c42474df3f0474a851bdc2e62e59da403 |
| Headers | show |
| Series | allow overriding remote.*.url | expand |
On Fri, Jun 14, 2024 at 3:32 AM Jeff King <peff@peff.net> wrote: > > Because remote.*.url is treated as a multi-valued key, there is no way > to override previous config. So for example if you have > remote.origin.url set to some wrong value, doing: > > git -c remote.origin.url=right fetch > > would not work. It would append "right" to the list, which means we'd > still fetch from "wrong" (since subsequent values are used only as push > urls). > > Let's provide a mechanism to reset the list, like we do for other > multi-valued keys (e.g., credential.helper, http.extraheaders, and > merge.suppressDest all use this "empty string means reset" pattern). > > Reported-by: Mathew George <mathewegeorge@gmail.com> > Signed-off-by: Jeff King <peff@peff.net> > --- > By the way, I think the nearby remote.*.fetch and remote.*.push could > learn the same trick. I left that out of this series, mostly because it > was getting long. But also because I had trouble imagining how a one-off > refspec change would be useful. We can revisit it on top if we want. Yeah, deferring makes sense to me. > Documentation/config/remote.txt | 5 ++++- > remote.c | 10 +++++++-- > t/t5505-remote.sh | 36 +++++++++++++++++++++++++++++++++ > 3 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt > index eef0bf4f62..8efc53e836 100644 > --- a/Documentation/config/remote.txt > +++ b/Documentation/config/remote.txt > @@ -8,13 +8,16 @@ remote.<name>.url:: > linkgit:git-push[1]. A configured remote can have multiple URLs; > in this case the first is used for fetching, and all are used > for pushing (assuming no `remote.<name>.pushurl` is defined). > + Setting this key to the empty string clears the list of urls, > + allowing you to override earlier config. > > remote.<name>.pushurl:: > The push URL of a remote repository. See linkgit:git-push[1]. > If a `pushurl` option is present in a configured remote, it > is used for pushing instead of `remote.<name>.url`. A configured > remote can have multiple push URLs; in this case a push goes to > - all of them. > + all of them. Setting this key to the empty string clears the > + list of urls, allowing you to override earlier config. > > remote.<name>.proxy:: > For remotes that require curl (http, https and ftp), the URL to Thanks for documenting this too. > diff --git a/remote.c b/remote.c > index 9417d83e51..b7262964fb 100644 > --- a/remote.c > +++ b/remote.c > @@ -63,12 +63,18 @@ static char *alias_url(const char *url, struct rewrites *r) > > static void add_url(struct remote *remote, const char *url) > { > - strvec_push(&remote->url, url); > + if (*url) > + strvec_push(&remote->url, url); > + else > + strvec_clear(&remote->url); > } > > static void add_pushurl(struct remote *remote, const char *pushurl) > { > - strvec_push(&remote->pushurl, pushurl); > + if (*pushurl) > + strvec_push(&remote->pushurl, pushurl); > + else > + strvec_clear(&remote->pushurl); > } Nice that after the preparation patches the fix is nice and simple. > static void add_pushurl_alias(struct remote_state *remote_state, > diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh > index 7789ff12c4..08424e878e 100755 > --- a/t/t5505-remote.sh > +++ b/t/t5505-remote.sh > @@ -1492,4 +1492,40 @@ test_expect_success 'refs/remotes/* <src> refspec and unqualified <dst> DWIM and > ) > ' > > +test_expect_success 'empty config clears remote.*.url list' ' > + test_when_finished "git config --remove-section remote.multi" && > + git config --add remote.multi.url wrong-one && > + git config --add remote.multi.url wrong-two && > + git -c remote.multi.url= \ > + -c remote.multi.url=right-one \ > + -c remote.multi.url=right-two \ > + remote show -n multi >actual.raw && > + grep URL actual.raw >actual && > + cat >expect <<-\EOF && > + Fetch URL: right-one > + Push URL: right-one > + Push URL: right-two > + EOF > + test_cmp expect actual > +' > + > +test_expect_success 'empty config clears remote.*.pushurl list' ' > + test_when_finished "git config --remove-section remote.multi" && > + git config --add remote.multi.url right && > + git config --add remote.multi.url will-be-ignored && > + git config --add remote.multi.pushurl wrong-push-one && > + git config --add remote.multi.pushurl wrong-push-two && > + git -c remote.multi.pushurl= \ > + -c remote.multi.pushurl=right-push-one \ > + -c remote.multi.pushurl=right-push-two \ > + remote show -n multi >actual.raw && > + grep URL actual.raw >actual && > + cat >expect <<-\EOF && > + Fetch URL: right > + Push URL: right-push-one > + Push URL: right-push-two > + EOF > + test_cmp expect actual > +' > + > test_done > -- > 2.45.2.937.g0bcb3c087a Make sense.
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt index eef0bf4f62..8efc53e836 100644 --- a/Documentation/config/remote.txt +++ b/Documentation/config/remote.txt @@ -8,13 +8,16 @@ remote.<name>.url:: linkgit:git-push[1]. A configured remote can have multiple URLs; in this case the first is used for fetching, and all are used for pushing (assuming no `remote.<name>.pushurl` is defined). + Setting this key to the empty string clears the list of urls, + allowing you to override earlier config. remote.<name>.pushurl:: The push URL of a remote repository. See linkgit:git-push[1]. If a `pushurl` option is present in a configured remote, it is used for pushing instead of `remote.<name>.url`. A configured remote can have multiple push URLs; in this case a push goes to - all of them. + all of them. Setting this key to the empty string clears the + list of urls, allowing you to override earlier config. remote.<name>.proxy:: For remotes that require curl (http, https and ftp), the URL to diff --git a/remote.c b/remote.c index 9417d83e51..b7262964fb 100644 --- a/remote.c +++ b/remote.c @@ -63,12 +63,18 @@ static char *alias_url(const char *url, struct rewrites *r) static void add_url(struct remote *remote, const char *url) { - strvec_push(&remote->url, url); + if (*url) + strvec_push(&remote->url, url); + else + strvec_clear(&remote->url); } static void add_pushurl(struct remote *remote, const char *pushurl) { - strvec_push(&remote->pushurl, pushurl); + if (*pushurl) + strvec_push(&remote->pushurl, pushurl); + else + strvec_clear(&remote->pushurl); } static void add_pushurl_alias(struct remote_state *remote_state, diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 7789ff12c4..08424e878e 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -1492,4 +1492,40 @@ test_expect_success 'refs/remotes/* <src> refspec and unqualified <dst> DWIM and ) ' +test_expect_success 'empty config clears remote.*.url list' ' + test_when_finished "git config --remove-section remote.multi" && + git config --add remote.multi.url wrong-one && + git config --add remote.multi.url wrong-two && + git -c remote.multi.url= \ + -c remote.multi.url=right-one \ + -c remote.multi.url=right-two \ + remote show -n multi >actual.raw && + grep URL actual.raw >actual && + cat >expect <<-\EOF && + Fetch URL: right-one + Push URL: right-one + Push URL: right-two + EOF + test_cmp expect actual +' + +test_expect_success 'empty config clears remote.*.pushurl list' ' + test_when_finished "git config --remove-section remote.multi" && + git config --add remote.multi.url right && + git config --add remote.multi.url will-be-ignored && + git config --add remote.multi.pushurl wrong-push-one && + git config --add remote.multi.pushurl wrong-push-two && + git -c remote.multi.pushurl= \ + -c remote.multi.pushurl=right-push-one \ + -c remote.multi.pushurl=right-push-two \ + remote show -n multi >actual.raw && + grep URL actual.raw >actual && + cat >expect <<-\EOF && + Fetch URL: right + Push URL: right-push-one + Push URL: right-push-two + EOF + test_cmp expect actual +' + test_done
Because remote.*.url is treated as a multi-valued key, there is no way to override previous config. So for example if you have remote.origin.url set to some wrong value, doing: git -c remote.origin.url=right fetch would not work. It would append "right" to the list, which means we'd still fetch from "wrong" (since subsequent values are used only as push urls). Let's provide a mechanism to reset the list, like we do for other multi-valued keys (e.g., credential.helper, http.extraheaders, and merge.suppressDest all use this "empty string means reset" pattern). Reported-by: Mathew George <mathewegeorge@gmail.com> Signed-off-by: Jeff King <peff@peff.net> --- By the way, I think the nearby remote.*.fetch and remote.*.push could learn the same trick. I left that out of this series, mostly because it was getting long. But also because I had trouble imagining how a one-off refspec change would be useful. We can revisit it on top if we want. Documentation/config/remote.txt | 5 ++++- remote.c | 10 +++++++-- t/t5505-remote.sh | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-)