{"thread":{"id":"29152","subject":"[PATCH 1/6] t5519: push two branches to alternate repo","startedAt":"2011-12-13T20:06:45Z","lastAt":"2011-12-13T20:06:51Z","messageCount":7,"participants":["mhagger@alum.mit.edu"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"181070","messageId":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":null,"subject":"[PATCH 0/6] Handle extra_refs separately from ref_caches","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:45Z","receivedAt":"2011-12-13T20:06:45Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nExtra refs don't have much to do with real references, and in fact\nthey have to be handled differently.  For example, they do not support\nflags, they might not have unique names (indeed, the names are rather\nmeaningless), and they are only ever iterated over, never looked up.\nSo seemingly innocent things that one might want to do with real\nreferences, like check for conflicting duplicates, must not be done\nfor extra refs.\n\nThis patch series creates a new linked-list data structure for the\nextra refs, separates iteration over the extra refs into a new\nfunction, and changes a test to actually create multiple extra refs\nwith the same name.\n\nThis patch series applies on top of master.  If this approach is\nselected, then the ref-api-D series will have to be rebased on top of\nit and touched up to avoid the problems that it has with duplicate\nextra refs.\n\nBy the way, I have been carrying around the CC list of this email for\nquite a while.  If you are tired of being spammed with my patch\nseries, send me a private email and I will be happy to remove you from\nfuture mailings.\n\nMichael Haggerty (6):\n  t5519: push two branches to alternate repo\n  add_extra_ref(): remove flag argument\n  Extract a function do_for_each_extra_ref()\n  Store extra_refs in a separate data structure\n  Omit extra_refs except when iterating using for_each_ref()\n  do_for_each_extra_ref(): simplify signature\n\n builtin/clone.c            |    4 ++--\n builtin/receive-pack.c     |    2 +-\n refs.c                     |   44 ++++++++++++++++++++++++++++++++++----------\n refs.h                     |    2 +-\n t/t5519-push-alternates.sh |   10 +++++++++-\n 5 files changed, 47 insertions(+), 15 deletions(-)\n\n-- \n1.7.8\n"},{"id":"181069","messageId":"1323806811-5798-2-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 1/6] t5519: push two branches to alternate repo","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:46Z","receivedAt":"2011-12-13T20:06:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nSince each branch in the alternate repo results in an \"extra_ref\"\nnamed \".have\", pushing two of them results in two extra_refs with the\nsame name.  This change to the test therefore makes sure that we can\nhandle extra_refs names that are not unique.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nI'm not sure how well this change fits into the other things that the\ntest wants to do, but it triggers the failure mode in ref-api-D v2\nthat was predicted by Junio.\n\n t/t5519-push-alternates.sh |   10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh\nindex c00c9b0..315f65d 100755\n--- a/t/t5519-push-alternates.sh\n+++ b/t/t5519-push-alternates.sh\n@@ -17,7 +17,15 @@ test_expect_success setup '\n \t\t>file &&\n \t\tgit add . &&\n \t\tgit commit -m initial &&\n-\t\tgit push ../alice-pub master\n+\t\tgit checkout -b foo &&\n+\t\t>file1 &&\n+\t\tgit add . &&\n+\t\tgit commit -m file1 &&\n+\t\tgit checkout master &&\n+\t\t>file2 &&\n+\t\tgit add . &&\n+\t\tgit commit -m file2 &&\n+\t\tgit push ../alice-pub master foo\n \t) &&\n \n \t# Project Bob is a fork of project Alice\n-- \n1.7.8\n"},{"id":"181072","messageId":"1323806811-5798-3-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 2/6] add_extra_ref(): remove flag argument","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:47Z","receivedAt":"2011-12-13T20:06:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThe argument was always set to 0 (and other values do not make sense)\nso remove the argument.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/clone.c        |    4 ++--\n builtin/receive-pack.c |    2 +-\n refs.c                 |    4 ++--\n refs.h                 |    2 +-\n 4 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex efe8b6c..5035767 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -252,7 +252,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \ttransport = transport_get(remote, ref_git);\n \tfor (extra = transport_get_remote_refs(transport); extra;\n \t     extra = extra->next)\n-\t\tadd_extra_ref(extra->name, extra->old_sha1, 0);\n+\t\tadd_extra_ref(extra->name, extra->old_sha1);\n \n \ttransport_disconnect(transport);\n \tfree(ref_git);\n@@ -441,7 +441,7 @@ static void write_remote_refs(const struct ref *local_refs)\n \tfor (r = local_refs; r; r = r->next) {\n \t\tif (!r->peer_ref)\n \t\t\tcontinue;\n-\t\tadd_extra_ref(r->peer_ref->name, r->old_sha1, 0);\n+\t\tadd_extra_ref(r->peer_ref->name, r->old_sha1);\n \t}\n \n \tpack_refs(PACK_REFS_ALL);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex b6d957c..e3b46ce 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -872,7 +872,7 @@ static int delete_only(struct command *commands)\n \n static void add_one_alternate_sha1(const unsigned char sha1[20], void *unused)\n {\n-\tadd_extra_ref(\".have\", sha1, 0);\n+\tadd_extra_ref(\".have\", sha1);\n }\n \n static void collect_one_alternate_ref(const struct ref *ref, void *data)\ndiff --git a/refs.c b/refs.c\nindex f5cb297..6115487 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -248,9 +248,9 @@ static void read_packed_refs(FILE *f, struct ref_array *array)\n \tsort_ref_array(array);\n }\n \n-void add_extra_ref(const char *name, const unsigned char *sha1, int flag)\n+void add_extra_ref(const char *name, const unsigned char *sha1)\n {\n-\tadd_ref(name, sha1, flag, 0, &extra_refs, NULL);\n+\tadd_ref(name, sha1, 0, 0, &extra_refs, NULL);\n }\n \n void clear_extra_refs(void)\ndiff --git a/refs.h b/refs.h\nindex 3fd5536..39bb289 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -56,7 +56,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn\n  * called. Only extra refs added before for_each_ref() is called will\n  * be listed on a given call of for_each_ref().\n  */\n-extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);\n+extern void add_extra_ref(const char *refname, const unsigned char *sha1);\n extern void clear_extra_refs(void);\n extern int ref_exists(const char *);\n \n-- \n1.7.8\n"},{"id":"181075","messageId":"1323806811-5798-4-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 3/6] Extract a function do_for_each_extra_ref()","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:48Z","receivedAt":"2011-12-13T20:06:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nWe want to hold the extra refs at arms length.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   21 ++++++++++++++++-----\n 1 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6115487..268bda9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -693,17 +693,28 @@ fallback:\n \treturn -1;\n }\n \n+static int do_for_each_extra_ref(const char *base, each_ref_fn fn,\n+\t\t\t\t int trim, int flags, void *cb_data)\n+{\n+\tint i;\n+\tfor (i = 0; i < extra_refs.nr; i++) {\n+\t\tint retval = do_one_ref(base, fn, trim, flags, cb_data, extra_refs.refs[i]);\n+\t\tif (retval)\n+\t\t\treturn retval;\n+\t}\n+\treturn 0;\n+}\n+\n static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,\n \t\t\t   int trim, int flags, void *cb_data)\n {\n-\tint retval = 0, i, p = 0, l = 0;\n+\tint retval = 0, p = 0, l = 0;\n \tstruct ref_array *packed = get_packed_refs(submodule);\n \tstruct ref_array *loose = get_loose_refs(submodule);\n \n-\tstruct ref_array *extra = &extra_refs;\n-\n-\tfor (i = 0; i < extra->nr; i++)\n-\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);\n+\tretval = do_for_each_extra_ref(base, fn, trim, flags, cb_data);\n+\tif (retval)\n+\t\tgoto end_each;\n \n \twhile (p < packed->nr && l < loose->nr) {\n \t\tstruct ref_entry *entry;\n-- \n1.7.8\n"},{"id":"181071","messageId":"1323806811-5798-5-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 4/6] Store extra_refs in a separate data structure","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:49Z","receivedAt":"2011-12-13T20:06:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nExtra references really don't have much to do with real references,\nand we want to change how real references are handled.  So hold extra\nreferences in a separate data structure.\n\nSince extra references cannot be broken, don't store flags with them\nand don't pass the flags argument (a different \"flags\"!) to\ndo_for_each_extra_ref().\n\nFinally, current_ref is not useful while iterating through the\nextra_refs.  In fact, peel_ref() doesn't even work for extra refs\nbecause they cannot be peeled themselves, and moreover they might\ncoincidentally have the same name as a \"real\" reference.  So\nexplicitly set current_ref to NULL while iterating over extra_refs to\navoid confusion.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nStrictly speaking, this patch changes how an extra reference with an\ninvalid SHA1 would be treated.  In the old code, it would be skipped\nover (assuming DO_FOR_EACH_INCLUDE_BROKEN is not set).  In the new\ncode the SHA1 is not checked.  However, from my understanding of how\nextra_refs are used, they should never have invalid SHA1s and so the\nold test is superfluous.\n\n refs.c |   38 ++++++++++++++++++++++++++++----------\n 1 files changed, 28 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 268bda9..843c530 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -146,7 +146,11 @@ static struct ref_cache {\n \n static struct ref_entry *current_ref;\n \n-static struct ref_array extra_refs;\n+static struct extra_ref {\n+\tstruct extra_ref *next;\n+\tunsigned char sha1[20];\n+\tchar name[FLEX_ARRAY];\n+} *extra_refs;\n \n static void free_ref_array(struct ref_array *array)\n {\n@@ -250,12 +254,21 @@ static void read_packed_refs(FILE *f, struct ref_array *array)\n \n void add_extra_ref(const char *name, const unsigned char *sha1)\n {\n-\tadd_ref(name, sha1, 0, 0, &extra_refs, NULL);\n+\tint len = strlen(name) + 1;\n+\tstruct extra_ref *entry = xmalloc(sizeof(struct extra_ref) + len);\n+\thashcpy(entry->sha1, sha1);\n+\tmemcpy(entry->name, name, len);\n+\tentry->next = extra_refs;\n+\textra_refs = entry;\n }\n \n void clear_extra_refs(void)\n {\n-\tfree_ref_array(&extra_refs);\n+\twhile (extra_refs) {\n+\t\tstruct extra_ref *entry = extra_refs;\n+\t\textra_refs = entry->next;\n+\t\tfree(entry);\n+\t}\n }\n \n static struct ref_array *get_packed_refs(const char *submodule)\n@@ -694,13 +707,18 @@ fallback:\n }\n \n static int do_for_each_extra_ref(const char *base, each_ref_fn fn,\n-\t\t\t\t int trim, int flags, void *cb_data)\n+\t\t\t\t int trim, void *cb_data)\n {\n-\tint i;\n-\tfor (i = 0; i < extra_refs.nr; i++) {\n-\t\tint retval = do_one_ref(base, fn, trim, flags, cb_data, extra_refs.refs[i]);\n-\t\tif (retval)\n-\t\t\treturn retval;\n+\tstruct extra_ref *extra;\n+\n+\tcurrent_ref = NULL;\n+\tfor (extra = extra_refs; extra; extra = extra->next) {\n+\t\tif (!prefixcmp(extra->name, base)) {\n+\t\t\tint retval = fn(extra->name + trim, extra->sha1,\n+\t\t\t\t\t0, cb_data);\n+\t\t\tif (retval)\n+\t\t\t\treturn retval;\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -712,7 +730,7 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn\n \tstruct ref_array *packed = get_packed_refs(submodule);\n \tstruct ref_array *loose = get_loose_refs(submodule);\n \n-\tretval = do_for_each_extra_ref(base, fn, trim, flags, cb_data);\n+\tretval = do_for_each_extra_ref(base, fn, trim, cb_data);\n \tif (retval)\n \t\tgoto end_each;\n \n-- \n1.7.8\n"},{"id":"181074","messageId":"1323806811-5798-6-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 5/6] Omit extra_refs except when iterating using for_each_ref()","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:50Z","receivedAt":"2011-12-13T20:06:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nAccording to Junio, the only reference iteration function that needs\nto include the extra refs is for_each_ref().  So call\ndo_for_each_extra_ref() directly from there instead of from\ndo_for_each_ref().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |    7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 843c530..f52d8b5 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -730,10 +730,6 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn\n \tstruct ref_array *packed = get_packed_refs(submodule);\n \tstruct ref_array *loose = get_loose_refs(submodule);\n \n-\tretval = do_for_each_extra_ref(base, fn, trim, cb_data);\n-\tif (retval)\n-\t\tgoto end_each;\n-\n \twhile (p < packed->nr && l < loose->nr) {\n \t\tstruct ref_entry *entry;\n \t\tint cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);\n@@ -798,6 +794,9 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n \n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n+\tint retval = do_for_each_extra_ref(\"\", fn, 0, cb_data);\n+\tif (retval)\n+\t\treturn retval;\n \treturn do_for_each_ref(NULL, \"\", fn, 0, 0, cb_data);\n }\n \n-- \n1.7.8\n"},{"id":"181073","messageId":"1323806811-5798-7-git-send-email-mhagger@alum.mit.edu","threadId":"29152","inReplyTo":"1323806811-5798-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 6/6] do_for_each_extra_ref(): simplify signature","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T20:06:51Z","receivedAt":"2011-12-13T20:06:51Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nNow that do_for_each_extra_ref() is only called from one place, it can\nbe less general.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   14 +++++---------\n 1 files changed, 5 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f52d8b5..4b2ba3f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -706,19 +706,15 @@ fallback:\n \treturn -1;\n }\n \n-static int do_for_each_extra_ref(const char *base, each_ref_fn fn,\n-\t\t\t\t int trim, void *cb_data)\n+static int do_for_each_extra_ref(each_ref_fn fn, void *cb_data)\n {\n \tstruct extra_ref *extra;\n \n \tcurrent_ref = NULL;\n \tfor (extra = extra_refs; extra; extra = extra->next) {\n-\t\tif (!prefixcmp(extra->name, base)) {\n-\t\t\tint retval = fn(extra->name + trim, extra->sha1,\n-\t\t\t\t\t0, cb_data);\n-\t\t\tif (retval)\n-\t\t\t\treturn retval;\n-\t\t}\n+\t\tint retval = fn(extra->name, extra->sha1, 0, cb_data);\n+\t\tif (retval)\n+\t\t\treturn retval;\n \t}\n \treturn 0;\n }\n@@ -794,7 +790,7 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n \n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n-\tint retval = do_for_each_extra_ref(\"\", fn, 0, cb_data);\n+\tint retval = do_for_each_extra_ref(fn, cb_data);\n \tif (retval)\n \t\treturn retval;\n \treturn do_for_each_ref(NULL, \"\", fn, 0, 0, cb_data);\n-- \n1.7.8\n"}]}