From 080bc4990f56f9d0687919e5136cb138509f1269 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 8 Sep 2022 15:24:21 -0400 Subject: [PATCH 1/2] fetch: stop checking for NULL transport->remote in do_fetch() This field will never be NULL; if it were, we'd segfault earlier in the function when we unconditionally check transport->remote->fetch_tags. Likewise, many other functions dereference it unconditionally. This is a small simplification, but it will make things easier as we extend this conditional in the next patch. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ac29c2b1ae..d39098d9ab 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1626,7 +1626,7 @@ static int do_fetch(struct transport *transport, break; } } - } else if (transport->remote && transport->remote->fetch.nr) + } else if (transport->remote->fetch.nr) refspec_ref_prefixes(&transport->remote->fetch, &transport_ls_refs_options.ref_prefixes); From 49ca2fba393fa277ab70253337c53c7831597c3a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 8 Sep 2022 15:26:09 -0400 Subject: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension When running "git pull" with no arguments, we'll do a default "git fetch" and then try to merge the branch specified by the branch.*.merge config. There's code in get_ref_map() to treat that "merge" branch as something we want to fetch, even if it is not otherwise covered by the default refspec. This works fine with the v0 protocol, as the server tells us about all of the refs, and get_ref_map() is the ultimate decider of what we fetch. But in the v2 protocol, we send the ref-prefix extension to the server, asking it to limit the ref advertisement. And we only tell it about the default refspec for the remote; we don't mention the branch.*.merge config at all. This usually doesn't matter, because the default refspec matches "refs/heads/*", which covers all branches. But if you explicitly use a narrow refspec, then "git pull" on some branches may fail. The server doesn't advertise the branch, so we don't fetch it, and "git pull" thinks that it went away upstream. We can fix this by including any branch.*.merge entries for the current branch in the list of ref-prefixes we pass to the server. This only needs to happen when using the default configured refspec (since command-line refspecs are already added, and take precedence in deciding what we fetch). We don't otherwise need to replicate any of the "what to fetch" logic in get_ref_map(). These ref-prefixes are an optimization, so it's OK if we tell the server to advertise the branch.*.merge ref, even if we're not going to pull it. We'll just choose not to fetch it. The test here is based on one constructed by Johannes. I modified the branch names to trigger the ref-prefix issue (and be more descriptive), and to confirm that "git pull" actually updated the local ref, which should be more robust than just checking stderr. Reported-by: Lana Deere Helped-by: Johannes Schindelin Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch.c | 18 +++++++++++++++--- t/t5520-pull.sh | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index d39098d9ab..acc4e89bc9 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1626,9 +1626,21 @@ static int do_fetch(struct transport *transport, break; } } - } else if (transport->remote->fetch.nr) - refspec_ref_prefixes(&transport->remote->fetch, - &transport_ls_refs_options.ref_prefixes); + } else { + struct branch *branch = branch_get(NULL); + + if (transport->remote->fetch.nr) + refspec_ref_prefixes(&transport->remote->fetch, + &transport_ls_refs_options.ref_prefixes); + if (branch_has_merge_config(branch) && + !strcmp(branch->remote_name, transport->remote->name)) { + int i; + for (i = 0; i < branch->merge_nr; i++) { + strvec_push(&transport_ls_refs_options.ref_prefixes, + branch->merge[i]->src); + } + } + } if (tags == TAGS_SET || tags == TAGS_DEFAULT) { must_list_refs = 1; diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 081808009b..0b72112fb1 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -218,6 +218,23 @@ test_expect_success 'fail if upstream branch does not exist' ' test_cmp expect file ' +test_expect_success 'fetch upstream branch even if refspec excludes it' ' + # the branch names are not important here except that + # the first one must not be a prefix of the second, + # since otherwise the ref-prefix protocol extension + # would match both + git branch in-refspec HEAD^ && + git branch not-in-refspec HEAD && + git init -b in-refspec downstream && + git -C downstream remote add -t in-refspec origin "file://$(pwd)/.git" && + git -C downstream config branch.in-refspec.remote origin && + git -C downstream config branch.in-refspec.merge refs/heads/not-in-refspec && + git -C downstream pull && + git rev-parse --verify not-in-refspec >expect && + git -C downstream rev-parse --verify HEAD >actual && + test_cmp expect actual +' + test_expect_success 'fail if the index has unresolved entries' ' git checkout -b third second^ && test_when_finished "git checkout -f copy && git branch -D third" &&