{"thread":{"id":"32182","subject":"[PATCH v6 p2 0/9] transport-helper and fast-export fixes","startedAt":"2012-11-24T03:25:11Z","lastAt":"2012-11-28T02:15:16Z","messageCount":16,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":6,"patchTotal":9},"messages":[{"id":"203743","messageId":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":null,"subject":"[PATCH v6 p2 0/9] transport-helper and fast-export fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:11Z","receivedAt":"2012-11-24T03:25:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nTrying to fix the remaining issues with transport-helper I stumbled upon a\nproblem with the first patch attached. Now that the namespaced refs of the\nremote helper are properly tracked, there's a problem when pushing more than\none ref at the same time *and* the last patch on the v5 patch series is\napplied.\n\nThe second patch here tries to solve that problem.\n\nThe rest are cleanups and trivial fixes.\n\nFelipe Contreras (9):\n  transport-helper: update remote helper namespace\n  fast-export: don't handle uninteresting refs\n  transport-helper: trivial code shuffle\n  transport-helper: fix push without refspec\n  transport-helper: fix pushing with straight refspec\n  transport-helper: fix push without marks\n  fast-export: make extra_refs global\n  fast-export: refactor get_tags_and_duplicates()\n  fast-export: trivial cleanups\n\n builtin/fast-export.c     | 99 +++++++++++++++++++++++++----------------------\n t/t5801-remote-helpers.sh | 14 +++++--\n t/t9350-fast-export.sh    | 30 ++++++++++++++\n transport-helper.c        | 39 ++++++++++++-------\n 4 files changed, 120 insertions(+), 62 deletions(-)\n\n-- \n1.8.0\n"},{"id":"203744","messageId":"1353727520-26039-2-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 1/9] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:12Z","receivedAt":"2012-11-24T03:25:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"When pushing, the remote namespace is updated correctly\n(e.g. refs/origin/master), but not the remote helper's\n(e.g. refs/testgit/origin/master).\n\nThis alone should not cause any regressions, but combined with other\npatches to handle negative refs correctly (upcoming patch), it might.\nHowever, that's a good thing;  otherwise those issues would go\nunnoticed.\n\nFor the moment though, this patch alone shouldn't cause any issues, in\nfact the rest of the code seems to rely on this happening.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n transport-helper.c | 22 ++++++++++++++++++----\n 1 file changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex cfe0988..32ad877 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -10,6 +10,7 @@\n #include \"string-list.h\"\n #include \"thread-utils.h\"\n #include \"sigchain.h\"\n+#include \"refs.h\"\n \n static int debug;\n \n@@ -600,7 +601,7 @@ static int fetch(struct transport *transport,\n \treturn -1;\n }\n \n-static void push_update_ref_status(struct strbuf *buf,\n+static int push_update_ref_status(struct strbuf *buf,\n \t\t\t\t   struct ref **ref,\n \t\t\t\t   struct ref *remote_refs)\n {\n@@ -651,7 +652,7 @@ static void push_update_ref_status(struct strbuf *buf,\n \t\t*ref = find_ref_by_name(remote_refs, refname);\n \tif (!*ref) {\n \t\twarning(\"helper reported unexpected status of %s\", refname);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \tif ((*ref)->status != REF_STATUS_NONE) {\n@@ -660,11 +661,12 @@ static void push_update_ref_status(struct strbuf *buf,\n \t\t * status reported by the remote helper if the latter is 'no match'.\n \t\t */\n \t\tif (status == REF_STATUS_NONE)\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t}\n \n \t(*ref)->status = status;\n \t(*ref)->remote_status = msg;\n+\treturn 0;\n }\n \n static void push_update_refs_status(struct helper_data *data,\n@@ -673,11 +675,23 @@ static void push_update_refs_status(struct helper_data *data,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct ref *ref = remote_refs;\n \tfor (;;) {\n+\t\tchar *private;\n+\n \t\trecvline(data, &buf);\n \t\tif (!buf.len)\n \t\t\tbreak;\n \n-\t\tpush_update_ref_status(&buf, &ref, remote_refs);\n+\t\tif (push_update_ref_status(&buf, &ref, remote_refs))\n+\t\t\tcontinue;\n+\n+\t\tif (!data->refspecs)\n+\t\t\tcontinue;\n+\n+\t\t/* propagate back the update to the remote namespace */\n+\t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n+\t\tif (!private)\n+\t\t\tcontinue;\n+\t\tupdate_ref(\"update by helper\", private, ref->new_sha1, NULL, 0, 0);\n \t}\n \tstrbuf_release(&buf);\n }\n-- \n1.8.0\n"},{"id":"203745","messageId":"1353727520-26039-3-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 2/9] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:13Z","receivedAt":"2012-11-24T03:25:13Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"They have been marked as UNINTERESTING for a reason, lets respect that.\n\nCurrently the first ref is handled properly, but not the rest:\n\n  % git fast-export master ^uninteresting ^foo ^bar\n  reset refs/heads/bar\n  from :0\n\n  reset refs/heads/foo\n  from :0\n\n  reset refs/heads/uninteresting\n  from :0\n\n  % git fast-export ^uninteresting ^foo ^bar master\n  reset refs/heads/master\n  from :0\n\n  reset refs/heads/bar\n  from :0\n\n  reset refs/heads/foo\n  from :0\n\nClearly this is wrong; the negative refs should be ignored.\n\nAfter this patch:\n\n  % git fast-export ^uninteresting ^foo ^bar master\n  # nothing\n  % git fast-export master ^uninteresting ^foo ^bar\n  # nothing\n\nAnd even more, it would only happen if the ref is pointing to exactly\nthe same commit, but not otherwise:\n\n % git fast-export ^next next\n reset refs/heads/next\n from :0\n\n % git fast-export ^next next^{commit}\n # nothing\n % git fast-export ^next next~0\n # nothing\n % git fast-export ^next next~1\n # nothing\n % git fast-export ^next next~2\n # nothing\n\nThe reason this happens is that before traversing the commits,\nfast-export checks if any of the refs point to the same object, and any\nduplicated ref gets added to a list in order to issue 'reset' commands\nafter the traversing. Unfortunately, it's not even checking if the\ncommit is flagged as UNINTERESTING. The fix of course, is to do\nprecisely that.\n\nHowever, in order to do it properly we need to get the UNINTERESTING flag\nfrom the command line ref, not from the commit object. Fortunately we\ncan simply use revs.pending, which contains all the information we need\nfor get_tags_and_duplicates(), plus the ref flag. This way the rest of\nthe positive refs will remain untouched; it's only the negative ones\nthat change in behavior.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n\nThe difference with the previous version is that now '^uninteresting ^bad\nmaster new' would handle 'master' and 'new', while the previous one would not\ndo anything. The current behavior would handle 'bad', 'master', and 'new'.\n\n builtin/fast-export.c     | 11 +++++++----\n t/t5801-remote-helpers.sh |  8 ++++++++\n t/t9350-fast-export.sh    | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 31bfbee..77dffd1 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag *tag)\n \t       (int)message_size, (int)message_size, message ? message : \"\");\n }\n \n-static void get_tags_and_duplicates(struct object_array *pending,\n+static void get_tags_and_duplicates(struct rev_cmdline_info *info,\n \t\t\t\t    struct string_list *extra_refs)\n {\n \tstruct tag *tag;\n \tint i;\n \n-\tfor (i = 0; i < pending->nr; i++) {\n-\t\tstruct object_array_entry *e = pending->objects + i;\n+\tfor (i = 0; i < info->nr; i++) {\n+\t\tstruct rev_cmdline_entry *e = info->rev + i;\n \t\tunsigned char sha1[20];\n \t\tstruct commit *commit;\n \t\tchar *full_name;\n \n+\t\tif (e->flags & UNINTERESTING)\n+\t\t\tcontinue;\n+\n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)\n \t\t\tcontinue;\n \n@@ -685,7 +688,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (import_filename && revs.prune_data.nr)\n \t\tfull_tree = 1;\n \n-\tget_tags_and_duplicates(&revs.pending, &extra_refs);\n+\tget_tags_and_duplicates(&revs.cmdline, &extra_refs);\n \n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex b2782a2..2e027c8 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -158,4 +158,12 @@ test_expect_success 'push ref with existing object' '\n \tcompare_refs local dup server dup\n '\n \n+test_expect_success 'push all with existing object' '\n+\t(cd local &&\n+\tgit branch dup2 master &&\n+\tgit push origin --all\n+\t) &&\n+\tcompare_refs local dup2 server dup2\n+'\n+\n test_done\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 237d2e5..2e7187e 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -469,4 +469,34 @@ test_expect_success 'refs are updated even if no commits need to be exported' '\n \ttest_cmp expected actual\n '\n \n+cat > expected << EOF\n+blob\n+mark :13\n+data 5\n+bump\n+\n+commit refs/heads/master\n+mark :14\n+author A U Thor <author@example.com> 1112912773 -0700\n+committer C O Mitter <committer@example.com> 1112912773 -0700\n+data 5\n+bump\n+from :12\n+M 100644 :13 file\n+\n+EOF\n+\n+test_expect_success 'refs are updated even if no commits need to be exported' '\n+\t> tmp-marks &&\n+\tgit fast-export --import-marks=tmp-marks \\\n+\t\t--export-marks=tmp-marks master > /dev/null &&\n+\tgit tag v1.0 &&\n+\tgit branch uninteresting &&\n+\techo bump > file &&\n+\tgit commit -a -m bump &&\n+\tgit fast-export --import-marks=tmp-marks \\\n+\t\t--export-marks=tmp-marks ^uninteresting ^v1.0 master > actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"203746","messageId":"1353727520-26039-4-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 3/9] transport-helper: trivial code shuffle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:14Z","receivedAt":"2012-11-24T03:25:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Just shuffle the die() part to make it more explicit, and cleanup the\ncode-style.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n transport-helper.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 32ad877..0c95101 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -775,6 +775,9 @@ static int push_refs_with_export(struct transport *transport,\n \t\tchar *private;\n \t\tunsigned char sha1[20];\n \n+\t\tif (ref->deletion)\n+\t\t\tdie(\"remote-helpers do not support ref deletion\");\n+\n \t\tif (!data->refspecs)\n \t\t\tcontinue;\n \t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n@@ -784,10 +787,6 @@ static int push_refs_with_export(struct transport *transport,\n \t\t}\n \t\tfree(private);\n \n-\t\tif (ref->deletion) {\n-\t\t\tdie(\"remote-helpers do not support ref deletion\");\n-\t\t}\n-\n \t\tif (ref->peer_ref)\n \t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n \n-- \n1.8.0\n"},{"id":"203747","messageId":"1353727520-26039-5-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 4/9] transport-helper: fix push without refspec","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:15Z","receivedAt":"2012-11-24T03:25:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The refspec feature is not mandatory.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t5801-remote-helpers.sh | 2 +-\n transport-helper.c        | 6 +++---\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 2e027c8..b268cd2 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -111,7 +111,7 @@ test_expect_success 'pulling without refspecs' '\n \tcompare_refs local2 HEAD server HEAD\n '\n \n-test_expect_failure 'pushing without refspecs' '\n+test_expect_success 'pushing without refspecs' '\n \ttest_when_finished \"(cd local2 && git reset --hard origin)\" &&\n \t(cd local2 &&\n \techo content >>file &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 0c95101..899eb36 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -778,6 +778,9 @@ static int push_refs_with_export(struct transport *transport,\n \t\tif (ref->deletion)\n \t\t\tdie(\"remote-helpers do not support ref deletion\");\n \n+\t\tif (ref->peer_ref)\n+\t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n+\n \t\tif (!data->refspecs)\n \t\t\tcontinue;\n \t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n@@ -787,9 +790,6 @@ static int push_refs_with_export(struct transport *transport,\n \t\t}\n \t\tfree(private);\n \n-\t\tif (ref->peer_ref)\n-\t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n-\n \t}\n \n \tif (get_exporter(transport, &exporter, &revlist_args))\n-- \n1.8.0\n"},{"id":"203748","messageId":"1353727520-26039-6-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 5/9] transport-helper: fix pushing with straight refspec","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:16Z","receivedAt":"2012-11-24T03:25:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"For example '*:*'.\n\nObviously '^refs/heads/master refs/heads/master' is not going to work,\nso lets check that the ref we are negating is not the same we are\npushing.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t5801-remote-helpers.sh | 2 +-\n transport-helper.c        | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex b268cd2..9c7871b 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -126,7 +126,7 @@ test_expect_success 'pulling with straight refspec' '\n \tcompare_refs local2 HEAD server HEAD\n '\n \n-test_expect_failure 'pushing with straight refspec' '\n+test_expect_success 'pushing with straight refspec' '\n \ttest_when_finished \"(cd local2 && git reset --hard origin)\" &&\n \t(cd local2 &&\n \techo content >>file &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 899eb36..6dbb72e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -784,7 +784,7 @@ static int push_refs_with_export(struct transport *transport,\n \t\tif (!data->refspecs)\n \t\t\tcontinue;\n \t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n-\t\tif (private && !get_sha1(private, sha1)) {\n+\t\tif (private && strcmp(private, ref->name) && !get_sha1(private, sha1)) {\n \t\t\tstrbuf_addf(&buf, \"^%s\", private);\n \t\t\tstring_list_append(&revlist_args, strbuf_detach(&buf, NULL));\n \t\t}\n-- \n1.8.0\n"},{"id":"203750","messageId":"1353727520-26039-7-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 6/9] transport-helper: fix push without marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:17Z","receivedAt":"2012-11-24T03:25:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's not much to do when marks are not available, except pushing\neverything, so let's do so by avoiding the negative refs (e.g.\n^refs/testgit/origin/master).\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t5801-remote-helpers.sh | 2 +-\n transport-helper.c        | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 9c7871b..456303b 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -141,7 +141,7 @@ test_expect_success 'pulling without marks' '\n \tcompare_refs local2 HEAD server HEAD\n '\n \n-test_expect_failure 'pushing without marks' '\n+test_expect_success 'pushing without marks' '\n \ttest_when_finished \"(cd local2 && git reset --hard origin)\" &&\n \t(cd local2 &&\n \techo content >>file &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 6dbb72e..78e4e82 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -781,7 +781,7 @@ static int push_refs_with_export(struct transport *transport,\n \t\tif (ref->peer_ref)\n \t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n \n-\t\tif (!data->refspecs)\n+\t\tif (!data->refspecs || !data->import_marks)\n \t\t\tcontinue;\n \t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n \t\tif (private && strcmp(private, ref->name) && !get_sha1(private, sha1)) {\n-- \n1.8.0\n"},{"id":"203749","messageId":"1353727520-26039-8-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 7/9] fast-export: make extra_refs global","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:18Z","receivedAt":"2012-11-24T03:25:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to pass it around everywhere. This would make easier\nfurther refactoring that makes use of this variable.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/fast-export.c | 21 ++++++++++-----------\n 1 file changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 77dffd1..5035382 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -30,6 +30,7 @@ static int fake_missing_tagger;\n static int use_done_feature;\n static int no_data;\n static int full_tree;\n+static struct string_list extra_refs = STRING_LIST_INIT_NODUP;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n \t\t\t\t     const char *arg, int unset)\n@@ -474,8 +475,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t       (int)message_size, (int)message_size, message ? message : \"\");\n }\n \n-static void get_tags_and_duplicates(struct rev_cmdline_info *info,\n-\t\t\t\t    struct string_list *extra_refs)\n+static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n {\n \tstruct tag *tag;\n \tint i;\n@@ -502,7 +502,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info,\n \t\t\t/* handle nested tags */\n \t\t\twhile (tag && tag->object.type == OBJ_TAG) {\n \t\t\t\tparse_object(tag->object.sha1);\n-\t\t\t\tstring_list_append(extra_refs, full_name)->util = tag;\n+\t\t\t\tstring_list_append(&extra_refs, full_name)->util = tag;\n \t\t\t\ttag = (struct tag *)tag->tagged;\n \t\t\t}\n \t\t\tif (!tag)\n@@ -532,20 +532,20 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info,\n \t\t * sure it gets properly updated eventually.\n \t\t */\n \t\tif (commit->util || commit->object.flags & SHOWN)\n-\t\t\tstring_list_append(extra_refs, full_name)->util = commit;\n+\t\t\tstring_list_append(&extra_refs, full_name)->util = commit;\n \t\tif (!commit->util)\n \t\t\tcommit->util = full_name;\n \t}\n }\n \n-static void handle_tags_and_duplicates(struct string_list *extra_refs)\n+static void handle_tags_and_duplicates(void)\n {\n \tstruct commit *commit;\n \tint i;\n \n-\tfor (i = extra_refs->nr - 1; i >= 0; i--) {\n-\t\tconst char *name = extra_refs->items[i].string;\n-\t\tstruct object *object = extra_refs->items[i].util;\n+\tfor (i = extra_refs.nr - 1; i >= 0; i--) {\n+\t\tconst char *name = extra_refs.items[i].string;\n+\t\tstruct object *object = extra_refs.items[i].util;\n \t\tswitch (object->type) {\n \t\tcase OBJ_TAG:\n \t\t\thandle_tag(name, (struct tag *)object);\n@@ -638,7 +638,6 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n \tstruct object_array commits = OBJECT_ARRAY_INIT;\n-\tstruct string_list extra_refs = STRING_LIST_INIT_NODUP;\n \tstruct commit *commit;\n \tchar *export_filename = NULL, *import_filename = NULL;\n \tstruct option options[] = {\n@@ -688,7 +687,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (import_filename && revs.prune_data.nr)\n \t\tfull_tree = 1;\n \n-\tget_tags_and_duplicates(&revs.cmdline, &extra_refs);\n+\tget_tags_and_duplicates(&revs.cmdline);\n \n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n@@ -704,7 +703,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\thandle_tags_and_duplicates(&extra_refs);\n+\thandle_tags_and_duplicates();\n \n \tif (export_filename)\n \t\texport_marks(export_filename);\n-- \n1.8.0\n"},{"id":"203751","messageId":"1353727520-26039-9-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 8/9] fast-export: refactor get_tags_and_duplicates()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:19Z","receivedAt":"2012-11-24T03:25:19Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Split into a separate helper function get_commit() so that the part that\nfinds the relevant commit, and the part that does something with it\n(handle tag object, etc.) are in different places.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/fast-export.c | 68 ++++++++++++++++++++++++++++-----------------------\n 1 file changed, 38 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 5035382..d88fa10 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -475,9 +475,32 @@ static void handle_tag(const char *name, struct tag *tag)\n \t       (int)message_size, (int)message_size, message ? message : \"\");\n }\n \n+static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)\n+{\n+\tswitch (e->item->type) {\n+\tcase OBJ_COMMIT:\n+\t\treturn (struct commit *)e->item;\n+\tcase OBJ_TAG: {\n+\t\tstruct tag *tag = (struct tag *)e->item;\n+\n+\t\t/* handle nested tags */\n+\t\twhile (tag && tag->object.type == OBJ_TAG) {\n+\t\t\tparse_object(tag->object.sha1);\n+\t\t\tstring_list_append(&extra_refs, full_name)->util = tag;\n+\t\t\ttag = (struct tag *)tag->tagged;\n+\t\t}\n+\t\tif (!tag)\n+\t\t\tdie(\"Tag %s points nowhere?\", e->name);\n+\t\treturn (struct commit *)tag;\n+\t\tbreak;\n+\t}\n+\tdefault:\n+\t\treturn NULL;\n+\t}\n+}\n+\n static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n {\n-\tstruct tag *tag;\n \tint i;\n \n \tfor (i = 0; i < info->nr; i++) {\n@@ -492,41 +515,26 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)\n \t\t\tcontinue;\n \n-\t\tswitch (e->item->type) {\n-\t\tcase OBJ_COMMIT:\n-\t\t\tcommit = (struct commit *)e->item;\n-\t\t\tbreak;\n-\t\tcase OBJ_TAG:\n-\t\t\ttag = (struct tag *)e->item;\n-\n-\t\t\t/* handle nested tags */\n-\t\t\twhile (tag && tag->object.type == OBJ_TAG) {\n-\t\t\t\tparse_object(tag->object.sha1);\n-\t\t\t\tstring_list_append(&extra_refs, full_name)->util = tag;\n-\t\t\t\ttag = (struct tag *)tag->tagged;\n-\t\t\t}\n-\t\t\tif (!tag)\n-\t\t\t\tdie (\"Tag %s points nowhere?\", e->name);\n-\t\t\tswitch(tag->object.type) {\n-\t\t\tcase OBJ_COMMIT:\n-\t\t\t\tcommit = (struct commit *)tag;\n-\t\t\t\tbreak;\n-\t\t\tcase OBJ_BLOB:\n-\t\t\t\thandle_object(tag->object.sha1);\n-\t\t\t\tcontinue;\n-\t\t\tdefault: /* OBJ_TAG (nested tags) is already handled */\n-\t\t\t\twarning(\"Tag points to object of unexpected type %s, skipping.\",\n-\t\t\t\t\ttypename(tag->object.type));\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tbreak;\n-\t\tdefault:\n+\t\tcommit = get_commit(e, full_name);\n+\t\tif (!commit) {\n \t\t\twarning(\"%s: Unexpected object of type %s, skipping.\",\n \t\t\t\te->name,\n \t\t\t\ttypename(e->item->type));\n \t\t\tcontinue;\n \t\t}\n \n+\t\tswitch(commit->object.type) {\n+\t\tcase OBJ_COMMIT:\n+\t\t\tbreak;\n+\t\tcase OBJ_BLOB:\n+\t\t\thandle_object(commit->object.sha1);\n+\t\t\tcontinue;\n+\t\tdefault: /* OBJ_TAG (nested tags) is already handled */\n+\t\t\twarning(\"Tag points to object of unexpected type %s, skipping.\",\n+\t\t\t\ttypename(commit->object.type));\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/*\n \t\t * This ref will not be updated through a commit, lets make\n \t\t * sure it gets properly updated eventually.\n-- \n1.8.0\n"},{"id":"203752","messageId":"1353727520-26039-10-git-send-email-felipe.contreras@gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v6 p2 9/9] fast-export: trivial cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:25:20Z","receivedAt":"2012-11-24T03:25:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/fast-export.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex d88fa10..bf5c822 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -548,7 +548,6 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n \n static void handle_tags_and_duplicates(void)\n {\n-\tstruct commit *commit;\n \tint i;\n \n \tfor (i = extra_refs.nr - 1; i >= 0; i--) {\n@@ -560,9 +559,7 @@ static void handle_tags_and_duplicates(void)\n \t\t\tbreak;\n \t\tcase OBJ_COMMIT:\n \t\t\t/* create refs pointing to already seen commits */\n-\t\t\tcommit = (struct commit *)object;\n-\t\t\tprintf(\"reset %s\\nfrom :%d\\n\\n\", name,\n-\t\t\t       get_object_mark(&commit->object));\n+\t\t\tprintf(\"reset %s\\nfrom :%d\\n\\n\", name, get_object_mark(object));\n \t\t\tshow_progress();\n \t\t\tbreak;\n \t\t}\n-- \n1.8.0\n"},{"id":"203753","messageId":"CAMP44s3Gdr3W5oDL79F_LGqhB77LT6Ay7p8rWWuygbOgHzEdsw@mail.gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v6 p2 2/9] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:27:52Z","receivedAt":"2012-11-24T03:27:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 24, 2012 at 4:25 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n\n> +cat > expected << EOF\n> +blob\n> +mark :13\n> +data 5\n> +bump\n> +\n> +commit refs/heads/master\n> +mark :14\n> +author A U Thor <author@example.com> 1112912773 -0700\n> +committer C O Mitter <committer@example.com> 1112912773 -0700\n> +data 5\n> +bump\n> +from :12\n> +M 100644 :13 file\n> +\n> +EOF\n> +\n> +test_expect_success 'refs are updated even if no commits need to be exported' '\n\nThe title should be updated: 'avoid uninteresting refs'\n\n-- \nFelipe Contreras\n"},{"id":"203754","messageId":"CAMP44s070w14K5ODv=5s2f0agV1aVn3FSBmXMmM6M2UqPd7Sdw@mail.gmail.com","threadId":"32182","inReplyTo":"1353727520-26039-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v6 p2 1/9] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:28:39Z","receivedAt":"2012-11-24T03:28:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 24, 2012 at 4:25 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n\n> @@ -673,11 +675,23 @@ static void push_update_refs_status(struct helper_data *data,\n>         struct strbuf buf = STRBUF_INIT;\n>         struct ref *ref = remote_refs;\n>         for (;;) {\n> +               char *private;\n> +\n>                 recvline(data, &buf);\n>                 if (!buf.len)\n>                         break;\n>\n> -               push_update_ref_status(&buf, &ref, remote_refs);\n> +               if (push_update_ref_status(&buf, &ref, remote_refs))\n> +                       continue;\n> +\n> +               if (!data->refspecs)\n> +                       continue;\n> +\n> +               /* propagate back the update to the remote namespace */\n> +               private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n> +               if (!private)\n> +                       continue;\n> +               update_ref(\"update by helper\", private, ref->new_sha1, NULL, 0, 0);\n\nfree(private); I guess\n\n\n-- \nFelipe Contreras\n"},{"id":"203993","messageId":"7vk3t7nfql.fsf@alter.siamese.dyndns.org","threadId":"32182","inReplyTo":"1353727520-26039-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T17:00:02Z","receivedAt":"2012-11-27T17:00:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Just shuffle the die() part to make it more explicit, and cleanup the\n> code-style.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  transport-helper.c | 7 +++----\n>  1 file changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 32ad877..0c95101 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -775,6 +775,9 @@ static int push_refs_with_export(struct transport *transport,\n>  \t\tchar *private;\n>  \t\tunsigned char sha1[20];\n>  \n> +\t\tif (ref->deletion)\n> +\t\t\tdie(\"remote-helpers do not support ref deletion\");\n> +\n>  \t\tif (!data->refspecs)\n>  \t\t\tcontinue;\n\nThis is not just \"just shuffle the die to make it explicit\" but it\ndoes change the semantics; earlier ref->deletion was perfectly fine\nas long as data->refspecs is not given, but the new code always\ndies.\n\nIf this semantic change is a good thing, please explain why it is so\nin the log message.  If the change is \"it does not matter because\nwhen data->refspecs is not given and ref->deletion is set, we die\nlater elsewhere in the code anyway\", then it needs to be described.\n\n\nThanks.\n\n> @@ -784,10 +787,6 @@ static int push_refs_with_export(struct transport *transport,\n>  \t\t}\n>  \t\tfree(private);\n>  \n> -\t\tif (ref->deletion) {\n> -\t\t\tdie(\"remote-helpers do not support ref deletion\");\n> -\t\t}\n> -\n>  \t\tif (ref->peer_ref)\n>  \t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n"},{"id":"204065","messageId":"CAMP44s0fmt+bHN-ycza8b+y8Ep-Cyqmg1U1PVas267fTY5iPPQ@mail.gmail.com","threadId":"32182","inReplyTo":"7vk3t7nfql.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-28T00:05:34Z","receivedAt":"2012-11-28T00:05:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 27, 2012 at 6:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Just shuffle the die() part to make it more explicit, and cleanup the\n>> code-style.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  transport-helper.c | 7 +++----\n>>  1 file changed, 3 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/transport-helper.c b/transport-helper.c\n>> index 32ad877..0c95101 100644\n>> --- a/transport-helper.c\n>> +++ b/transport-helper.c\n>> @@ -775,6 +775,9 @@ static int push_refs_with_export(struct transport *transport,\n>>               char *private;\n>>               unsigned char sha1[20];\n>>\n>> +             if (ref->deletion)\n>> +                     die(\"remote-helpers do not support ref deletion\");\n>> +\n>>               if (!data->refspecs)\n>>                       continue;\n>\n> This is not just \"just shuffle the die to make it explicit\" but it\n> does change the semantics; earlier ref->deletion was perfectly fine\n> as long as data->refspecs is not given, but the new code always\n> dies.\n>\n> If this semantic change is a good thing, please explain why it is so\n> in the log message.  If the change is \"it does not matter because\n> when data->refspecs is not given and ref->deletion is set, we die\n> later elsewhere in the code anyway\", then it needs to be described.\n\nrefspecs are optional, but when they are not present the code doesn't\nwork at all. This patch changes the behavior that was totally broken\nanyway.\n\n-- \nFelipe Contreras\n"},{"id":"204091","messageId":"7vwqx6jxv2.fsf@alter.siamese.dyndns.org","threadId":"32182","inReplyTo":"CAMP44s0fmt+bHN-ycza8b+y8Ep-Cyqmg1U1PVas267fTY5iPPQ@mail.gmail.com","subject":"Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-28T01:54:25Z","receivedAt":"2012-11-28T01:54:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> This is not just \"just shuffle the die to make it explicit\" but it\n>> does change the semantics; earlier ref->deletion was perfectly fine\n>> as long as data->refspecs is not given, but the new code always\n>> dies.\n>>\n>> If this semantic change is a good thing, please explain why it is so\n>> in the log message.  If the change is \"it does not matter because\n>> when data->refspecs is not given and ref->deletion is set, we die\n>> later elsewhere in the code anyway\", then it needs to be described.\n>\n> refspecs are optional, but when they are not present the code doesn't\n> work at all. This patch changes the behavior that was totally broken\n> anyway.\n\nIn case it was not clear, I did not request/expect responses in the\ndiscussion thread, but a rerolled series with updated description.\n\nThanks.\n"},{"id":"204100","messageId":"CAMP44s2MtEie+tF4MDyzNV3A27zu+UhinS+EkaLE8DMZ71UNOQ@mail.gmail.com","threadId":"32182","inReplyTo":"7vwqx6jxv2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-28T02:15:16Z","receivedAt":"2012-11-28T02:15:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 28, 2012 at 2:54 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>> This is not just \"just shuffle the die to make it explicit\" but it\n>>> does change the semantics; earlier ref->deletion was perfectly fine\n>>> as long as data->refspecs is not given, but the new code always\n>>> dies.\n>>>\n>>> If this semantic change is a good thing, please explain why it is so\n>>> in the log message.  If the change is \"it does not matter because\n>>> when data->refspecs is not given and ref->deletion is set, we die\n>>> later elsewhere in the code anyway\", then it needs to be described.\n>>\n>> refspecs are optional, but when they are not present the code doesn't\n>> work at all. This patch changes the behavior that was totally broken\n>> anyway.\n>\n> In case it was not clear, I did not request/expect responses in the\n> discussion thread, but a rerolled series with updated description.\n\nAn updated description that is irrelevant; the stuff is totally\nbroken, that's my point. But I'm tired of explaining it, and showing\nit with test cases, and patches that fix those test cases, it's not my\nitch, and \"the other camp\" doesn't even bother to acknowledge that\nindeed remote helpers without marks just don't work, or even utter a\nsingle word replying to one of these patches, nothing.\n\nNot blaming you, just saying that I don't think anybody cares, clearly\nnobody is exercising this code.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}