Correct handling of branch.$name.merge in builtin-fetch

My prior bug fix for git-push titled "Don't configure remote "." to
fetch everything to itself" actually broke t5520 as we were unable
to evaluate a branch configuration of:

  [branch "copy"]
    remote = .
    merge = refs/heads/master

as remote "." did not have a "remote...fetch" configuration entry to
offer up refs/heads/master as a possible candidate available to be
fetched and merged.  In shell script git-fetch and prior to the above
mentioned commit this was hardcoded for a url of "." to be the set of
local branches.

Chasing down this bug led me to the conclusion that our prior behavior
with regards to branch.$name.merge was incorrect.  In the shell script
based git-fetch implementation we only fetched and merged a branch if
it appeared both in branch.$name.merge *and* in remote.$r.fetch, where
$r = branch.$name.remote.  In other words in the following config file:

  [remote "origin"]
    url = git://git.kernel.org/pub/scm/git/git.git
    fetch = refs/heads/master:refs/remotes/origin/master
  [branch "master"]
    remote = origin
    merge = refs/heads/master
  [branch "pu"]
    remote = origin
    merge = refs/heads/pu

Attempting to run `git pull` while on branch "pu" would always give
the user "Already up-to-date" as git-fetch did not fetch pu and thus
did not mark it for merge in .git/FETCH_HEAD.  The configured merge
would always be ignored and the user would be left scratching her
confused head wondering why merge did not work on "pu" but worked
fine on "master".

If we are using the "default fetch" specification for the current
branch and the current branch has a branch.$name.merge configured
we now union it with the list of refs in remote.$r.fetch.  This
way the above configuration does what the user expects it to do,
which is to fetch only "master" by default but when on "pu" to
fetch both "master" and "pu".

This uncovered some breakage in the test suite where old-style Cogito
branches (.git/branches/$r) did not fetch the branches listed in
.git/config for merging and thus did not actually merge them if the
user tried to use `git pull` on that branch.  Junio and I discussed
it on list and felt that the union approach here makes more sense to
DWIM for the end-user than silently ignoring their configured request
so the test vectors for t5515 have been updated to include for-merge
lines in .git/FETCH_HEAD where they have been configured for-merge
in .git/config.

Since we are now performing a union of the fetch specification and
the merge specification and we cannot allow a branch to be listed
twice (otherwise it comes out twice in .git/FETCH_HEAD) we need to
perform a double loop here over all of the branch.$name.merge lines
and try to set their merge flag if we have already schedule that
branch for fetching by remote.$r.fetch.  If no match is found then
we must add new specifications to fetch the branch but not store it
as no local tracking branch has been designated.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 20926e05439d7fb5c2a79374225a89d8aa7fa79a..670af0b53f72e370368fddc1e911504a01979f6a 100644 (file)
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -28,20 +28,37 @@ static void unlock_pack_on_signal(int signo)
        raise(signo);
 }
 
