From acede2ebc939f312058ab7aa15e9ede71029ee85 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:20:55 +0100 Subject: [PATCH 1/7] t5700: document a failure of alternates to affect fetch If an alternate supplies some, but not all, of the objects needed for a fetch, fetch-pack nevertheless generates "want" lines for the alternate objects that are present. Demonstrate this problem via a failing test. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t5700-clone-reference.sh | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh index c4c375ac04..bd7a1d7dbb 100755 --- a/t/t5700-clone-reference.sh +++ b/t/t5700-clone-reference.sh @@ -52,13 +52,13 @@ test_cmp expected current' cd "$base_dir" -rm -f "$U" +rm -f "$U.D" test_expect_success 'cloning with reference (no -l -s)' \ -'GIT_DEBUG_SEND_PACK=3 git clone --reference B "file://$(pwd)/A" D 3>"$U"' +'GIT_DEBUG_SEND_PACK=3 git clone --reference B "file://$(pwd)/A" D 3>"$U.D"' test_expect_success 'fetched no objects' \ -'! grep "^want" "$U"' +'! grep "^want" "$U.D"' cd "$base_dir" @@ -153,4 +153,32 @@ test_expect_success 'clone with reference from a tagged repository' ' git clone --reference=A A I ' +test_expect_success 'prepare branched repository' ' + git clone A J && + ( + cd J && + git checkout -b other master^ && + echo other >otherfile && + git add otherfile && + git commit -m other && + git checkout master + ) +' + +rm -f "$U.K" + +test_expect_failure 'fetch with incomplete alternates' ' + git init K && + echo "$base_dir/A/.git/objects" >K/.git/objects/info/alternates && + ( + cd K && + git remote add J "file://$base_dir/J" && + GIT_DEBUG_SEND_PACK=3 git fetch J 3>"$U.K" + ) && + master_object=$(cd A && git for-each-ref --format="%(objectname)" refs/heads/master) && + ! grep "^want $master_object" "$U.K" && + tag_object=$(cd A && git for-each-ref --format="%(objectname)" refs/tags/HEAD) && + ! grep "^want $tag_object" "$U.K" +' + test_done From 5b05795c4cdf39348c18e40c12e70bd00a7192e5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:20:56 +0100 Subject: [PATCH 2/7] clone.c: move more code into the "if (refs)" conditional The bahavior of a bunch of code before the "if (refs)" statement also depends on whether refs is set, so make the logic clearer by shifting this code into the if statement. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/clone.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index c62d4b5737..279fdf0d25 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -813,28 +813,28 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } refs = transport_get_remote_refs(transport); - mapped_refs = refs ? wanted_peer_refs(refs, refspec) : NULL; - - /* - * transport_get_remote_refs() may return refs with null sha-1 - * in mapped_refs (see struct transport->get_refs_list - * comment). In that case we need fetch it early because - * remote_head code below relies on it. - * - * for normal clones, transport_get_remote_refs() should - * return reliable ref set, we can delay cloning until after - * remote HEAD check. - */ - for (ref = refs; ref; ref = ref->next) - if (is_null_sha1(ref->old_sha1)) { - complete_refs_before_fetch = 0; - break; - } - - if (!is_local && !complete_refs_before_fetch && refs) - transport_fetch_refs(transport, mapped_refs); if (refs) { + mapped_refs = wanted_peer_refs(refs, refspec); + /* + * transport_get_remote_refs() may return refs with null sha-1 + * in mapped_refs (see struct transport->get_refs_list + * comment). In that case we need fetch it early because + * remote_head code below relies on it. + * + * for normal clones, transport_get_remote_refs() should + * return reliable ref set, we can delay cloning until after + * remote HEAD check. + */ + for (ref = refs; ref; ref = ref->next) + if (is_null_sha1(ref->old_sha1)) { + complete_refs_before_fetch = 0; + break; + } + + if (!is_local && !complete_refs_before_fetch) + transport_fetch_refs(transport, mapped_refs); + remote_head = find_ref_by_name(refs, "HEAD"); remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0); @@ -852,6 +852,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } else { warning(_("You appear to have cloned an empty repository.")); + mapped_refs = NULL; our_head_points_at = NULL; remote_head_points_at = NULL; remote_head = NULL; From 65385ef7d457d740eddae2c4179064b8e13ac777 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:20:57 +0100 Subject: [PATCH 3/7] fetch-pack.c: rename some parameters from "path" to "refname" The parameters denote reference names, which are no longer 1:1 with filesystem paths. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 6207ecd298..9bd2096229 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -58,9 +58,9 @@ static void rev_list_push(struct commit *commit, int mark) } } -static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data) +static int rev_list_insert_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { - struct object *o = deref_tag(parse_object(sha1), path, 0); + struct object *o = deref_tag(parse_object(sha1), refname, 0); if (o && o->type == OBJ_COMMIT) rev_list_push((struct commit *)o, SEEN); @@ -68,9 +68,9 @@ static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int return 0; } -static int clear_marks(const char *path, const unsigned char *sha1, int flag, void *cb_data) +static int clear_marks(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { - struct object *o = deref_tag(parse_object(sha1), path, 0); + struct object *o = deref_tag(parse_object(sha1), refname, 0); if (o && o->type == OBJ_COMMIT) clear_commit_marks((struct commit *)o, @@ -493,7 +493,7 @@ done: static struct commit_list *complete; -static int mark_complete(const char *path, const unsigned char *sha1, int flag, void *cb_data) +static int mark_complete(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { struct object *o = parse_object(sha1); From c41a802fe9593db67f458bde9f15f51fdf3f4f1a Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:20:58 +0100 Subject: [PATCH 4/7] fetch-pack.c: inline insert_alternate_refs() The logic of the (single) caller is clearer without encapsulating this one line in a function. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 9bd2096229..dbe9acbc39 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -256,11 +256,6 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused) rev_list_insert_ref(NULL, ref->old_sha1, 0, NULL); } -static void insert_alternate_refs(void) -{ - for_each_alternate_ref(insert_one_alternate_ref, NULL); -} - #define INITIAL_FLUSH 16 #define PIPESAFE_FLUSH 32 #define LARGE_FLUSH 1024 @@ -295,7 +290,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, marked = 1; for_each_ref(rev_list_insert_ref, NULL); - insert_alternate_refs(); + for_each_alternate_ref(insert_one_alternate_ref, NULL); fetching = 0; for ( ; refs ; refs = refs->next) { From f2576591320f6296d341fa14bd50ba6918ced864 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:20:59 +0100 Subject: [PATCH 5/7] everything_local(): mark alternate refs as complete Objects in an alternate object database are already available to the local repository and therefore don't need to be fetched. So mark them as complete in everything_local(). This fixes a test in t5700. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 6 ++++++ t/t5700-clone-reference.sh | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index dbe9acbc39..0e8560f60f 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -581,6 +581,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match) *refs = newlist; } +static void mark_alternate_complete(const struct ref *ref, void *unused) +{ + mark_complete(NULL, ref->old_sha1, 0, NULL); +} + static int everything_local(struct ref **refs, int nr_match, char **match) { struct ref *ref; @@ -609,6 +614,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match) if (!args.depth) { for_each_ref(mark_complete, NULL); + for_each_alternate_ref(mark_alternate_complete, NULL); if (cutoff) mark_recent_complete_commits(cutoff); } diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh index bd7a1d7dbb..bbc4691bd7 100755 --- a/t/t5700-clone-reference.sh +++ b/t/t5700-clone-reference.sh @@ -167,7 +167,7 @@ test_expect_success 'prepare branched repository' ' rm -f "$U.K" -test_expect_failure 'fetch with incomplete alternates' ' +test_expect_success 'fetch with incomplete alternates' ' git init K && echo "$base_dir/A/.git/objects" >K/.git/objects/info/alternates && ( From a1287f7540d2a27fcde26b356dc176c5dbb4f4e2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:21:00 +0100 Subject: [PATCH 6/7] clone: do not add alternate references to extra_refs Alternate references are directly (and now, correctly) handled by fetch-pack, so there is no need to inform fetch-pack about them via the extra_refs back channel. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/clone.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 279fdf0d25..b15fccb588 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -232,9 +232,6 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) { char *ref_git; struct strbuf alternate = STRBUF_INIT; - struct remote *remote; - struct transport *transport; - const struct ref *extra; /* Beware: real_path() and mkpath() return static buffer */ ref_git = xstrdup(real_path(item->string)); @@ -249,14 +246,6 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) strbuf_addf(&alternate, "%s/objects", ref_git); add_to_alternates_file(alternate.buf); strbuf_release(&alternate); - - remote = remote_get(ref_git); - transport = transport_get(remote, ref_git); - for (extra = transport_get_remote_refs(transport); extra; - extra = extra->next) - add_extra_ref(extra->name, extra->old_sha1, 0); - - transport_disconnect(transport); free(ref_git); return 0; } @@ -500,7 +489,6 @@ static void update_remote_refs(const struct ref *refs, const char *msg) { if (refs) { - clear_extra_refs(); write_remote_refs(mapped_refs); if (option_single_branch) write_followtags(refs, msg); From cf6672edb1eb90ef7f79d37eca08c93a662802a8 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 11 Feb 2012 07:21:01 +0100 Subject: [PATCH 7/7] refs: remove the extra_refs API The extra_refs provided a kludgy way to create fake references at a global level in the hope that they would only affect some particular code path. The last user of this API been rewritten, so strip this stuff out before somebody else gets the bad idea of using it. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 23 +---------------------- refs.h | 8 -------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/refs.c b/refs.c index b8843bb476..c9f6835351 100644 --- a/refs.c +++ b/refs.c @@ -183,12 +183,6 @@ static struct ref_cache { static struct ref_entry *current_ref; -/* - * Never call sort_ref_array() on the extra_refs, because it is - * allowed to contain entries with duplicate names. - */ -static struct ref_array extra_refs; - static void clear_ref_array(struct ref_array *array) { int i; @@ -289,16 +283,6 @@ static void read_packed_refs(FILE *f, struct ref_array *array) } } -void add_extra_ref(const char *refname, const unsigned char *sha1, int flag) -{ - add_ref(&extra_refs, create_ref_entry(refname, sha1, flag, 0)); -} - -void clear_extra_refs(void) -{ - clear_ref_array(&extra_refs); -} - static struct ref_array *get_packed_refs(struct ref_cache *refs) { if (!refs->did_packed) { @@ -733,16 +717,11 @@ fallback: static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn, int trim, int flags, void *cb_data) { - int retval = 0, i, p = 0, l = 0; + int retval = 0, p = 0, l = 0; struct ref_cache *refs = get_ref_cache(submodule); struct ref_array *packed = get_packed_refs(refs); struct ref_array *loose = get_loose_refs(refs); - struct ref_array *extra = &extra_refs; - - for (i = 0; i < extra->nr; i++) - retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]); - sort_ref_array(packed); sort_ref_array(loose); while (p < packed->nr && l < loose->nr) { diff --git a/refs.h b/refs.h index 00ba1e2813..33202b0d4c 100644 --- a/refs.h +++ b/refs.h @@ -56,14 +56,6 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn */ extern void add_packed_ref(const char *refname, const unsigned char *sha1); -/* - * Extra refs will be listed by for_each_ref() before any actual refs - * for the duration of this process or until clear_extra_refs() is - * called. Only extra refs added before for_each_ref() is called will - * be listed on a given call of for_each_ref(). - */ -extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags); -extern void clear_extra_refs(void); extern int ref_exists(const char *); extern int peel_ref(const char *refname, unsigned char *sha1);