{"thread":{"id":"59205","subject":"[PATCH] Documentation: clarify multiple pushurls vs urls","startedAt":"2023-02-06T19:55:12Z","lastAt":"2023-02-06T23:01:32Z","messageCount":5,"participants":["Calvin Wan","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"471594","messageId":"20230206195503.3113048-1-calvinwan@google.com","threadId":"59205","inReplyTo":null,"subject":"[PATCH] Documentation: clarify multiple pushurls vs urls","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2023-02-06T19:55:03Z","receivedAt":"2023-02-06T19:55:12Z","isPatch":true,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"While it is possible to define multiple `url` fields in a remote to\npush to multiple remotes at once, it is preferable to achieve this by\ndefining multiple `pushurl` fields.\n\nDefining multiple `url` fields can cause confusion for users since\nrunning `git config remote..url` returns the last defined url\nwhich doesn't align with the url `git fetch ` uses (the first).\n\nAdd documentation to clarify how fetch interacts with multiple urls\nand the recommended method to push to multiple remotes.\n\nSigned-off-by: Calvin Wan \n---\n Documentation/urls-remotes.txt | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/urls-remotes.txt b/Documentation/urls-remotes.txt\nindex 86d0008f94..61aaded645 100644\n--- a/Documentation/urls-remotes.txt\n+++ b/Documentation/urls-remotes.txt\n@@ -33,7 +33,10 @@ config file would appear like this:\n ------------\n \n The `` is used for pushes only. It is optional and defaults\n-to ``.\n+to ``. Additional pushurls can be defined to push to multiple\n+remotes. While multiple URLs can be defined to achieve the same\n+outcome, this is not recommended since fetch only uses the first\n+defined URL.\n \n Named file in `$GIT_DIR/remotes`\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n-- \n2.39.1.519.gcb327c4b5f-goog\n\n"},{"id":"471595","messageId":"230206.86k00u5ygg.gmgdl@evledraar.gmail.com","threadId":"59205","inReplyTo":"20230206195503.3113048-1-calvinwan@google.com","subject":"Re: [PATCH] Documentation: clarify multiple pushurls vs urls","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-02-06T20:11:51Z","receivedAt":"2023-02-06T20:30:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Feb 06 2023, Calvin Wan wrote:\n\n> While it is possible to define multiple `url` fields in a remote to\n> push to multiple remotes at once, it is preferable to achieve this by\n> defining multiple `pushurl` fields.\n\nThe idea with \"url\" and \"pushurl\" surely is to disambiguate whether you\nwant the url for both push & fetch, or just push.\n\nI don't see why it's a given that it's preferrable to use \"pushurl\" over\n\"url\" yet, if your setup is e.g. 3 repos you push to, but it won't\nmatter what you pull from why not use \"url\"? As opposed to pushing\n\"pushurl\" to push to read-only mirrors you yourself are updating?\n\nBut let's read on...\n\n> Defining multiple `url` fields can cause confusion for users since\n> running `git config remote..url` returns the last defined url\n> which doesn't align with the url `git fetch ` uses (the first).\n\nI'm certainly confused, I had no idea it worked this way, I'd have thought it was last-set-wins like most things.\n\nFrom a glance fb0cc87ec0f (Allow programs to not depend on remotes\nhaving urls, 2009-11-18) mentions it as a known factor, but with:\n\t\n\tdiff --git a/transport.c b/transport.c\n\tindex 77a61a9d7bb..06159c4184e 100644\n\t--- a/transport.c\n\t+++ b/transport.c\n\t@@ -1115,7 +1115,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n\t \thelper = remote->foreign_vcs;\n\t \n\t \tif (!url && remote->url)\n\t-\t\turl = remote->url[0];\n\t+\t\turl = remote->url[remote->url_nr - 1];\n\t \tret->url = url;\n\t \n\t \t/* maybe it is a foreign URL? */\n\nAll tests pass for me, and it's selecting the last URL now. I can't find\nany other mention of these semantics in the docs (but maybe I didn't\nlook in the right places).\n\nSo is this just some accident, does anyone rely on it, and would we be\nbetter off just \"fixing\" this, rather than steering people away from\n\"url\"?\n\n> Add documentation to clarify how fetch interacts with multiple urls\n> and the recommended method to push to multiple remotes.\n>\n> Signed-off-by: Calvin Wan \n> ---\n> Documentation/urls-remotes.txt | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/urls-remotes.txt b/Documentation/urls-remotes.txt\n> index 86d0008f94..61aaded645 100644\n> --- a/Documentation/urls-remotes.txt\n> +++ b/Documentation/urls-remotes.txt\n> @@ -33,7 +33,10 @@ config file would appear like this:\n> ------------\n> \n> The `` is used for pushes only. It is optional and defaults\n> -to ``.\n> +to ``. Additional pushurls can be defined to push to multiple\n> +remotes. While multiple URLs can be defined to achieve the same\n> +outcome, this is not recommended since fetch only uses the first\n> +defined URL.\n\nMaybe it's just me, but I feel more confused reading this docs than\nbefore :)\n\nSurely if there's confusion about the priority of the *.url config\nvariable we should be documenting that explicitly where we discuss \"url\"\nitself (e.g. in Documentation/config/remote.txt). Just mentioning it in\npassing as we document \"pushUrl\" feels like the wrong place.\n\nBut I still don't quite see the premise. \"git push\" has a feature to\npush to all N urls, whether that's Url or pushUrl.\n\nWhen I configure it to have multiple URLs it pushes to the first\nconfigured one first, if the source of the confusion was that it didn't\nprefer the last configured one first, shouldn't it be doing them in\nreverse order?\n\nI don't think that would make sense, but I also don't see how\nrecommending \"pushurl\" over \"url\" un-confuses things.\n\nSo why is it confusing that \"fetch\" would use the same order, but due to\nthe semantics of a \"fetch\" we'd stop after the first one?\n"},{"id":"471601","messageId":"CAFySSZCO7M8bm8Cc97x7MpZYHd0qWwRHF_YRDmw1rryF6Q7dnQ@mail.gmail.com","threadId":"59205","inReplyTo":"230206.86k00u5ygg.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Documentation: clarify multiple pushurls vs urls","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2023-02-06T21:12:06Z","receivedAt":"2023-02-06T21:12:23Z","isPatch":true,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"> > Defining multiple `url` fields can cause confusion for users since\n> > running `git config remote..url` returns the last defined url\n> > which doesn't align with the url `git fetch ` uses (the first).\n>\n> I'm certainly confused, I had no idea it worked this way, I'd have thought it was last-set-wins like most things.\n>\n> From a glance fb0cc87ec0f (Allow programs to not depend on remotes\n> having urls, 2009-11-18) mentions it as a known factor, but with:\n>\n> diff --git a/transport.c b/transport.c\n> index 77a61a9d7bb..06159c4184e 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -1115,7 +1115,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n> helper = remote->foreign_vcs;\n>\n> if (!url && remote->url)\n> - url = remote->url[0];\n> + url = remote->url[remote->url_nr - 1];\n> ret->url = url;\n>\n> /* maybe it is a foreign URL? */\n>\n> All tests pass for me, and it's selecting the last URL now. I can't find\n> any other mention of these semantics in the docs (but maybe I didn't\n> look in the right places).\n>\n> So is this just some accident, does anyone rely on it, and would we be\n> better off just \"fixing\" this, rather than steering people away from\n> \"url\"?\n\nI should've mentioned running `git remote -v` on a config with multiple urls\n shows the correct fetch url, so functionally everything is working as\nintended -- just needs a doc update somewhere.\n\n> Surely if there's confusion about the priority of the *.url config\n> variable we should be documenting that explicitly where we discuss \"url\"\n> itself (e.g. in Documentation/config/remote.txt). Just mentioning it in\n> passing as we document \"pushUrl\" feels like the wrong place.\n>\n> But I still don't quite see the premise. \"git push\" has a feature to\n> push to all N urls, whether that's Url or pushUrl.\n>\n> When I configure it to have multiple URLs it pushes to the first\n> configured one first, if the source of the confusion was that it didn't\n> prefer the last configured one first, shouldn't it be doing them in\n> reverse order?\n>\n> I don't think that would make sense, but I also don't see how\n> recommending \"pushurl\" over \"url\" un-confuses things.\n>\n> So why is it confusing that \"fetch\" would use the same order, but due to\n> the semantics of a \"fetch\" we'd stop after the first one?\n\nI agree with you now that updating the documentation in\nDocumentation/config/remote.txt is the ideal way to go about this, but\nI'll mention what my original thought process was:\n\nIf a user wants one url to push/fetch to, then he defines 'url'\nIf a user wants to push to multiple urls, then he can either define\nmultiple urls or pushurls (one of the pushurls can be the same as the url).\nBut if a user has say url #2 and #3 defined, they act as pushurls anyways,\nso defining them as such removes any speculation as to what else they\ncould do (and also clears up the confusion when running\n`git config remote..url`).\n"},{"id":"471618","messageId":"230206.86pmam4exz.gmgdl@evledraar.gmail.com","threadId":"59205","inReplyTo":"CAFySSZCO7M8bm8Cc97x7MpZYHd0qWwRHF_YRDmw1rryF6Q7dnQ@mail.gmail.com","subject":"Re: [PATCH] Documentation: clarify multiple pushurls vs urls","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-02-06T21:55:11Z","receivedAt":"2023-02-06T22:17:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Feb 06 2023, Calvin Wan wrote:\n\n>> > Defining multiple `url` fields can cause confusion for users since\n>> > running `git config remote..url` returns the last defined url\n>> > which doesn't align with the url `git fetch ` uses (the first).\n>>\n>> I'm certainly confused, I had no idea it worked this way, I'd have thought it was last-set-wins like most things.\n>>\n>> From a glance fb0cc87ec0f (Allow programs to not depend on remotes\n>> having urls, 2009-11-18) mentions it as a known factor, but with:\n>>\n>> diff --git a/transport.c b/transport.c\n>> index 77a61a9d7bb..06159c4184e 100644\n>> --- a/transport.c\n>> +++ b/transport.c\n>> @@ -1115,7 +1115,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n>> helper = remote->foreign_vcs;\n>>\n>> if (!url && remote->url)\n>> - url = remote->url[0];\n>> + url = remote->url[remote->url_nr - 1];\n>> ret->url = url;\n>>\n>> /* maybe it is a foreign URL? */\n>>\n>> All tests pass for me, and it's selecting the last URL now. I can't find\n>> any other mention of these semantics in the docs (but maybe I didn't\n>> look in the right places).\n>>\n>> So is this just some accident, does anyone rely on it, and would we be\n>> better off just \"fixing\" this, rather than steering people away from\n>> \"url\"?\n>\n> I should've mentioned running `git remote -v` on a config with multiple urls\n> shows the correct fetch url, so functionally everything is working as\n> intended -- just needs a doc update somewhere.\n\nAh, yes, it seems to prefer the first configured one, whether that's\nwhat anyone intended (and should we use last configured?) is another\nmatter.\n\nBut in any case, figuring that out and having a test in-tree that fails\nif you pick the first or last of the list (depending on what we go for)\nwould be most welcome...\n\n>> Surely if there's confusion about the priority of the *.url config\n>> variable we should be documenting that explicitly where we discuss \"url\"\n>> itself (e.g. in Documentation/config/remote.txt). Just mentioning it in\n>> passing as we document \"pushUrl\" feels like the wrong place.\n>>\n>> But I still don't quite see the premise. \"git push\" has a feature to\n>> push to all N urls, whether that's Url or pushUrl.\n>>\n>> When I configure it to have multiple URLs it pushes to the first\n>> configured one first, if the source of the confusion was that it didn't\n>> prefer the last configured one first, shouldn't it be doing them in\n>> reverse order?\n>>\n>> I don't think that would make sense, but I also don't see how\n>> recommending \"pushurl\" over \"url\" un-confuses things.\n>>\n>> So why is it confusing that \"fetch\" would use the same order, but due to\n>> the semantics of a \"fetch\" we'd stop after the first one?\n>\n> I agree with you now that updating the documentation in\n> Documentation/config/remote.txt is the ideal way to go about this, but\n\nAside: I actually think near where you made the change in\nDocumentation/urls-remotes.txt is probably better, in remote.txt we just\npoint to git-fetch.txt or git-pull.txt etc., which in turn include that.\n\nIt just seems we'd need a short blurb about how URLs are selected, we\nprefer the first one, and for fetch it's always \"stop at the first\", and\nfor push \"push to all, from first to last\".\n\nI may have gotten that wrong, but that's my current understanding from\nlooking at it briefly.\n\n> I'll mention what my original thought process was:\n> If a user wants one url to push/fetch to, then he defines 'url'\n> If a user wants to push to multiple urls, then he can either define\n> multiple urls or pushurls (one of the pushurls can be the same as the url).\n> But if a user has say url #2 and #3 defined, they act as pushurls anyways,\n> so defining them as such removes any speculation as to what else they\n> could do (and also clears up the confusion when running\n> `git config remote..url`).\n\nI'm coming away from this with the impression that we should almost\nnever recommend \"pushUrl\", not that it's worthwhile to use it to solve\nthis ambiguity.\n\nTrying this out, given a config like:\n\t\n\t[remote \"avar\"]\n\t url = git@github.com:avar/git.git\n\t url = git@github.com:avar/git1.git\n\t url = git@github.com:avar/git2.git\n\nI'll get:\n\t\n\t$ ./git remote -v|grep avar\n\tavar git@github.com:avar/git.git (fetch)\n\tavar git@github.com:avar/git.git (push)\n\tavar git@github.com:avar/git1.git (push)\n\tavar git@github.com:avar/git2.git (push)\n\nSo, the semantics of \"url\" is that the first one is always the fetch,\nthere's no such thing as multiple fetch URLs, and the push URLs are the\nlist of all \"url\"'s.\n\nBut now let's change that to:\n\n\t[remote \"avar\"]\n\t url = git@github.com:avar/git.git\n\t url = git@github.com:avar/git1.git\n\t pushUrl = git@github.com:avar/git2.git\n\nWhich gives us:\n\n\t$ ./git remote -v|grep avar\n\tavar git@github.com:avar/git.git (fetch)\n\tavar git@github.com:avar/git2.git (push)\n\nSo, by defining a \"pushUrl\" all subsequent URLs in \"url\" have been\nshadowed.\n\t\nMaybe that was considered at the time, but it wouldn't surprise me if\nthat's a blindspot in the original \"pushUrl\" patch (203462347fc (Allow\npush and fetch urls to be different, 2009-06-09)), i.e. shouldn't we at\nleast warn on \"push\" now or somewhere else about the useless second\n\"url\"?\n\nSo, back to \"pushurl\". It seems to me that the only real use-case for it\nis the case where the URL really must by different in the case of\n\"fetch\" and \"push\", which I daresay is increasingly rare these days (but\nused to be more common when dumb http read + ssh write or whatever was\nmore common).\n\nWhereas the more common case is just wanting to fetch/push from/to\ngithub.com, and also wanting to mirror to gitlab.com or whathever.\n\nIn that case using \"pushUrl\" requires you to duplicate the \"fetch\" url,\nwhereas just having multiple \"url\" sections without \"pushurl\" does the\nsame thing without the duplication.\n\n"},{"id":"471634","messageId":"xmqqfsbi2ycd.fsf@gitster.g","threadId":"59205","inReplyTo":"CAFySSZCO7M8bm8Cc97x7MpZYHd0qWwRHF_YRDmw1rryF6Q7dnQ@mail.gmail.com","subject":"Re: [PATCH] Documentation: clarify multiple pushurls vs urls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-06T23:00:50Z","receivedAt":"2023-02-06T23:01:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Calvin Wan writes:\n\n> If a user wants one url to push/fetch to, then he defines 'url'\n> If a user wants to push to multiple urls, then he can either define\n\nmeaning \"the user wants to push to multiple, but never wants to\nfetch from the other ones\"? If so, then ...\n\n> multiple urls or pushurls (one of the pushurls can be the same as the url).\n\n... using multiple urls is not desirable as it becomes hard to tell\nwhich one should be used for fetching. \n\nWhen you define additional places you want to publish to, you do not\nexpect to see that new definition affect where you fetch from. In a\nsense, using the first one to fetch, not the last one, gives us less\ndamage, for that reason. Adding more URLs would not change where\nyou fetch from.\n\nBut still, I think what you gave is a good rule, regardless of the\nanswer to the \"which among multiple non-push urls do we fetch\"\nquestion. If we were designing this part of the system from\nscratch, we probably \n\n - would have forbidden multiple URLs, or\n - would allowed multiple URLs, but used the 'last one wins', or\n - would allowed multiple URLs, but and used later ones as fallback,\n only to be used when earlier ones fail.\n\nIt certainly is not a designed behaviour what we do when we have\nmultiple URLs, but if we haven't changed how we react to such a\nconfiguration in the past (and I do not think we ever did), changing\nthe behaviour in any way this late in the game likely breaks real\nusers' configuration.\n\nThanks.\n"}]}