-static void find_merge_config(struct ref *ref_map, struct remote *remote)
+static void add_merge_config(struct ref **head,
+                          struct ref *remote_refs,
+                          struct branch *branch,
+                          struct ref ***tail)
 {
-       struct ref *rm = ref_map;
-       struct branch *branch = branch_get(NULL);
+       int i;
 
-       for (rm = ref_map; rm; rm = rm->next) {
-               if (!branch_has_merge_config(branch)) {
-                       if (remote && remote->fetch &&
-                           !strcmp(remote->fetch[0].src, rm->name))
-                               rm->merge = 1;
-               } else {
-                       if (branch_merges(branch, rm->name))
+       for (i = 0; i < branch->merge_nr; i++) {
+               struct ref *rm, **old_tail = *tail;
+               struct refspec refspec;
+
+               for (rm = *head; rm; rm = rm->next) {
+                       if (branch_merge_matches(branch, i, rm->name)) {
                                rm->merge = 1;
+                               break;
+                       }
                }
+               if (rm)
+                       continue;
+
+               /* Not fetched to a tracking branch?  We need to fetch
+                * it anyway to allow this branch's "branch.$name.merge"
+                * to be honored by git-pull.
+                */
+               refspec.src = branch->merge[i]->src;
+               refspec.dst = NULL;
+               refspec.pattern = 0;
+               refspec.force = 0;
+               get_fetch_map(remote_refs, &refspec, tail);
+               for (rm = *old_tail; rm; rm = rm->next)
+                       rm->merge = 1;
        }
 }
 
@@ -76,17 +93,22 @@ static struct ref *get_ref_map(struct transport *transport,
        } else {
                /* Use the defaults */
                struct remote *remote = transport->remote;
-               if (remote->fetch_refspec_nr) {
+               struct branch *branch = branch_get(NULL);
+               int has_merge = branch_has_merge_config(branch);
+               if (remote && (remote->fetch_refspec_nr || has_merge)) {
                        for (i = 0; i < remote->fetch_refspec_nr; i++) {
                                get_fetch_map(remote_refs, &remote->fetch[i], &tail);
                                if (remote->fetch[i].dst &&
                                    remote->fetch[i].dst[0])
                                        *autotags = 1;
+                               if (!i && !has_merge && ref_map &&
+                                   !strcmp(remote->fetch[0].src, ref_map->name))
+                                       ref_map->merge = 1;
                        }
-                       find_merge_config(ref_map, remote);
+                       if (has_merge)
+                               add_merge_config(&ref_map, remote_refs, branch, &tail);
                } else {
                        ref_map = get_remote_ref(remote_refs, "HEAD");
-
                        ref_map->merge = 1;
                }
        }

diff --git a/remote.c b/remote.c
index af3c46bb35cb77db184447afd14e1cd434e7a9f5..31e2b70d37d9c1b5b781c7276a65441445ac8c15 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -772,16 +772,13 @@ int branch_has_merge_config(struct branch *branch)
        return branch && !!branch->merge;
 }
 
-int branch_merges(struct branch *branch, const char *refname)
+int branch_merge_matches(struct branch *branch,
+                                int i,
+                                const char *refname)
 {
-       int i;
-       if (!branch)
+       if (!branch || i < 0 || i >= branch->merge_nr)
                return 0;
-       for (i = 0; i < branch->merge_nr; i++) {
-               if (ref_matches_abbrev(branch->merge[i]->src, refname))
-                       return 1;
-       }
-       return 0;
+       return ref_matches_abbrev(branch->merge[i]->src, refname);
 }
 
 static struct ref *get_expanded_map(struct ref *remote_refs,

diff --git a/remote.h b/remote.h
index 89940527ad989ca7ab20794cfe7dba910cb36e34..b5b558f1f14eee93994eec8a9b26778219b80503 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -88,7 +88,6 @@ struct branch {
 struct branch *branch_get(const char *name);
 
 int branch_has_merge_config(struct branch *branch);
-
-int branch_merges(struct branch *branch, const char *refname);
+int branch_merge_matches(struct branch *, int n, const char *);
 
 #endif

diff --git a/t/t5515/fetch.br-branches-default-merge b/t/t5515/fetch.br-branches-default-merge
index 828bfd8e9e0dedd9dc06c44c13bb676f33e28e2b..ca2cc1d1b44e3edc8cd42e2e77d0f85658a52195 100644 (file)
--- a/t/t5515/fetch.br-branches-default-merge
+++ b/t/t5515/fetch.br-branches-default-merge
@@ -1,5 +1,6 @@
 # br-branches-default-merge
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   branch 'master' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b               branch 'three' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-default-merge_branches-default b/t/t5515/fetch.br-branches-default-merge_branches-default
index f1486730fa1bbd401dca2cb1142b9746b3bb3d61..7d947cd80f9cf656024206f1ea31da0d9f10f493 100644 (file)
--- a/t/t5515/fetch.br-branches-default-merge_branches-default
+++ b/t/t5515/fetch.br-branches-default-merge_branches-default
@@ -1,5 +1,6 @@
 # br-branches-default-merge branches-default
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   branch 'master' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b               branch 'three' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-default-octopus b/t/t5515/fetch.br-branches-default-octopus
index bb1a1915c68c80af3719a7d792ef660fe57bdec2..ec39c54b7e242ddbeec76f55b98f555d562aa271 100644 (file)
--- a/t/t5515/fetch.br-branches-default-octopus
+++ b/t/t5515/fetch.br-branches-default-octopus
@@ -1,5 +1,7 @@
 # br-branches-default-octopus
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   branch 'master' of ../
+8e32a6d901327a23ef831511badce7bf3bf46689               branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8               branch 'two' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-default-octopus_branches-default b/t/t5515/fetch.br-branches-default-octopus_branches-default
index 970fc937256ce2f35ac5f04069a540d72353ad85..6bf42e24b67b526bac49e3cdb287e32513f4a6c4 100644 (file)
--- a/t/t5515/fetch.br-branches-default-octopus_branches-default
+++ b/t/t5515/fetch.br-branches-default-octopus_branches-default
@@ -1,5 +1,7 @@
 # br-branches-default-octopus branches-default
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   branch 'master' of ../
+8e32a6d901327a23ef831511badce7bf3bf46689               branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8               branch 'two' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-one-merge b/t/t5515/fetch.br-branches-one-merge
index 24099fd53e23df58e45879fda727d488585cd38d..b4b3b35ce0e2f46a16b015a74b771eb90ed3ebad 100644 (file)
--- a/t/t5515/fetch.br-branches-one-merge
+++ b/t/t5515/fetch.br-branches-one-merge
@@ -1,5 +1,6 @@
 # br-branches-one-merge
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   branch 'one' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b               branch 'three' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-one-merge_branches-one b/t/t5515/fetch.br-branches-one-merge_branches-one
index e4b4fdee4c2e5886185eef7de4bae81449edbb13..2ecef384eb7d823104581bfe2b4bd240b449e5df 100644 (file)
--- a/t/t5515/fetch.br-branches-one-merge_branches-one
+++ b/t/t5515/fetch.br-branches-one-merge_branches-one
@@ -1,5 +1,6 @@
 # br-branches-one-merge branches-one
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   branch 'one' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b               branch 'three' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-one-octopus b/t/t5515/fetch.br-branches-one-octopus
index 53fe808a3b73cefe9af1407e459ccde22b78cad9..96e3029416b46ab4192d3e4aaa285a02489e4054 100644 (file)
--- a/t/t5515/fetch.br-branches-one-octopus
+++ b/t/t5515/fetch.br-branches-one-octopus
@@ -1,5 +1,6 @@
 # br-branches-one-octopus
 8e32a6d901327a23ef831511badce7bf3bf46689               branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8               branch 'two' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../

diff --git a/t/t5515/fetch.br-branches-one-octopus_branches-one b/t/t5515/fetch.br-branches-one-octopus_branches-one
index 41b18ff78a4e841efd688240f1f5060f42aea2d9..55e0bad621cde0c93e6a6fb92dc259c61986aba5 100644 (file)
--- a/t/t5515/fetch.br-branches-one-octopus_branches-one
+++ b/t/t5515/fetch.br-branches-one-octopus_branches-one
@@ -1,5 +1,6 @@
 # br-branches-one-octopus branches-one
 8e32a6d901327a23ef831511badce7bf3bf46689               branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8               branch 'two' of ../
 754b754407bf032e9a2f9d5a9ad05ca79a6b228f       not-for-merge   tag 'tag-master' of ../
 8e32a6d901327a23ef831511badce7bf3bf46689       not-for-merge   tag 'tag-one' of ../
 22feea448b023a2d864ef94b013735af34d238ba       not-for-merge   tag 'tag-one-tree' of ../