{"thread":{"id":"59124","subject":"[PATCH] fetch: fix duplicate remote parallel fetch bug","startedAt":"2023-01-19T22:22:42Z","lastAt":"2023-01-19T23:44:50Z","messageCount":3,"participants":["Calvin Wan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"470739","messageId":"20230119220538.1522464-1-calvinwan@google.com","threadId":"59124","inReplyTo":null,"subject":"[PATCH] fetch: fix duplicate remote parallel fetch bug","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2023-01-19T22:05:38Z","receivedAt":"2023-01-19T22:22:42Z","isPatch":true,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"Fetching in parallel from a remote group with a duplicated remote results\nin the following:\n\nerror: cannot lock ref '<ref>': is at <oid> but expected <oid>\n\nThis doesn't happen in serial since fetching from the same remote that\nhas already been fetched from is a noop. Therefore, remove any duplicated\nremotes after remote groups are parsed.\n\nSigned-off-by: Calvin Wan <calvinwan@google.com>\n---\n builtin/fetch.c          | 1 +\n t/t5506-remote-groups.sh | 9 +++++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex b06e454cbd..508ab2670c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -2225,6 +2225,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t\targv++;\n \t\t}\n \t}\n+\tstring_list_remove_duplicates(&list, 0);\n \n \tif (negotiate_only) {\n \t\tstruct oidset acked_commits = OIDSET_INIT;\ndiff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh\nindex 5bac03ede8..0e176175a3 100755\n--- a/t/t5506-remote-groups.sh\n+++ b/t/t5506-remote-groups.sh\n@@ -99,4 +99,13 @@ test_expect_success 'updating remote name updates that remote' '\n \t! repo_fetched two\n '\n \n+test_expect_success 'updating group in parallel with a duplicate remote does not fail (fetch)' '\n+\tmark fetch-group-duplicate &&\n+\tupdate_repo one &&\n+\tgit config --add remotes.duplicate one &&\n+\tgit config --add remotes.duplicate one &&\n+\tgit -c fetch.parallel=2 remote update duplicate &&\n+\trepo_fetched one\n+'\n+\n test_done\n-- \n2.39.0.246.g2a6d74b583-goog\n\n"},{"id":"470742","messageId":"xmqq8rhy172q.fsf@gitster.g","threadId":"59124","inReplyTo":"20230119220538.1522464-1-calvinwan@google.com","subject":"Re: [PATCH] fetch: fix duplicate remote parallel fetch bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-19T22:40:45Z","receivedAt":"2023-01-19T22:57:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Calvin Wan <calvinwan@google.com> writes:\n\n> Fetching in parallel from a remote group with a duplicated remote results\n> in the following:\n>\n> error: cannot lock ref '<ref>': is at <oid> but expected <oid>\n>\n> This doesn't happen in serial since fetching from the same remote that\n> has already been fetched from is a noop. Therefore, remove any duplicated\n> remotes after remote groups are parsed.\n>\n> Signed-off-by: Calvin Wan <calvinwan@google.com>\n> ---\n>  builtin/fetch.c          | 1 +\n>  t/t5506-remote-groups.sh | 9 +++++++++\n>  2 files changed, 10 insertions(+)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index b06e454cbd..508ab2670c 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -2225,6 +2225,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \t\t\targv++;\n>  \t\t}\n>  \t}\n> +\tstring_list_remove_duplicates(&list, 0);\n\nAs it always is possible to edit .git/config manually, it is\nnecessary to perform deduplication like this patch does on the\nconsumer side of the list, but do you know if our tool create\nduplication, or is it entirely something the end-user does manually?\n\nIf it is the former, I am wondering if we should also fix such a\ncode path that does so in the first place.\n\nWill queue.  Thanks.\n"},{"id":"470754","messageId":"CAFySSZA6MgjejnoBa7HrXAQBNb1zE93aASVZogabAK_ArDNmnA@mail.gmail.com","threadId":"59124","inReplyTo":"xmqq8rhy172q.fsf@gitster.g","subject":"Re: [PATCH] fetch: fix duplicate remote parallel fetch bug","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2023-01-19T23:44:32Z","receivedAt":"2023-01-19T23:44:50Z","isPatch":true,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"> As it always is possible to edit .git/config manually, it is\n> necessary to perform deduplication like this patch does on the\n> consumer side of the list, but do you know if our tool create\n> duplication, or is it entirely something the end-user does manually?\n\nI checked git-remote and there is protection against duplication\nthere, but I'm unsure if there are other places where remotes are\nbeing added/renamed. I discovered the bug initially by using\ngit-config.\n"}]}