* [PATCH 3/5] Correct handling of branch.$name.merge in builtin-fetch
@ 2007-09-18 8:54 Shawn O. Pearce
2007-09-18 15:05 ` Daniel Barkalow
0 siblings, 1 reply; 2+ messages in thread
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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>
---
builtin-fetch.c | 48 ++++++++++++++-----
remote.c | 13 ++---
remote.h | 3 +-
t/t5515/fetch.br-branches-default-merge | 1 +
...etch.br-branches-default-merge_branches-default | 1 +
t/t5515/fetch.br-branches-default-octopus | 2 +
...ch.br-branches-default-octopus_branches-default | 2 +
t/t5515/fetch.br-branches-one-merge | 1 +
t/t5515/fetch.br-branches-one-merge_branches-one | 1 +
t/t5515/fetch.br-branches-one-octopus | 1 +
t/t5515/fetch.br-branches-one-octopus_branches-one | 1 +
11 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 20926e0..670af0b 100644
--- 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 af3c46b..31e2b70 100644
--- 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 8994052..b5b558f 100644
--- 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 828bfd8..ca2cc1d 100644
--- 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 f148673..7d947cd 100644
--- 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 bb1a191..ec39c54 100644
--- 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 970fc93..6bf42e2 100644
--- 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 24099fd..b4b3b35 100644
--- 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 e4b4fde..2ecef38 100644
--- 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 53fe808..96e3029 100644
--- 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 41b18ff..55e0bad 100644
--- 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 ../
--
1.5.3.1.1000.g7319b
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 3/5] Correct handling of branch.$name.merge in builtin-fetch
2007-09-18 8:54 [PATCH 3/5] Correct handling of branch.$name.merge in builtin-fetch Shawn O. Pearce
@ 2007-09-18 15:05 ` Daniel Barkalow
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Barkalow @ 2007-09-18 15:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
On Tue, 18 Sep 2007, Shawn O. Pearce wrote:
> 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.
Ah, right. When you removed that, I remembered there being some reason I'd
put it in, but I couldn't remember what it was, and knew you'd turn it up
before I would.
> 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".
And store master, but don't store pu. This looks like the right solution
to me.
> 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.
Ah, okay. This is one of the things I'd changed, to make it obey the merge
configuration (which meant that it didn't get anything to merge), which
had previously been ignored. So there's nothing that's expecting exactly
the behavior that you're changing away from, except for that test case.
This whole series looks good to me.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-09-18 15:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-18 8:54 [PATCH 3/5] Correct handling of branch.$name.merge in builtin-fetch Shawn O. Pearce
2007-09-18 15:05 ` Daniel Barkalow
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).