{"thread":{"id":"33758","subject":"[PATCH 0/3] Support for old:new remote-helper push","startedAt":"2013-05-09T01:31:32Z","lastAt":"2013-05-16T09:15:59Z","messageCount":19,"participants":["Felipe Contreras","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"216745","messageId":"1368063095-22861-1-git-send-email-felipe.contreras@gmail.com","threadId":"33758","inReplyTo":null,"subject":"[PATCH 0/3] Support for old:new remote-helper push","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T01:31:32Z","receivedAt":"2013-05-09T01:31:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nIn order to support pushing old:new refspecs in remote-helpers, the best way to\ndo what is to add an option to fast-export to rename the refs in the stream, so\nhere it is:\n\nFelipe Contreras (3):\n fast-export: improve argument parsing\n fast-export: add new --refspec option\n transport-helper: add support for old:new refspec\n\n Documentation/git-fast-export.txt | 4 ++++\n builtin/fast-export.c | 33 ++++++++++++++++++++++++++++++++-\n t/t5801-remote-helpers.sh | 2 +-\n t/t9350-fast-export.sh | 7 +++++++\n transport-helper.c | 14 ++++++++++++--\n 5 files changed, 56 insertions(+), 4 deletions(-)\n\n-- \n1.8.3.rc1.553.gac13664\n"},{"id":"216746","messageId":"1368063095-22861-2-git-send-email-felipe.contreras@gmail.com","threadId":"33758","inReplyTo":"1368063095-22861-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/3] fast-export: improve argument parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T01:31:33Z","receivedAt":"2013-05-09T01:31:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We don't want to pass arguments specific to fast-export to\nsetup_revisions.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex d60d675..6e46057 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \trevs.topo_order = 1;\n \trevs.show_source = 1;\n \trevs.rewrite_parents = 1;\n+\targc = parse_options(argc, argv, prefix, options, fast_export_usage,\n+\t\t\tPARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n \targc = setup_revisions(argc, argv, &revs, NULL);\n-\targc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);\n \tif (argc > 1)\n \t\tusage_with_options (fast_export_usage, options);\n \n-- \n1.8.3.rc1.553.gac13664\n"},{"id":"216747","messageId":"1368063095-22861-3-git-send-email-felipe.contreras@gmail.com","threadId":"33758","inReplyTo":"1368063095-22861-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/3] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T01:31:34Z","receivedAt":"2013-05-09T01:31:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that we can covert the exported ref names.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-fast-export.txt | 4 ++++\n builtin/fast-export.c | 30 ++++++++++++++++++++++++++++++\n t/t9350-fast-export.sh | 7 +++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 03fc8c3..d1985d3 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -105,6 +105,10 @@ marks the same across runs.\n \tin the commit (as opposed to just listing the files which are\n \tdifferent from the commit's first parent).\n \n+--refspec::\n+\tApply the specified refspec to each ref exported. Multiple of them can\n+\tbe specified.\n+\n [...]::\n A list of arguments, acceptable to 'git rev-parse' and\n 'git rev-list', that specifies the specific objects and references\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 6e46057..0097103 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -17,6 +17,7 @@\n #include \"utf8.h\"\n #include \"parse-options.h\"\n #include \"quote.h\"\n+#include \"remote.h\"\n \n static const char *fast_export_usage[] = {\n \tN_(\"git fast-export [rev-list-opts]\"),\n@@ -30,6 +31,8 @@ static int fake_missing_tagger;\n static int use_done_feature;\n static int no_data;\n static int full_tree;\n+static struct refspec *refspecs;\n+static int refspecs_nr;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -502,6 +505,15 @@ 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\tif (refspecs) {\n+\t\t\tchar *private;\n+\t\t\tprivate = apply_refspecs(refspecs, refspecs_nr, full_name);\n+\t\t\tif (private) {\n+\t\t\t\tfree(full_name);\n+\t\t\t\tfull_name = private;\n+\t\t\t}\n+\t\t}\n+\n \t\tswitch (e->item->type) {\n \t\tcase OBJ_COMMIT:\n \t\t\tcommit = (struct commit *)e->item;\n@@ -653,6 +665,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tstruct commit *commit;\n \tchar *export_filename = NULL, *import_filename = NULL;\n \tuint32_t lastimportid;\n+\tstruct string_list refspecs_list;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"progress\", &progress,\n \t\t\t N_(\"show progress after objects\")),\n@@ -673,6 +686,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN(0, \"use-done-feature\", &use_done_feature,\n \t\t\t N_(\"Use the done feature to terminate the stream\")),\n \t\tOPT_BOOL(0, \"no-data\", &no_data, N_(\"Skip output of blob data\")),\n+\t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n+\t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_END()\n \t};\n \n@@ -692,6 +707,19 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (argc > 1)\n \t\tusage_with_options (fast_export_usage, options);\n \n+\tif (refspecs_list.nr) {\n+\t\tconst char *refspecs_str[refspecs_list.nr];\n+\t\tint i;\n+\n+\t\tfor (i = 0; i < refspecs_list.nr; i++)\n+\t\t\trefspecs_str[i] = refspecs_list.items[i].string;\n+\n+\t\trefspecs_nr = refspecs_list.nr;\n+\t\trefspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);\n+\n+\t\tstring_list_clear(&refspecs_list, 1);\n+\t}\n+\n \tif (use_done_feature)\n \t\tprintf(\"feature done\\n\");\n \n@@ -726,5 +754,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (use_done_feature)\n \t\tprintf(\"done\\n\");\n \n+\tfree_refspec(refspecs_nr, refspecs);\n+\n \treturn 0;\n }\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 2471bc6..ef2d76e 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -505,4 +505,11 @@ test_expect_success 'refs are updated even if no commits need to be exported' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'use refspec' '\n+\tgit fast-export --refspec refs/heads/master:refs/heads/foobar master | \\\n+\t\tgrep \"^commit \" | sort | uniq > actual &&\n+\techo \"commit refs/heads/foobar\" > expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.3.rc1.553.gac13664\n"},{"id":"216748","messageId":"1368063095-22861-4-git-send-email-felipe.contreras@gmail.com","threadId":"33758","inReplyTo":"1368063095-22861-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/3] transport-helper: add support for old:new refspec","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T01:31:35Z","receivedAt":"2013-05-09T01:31:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"By using fast-export's new --refspec option.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 2 +-\n transport-helper.c | 14 ++++++++++++--\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex dbb02e2..d15f794 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -92,7 +92,7 @@ test_expect_success 'push new branch by name' '\n \tcompare_refs local HEAD server refs/heads/new-name\n '\n \n-test_expect_failure 'push new branch with old:new refspec' '\n+test_expect_success 'push new branch with old:new refspec' '\n \t(cd local &&\n \t git push origin new-name:new-refspec\n \t) &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 835815f..b1fdd39 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -787,7 +787,7 @@ static int push_refs_with_export(struct transport *transport,\n \tstruct ref *ref;\n \tstruct child_process *helper, exporter;\n \tstruct helper_data *data = transport->data;\n-\tstruct string_list revlist_args = STRING_LIST_INIT_NODUP;\n+\tstruct string_list revlist_args = STRING_LIST_INIT_DUP;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \thelper = get_helper(transport);\n@@ -814,14 +814,24 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\tdie(\"remote-helpers do not support ref deletion\");\n \t\t}\n \n-\t\tif (ref->peer_ref)\n+\t\tif (ref->peer_ref) {\n+\t\t\tif (strcmp(ref->name, ref->peer_ref->name)) {\n+\t\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\t\tstrbuf_addf(&buf, \"%s:%s\", ref->peer_ref->name, ref->name);\n+\t\t\t\tstring_list_append(&revlist_args, \"--refspec\");\n+\t\t\t\tstring_list_append(&revlist_args, buf.buf);\n+\t\t\t\tstrbuf_release(&buf);\n+\t\t\t}\n \t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n+\t\t}\n \n \t}\n \n \tif (get_exporter(transport, &exporter, &revlist_args))\n \t\tdie(\"Couldn't run fast-export\");\n \n+\tstring_list_clear(&revlist_args, 1);\n+\n \tif (finish_command(&exporter))\n \t\tdie(\"Error while running fast-export\");\n \tpush_update_refs_status(data, remote_refs);\n-- \n1.8.3.rc1.553.gac13664\n"},{"id":"216822","messageId":"7vzjw37q7o.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"1368063095-22861-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/3] fast-export: improve argument parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-09T22:17:47Z","receivedAt":"2013-05-09T22:17:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We don't want to pass arguments specific to fast-export to\n> setup_revisions.\n\nInteresting. What bad things happen with the current order?\n\nDoes \"fast-export --export-marks=foo\" causes setup_revisions() to\nmistakenly eat --export-marks=foo and barf?\n\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/fast-export.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> index d60d675..6e46057 100644\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> \trevs.topo_order = 1;\n> \trevs.show_source = 1;\n> \trevs.rewrite_parents = 1;\n> +\targc = parse_options(argc, argv, prefix, options, fast_export_usage,\n> +\t\t\tPARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n> \targc = setup_revisions(argc, argv, &revs, NULL);\n> -\targc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);\n> \tif (argc > 1)\n> \t\tusage_with_options (fast_export_usage, options);\n\nThere is a SP between the function name and its arguments here ;-)\n"},{"id":"216828","messageId":"7vvc6r7pii.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"1368063095-22861-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-09T22:32:53Z","receivedAt":"2013-05-09T22:32:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> +test_expect_success 'use refspec' '\n> +\tgit fast-export --refspec refs/heads/master:refs/heads/foobar master | \\\n> +\t\tgrep \"^commit \" | sort | uniq > actual &&\n\nYou do not need backslash after the pipe symbol at the end of line;\nthe shell knows you haven't finished speaking at that point.\n\nThe usual \"pipe hides the error status of upstream commands\" applies\nhere. The command may die after writing enough to fill the pipe\nbuffer and showing the lines that begin with \"commit\".\n\nAlso it makes it harder to debug the test when something goes wrong.\n\nBy the way, don't you find that something does not feel quite right\nwith this command line?\n\n git fast-export --refspec=refs/heads/master:refs/heads/foobar master \n\nWhy do we even have to say \"master\" at the end, when the other\noption makes it clear that we are shipping \"master\" out?\n\nWithout thinking ramifications through, my gut feeling is that it\nwould feel more natural if we took:\n\n git fast-export master:foobar\n\nto mean the same thing (which is what happens to the users of \"git\npush\"). Is there a case where you have some ref on the left hand\nside of the --refspec but you do not push out the history leading to\nit?\n\nWith such an update, this part of the test would of course look\nlike:\n\n\tgit fast-export master:foobar >actual.dump &&\n grep \"^commit \" actual.dump |\n sort -u >actual &&\n ...\n\nand we do not need a new option. Just a new extension to express\nwhat gets pushed out.\n\nBut of course I may be missing some cases why there need to be a\nseparate option.\n"},{"id":"216827","messageId":"CAMP44s2yoBFZU+LzHWT30ijbQ2rknmV1ZoB9qDDYvvme2chB7g@mail.gmail.com","threadId":"33758","inReplyTo":"7vvc6r7pii.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T22:53:52Z","receivedAt":"2013-05-09T22:53:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 5:32 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> +test_expect_success 'use refspec' '\n>> + git fast-export --refspec refs/heads/master:refs/heads/foobar master | \\\n>> + grep \"^commit \" | sort | uniq > actual &&\n>\n> You do not need backslash after the pipe symbol at the end of line;\n> the shell knows you haven't finished speaking at that point.\n>\n> The usual \"pipe hides the error status of upstream commands\" applies\n> here. The command may die after writing enough to fill the pipe\n> buffer and showing the lines that begin with \"commit\".\n>\n> Also it makes it harder to debug the test when something goes wrong.\n>\n> By the way, don't you find that something does not feel quite right\n> with this command line?\n>\n> git fast-export --refspec=refs/heads/master:refs/heads/foobar master\n>\n> Why do we even have to say \"master\" at the end, when the other\n> option makes it clear that we are shipping \"master\" out?\n>\n> Without thinking ramifications through, my gut feeling is that it\n> would feel more natural if we took:\n>\n> git fast-export master:foobar\n>\n> to mean the same thing (which is what happens to the users of \"git\n> push\"). Is there a case where you have some ref on the left hand\n> side of the --refspec but you do not push out the history leading to\n> it?\n>\n> With such an update, this part of the test would of course look\n> like:\n>\n> git fast-export master:foobar >actual.dump &&\n> grep \"^commit \" actual.dump |\n> sort -u >actual &&\n> ...\n>\n> and we do not need a new option. Just a new extension to express\n> what gets pushed out.\n>\n> But of course I may be missing some cases why there need to be a\n> separate option.\n\nOf course, but how do you implement that? That's mixing refspecs and\nrevlist arguments, which AFAIK don't mix:\n\n% git fast-export ^next:new-next master:new-master --not\nrefs/tags/*:refs/tags/backup/*\n\n-- \nFelipe Contreras\n"},{"id":"216830","messageId":"CAMP44s2RYPENwsPAcoaKjmHxkuVA52vvRrQ_stmT_qY-Pmt6HA@mail.gmail.com","threadId":"33758","inReplyTo":"7vzjw37q7o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] fast-export: improve argument parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T23:02:21Z","receivedAt":"2013-05-09T23:02:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> We don't want to pass arguments specific to fast-export to\n>> setup_revisions.\n>\n> Interesting. What bad things happen with the current order?\n>\n> Does \"fast-export --export-marks=foo\" causes setup_revisions() to\n> mistakenly eat --export-marks=foo and barf?\n\nNo, apparently it skips them. But try 'git fast-export --export-marks\nmarks HEAD'.\n\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> builtin/fast-export.c | 3 ++-\n>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n>> index d60d675..6e46057 100644\n>> --- a/builtin/fast-export.c\n>> +++ b/builtin/fast-export.c\n>> @@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n>> revs.topo_order = 1;\n>> revs.show_source = 1;\n>> revs.rewrite_parents = 1;\n>> + argc = parse_options(argc, argv, prefix, options, fast_export_usage,\n>> + PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);\n>> argc = setup_revisions(argc, argv, &revs, NULL);\n>> - argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);\n>> if (argc > 1)\n>> usage_with_options (fast_export_usage, options);\n>\n> There is a SP between the function name and its arguments here ;-)\n\nYeah, and I already did my part: I sent a patch to fix this style. Not\nthat it has anything to do with this patch.\n\n-- \nFelipe Contreras\n"},{"id":"216837","messageId":"7v38tv7n5y.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"CAMP44s2yoBFZU+LzHWT30ijbQ2rknmV1ZoB9qDDYvvme2chB7g@mail.gmail.com","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-09T23:23:37Z","receivedAt":"2013-05-09T23:23:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Of course, but how do you implement that? That's mixing refspecs and\n> revlist arguments, which AFAIK don't mix:\n\nSimple. You treat everything as refspecs and form revision ranges\nout of them. Note that that is exactly the reason why \"git push\"\ncan take \"master\" as a short-hand for \"master:master\" [*1*].\n\n> % git fast-export ^next:new-next master:new-master --not\n> refs/tags/*:refs/tags/backup/*\n\nI thought you stopped mentioning the bottom of the range\n(e.g. ^next) in the output from export stream at around 49266e8a11cf\n(fast-export: don't handle uninteresting refs, 2012-11-28).\n\nWhat does ^next:new-next (or mapping after \"--not\" in general) even\nmean? They would not make sense, would they?\n\nSo I would imagine you would be spelling that as:\n\n git fast-export master:new-master --not next refs/tags/*\n\nor something, no?\n\n\n[Footnote]\n\n*1* Of course, unlike \"git push\", but similar to \"git bundle\",\n export does not know who the \"receiving side\" is and what they\n have, so in addition to the positives, you would need to tell\n the command where the bottoms of the range you are exporting\n are, so there needs to be some difference between the way \"git\n push\" and export/bundle specify the ranges.\n\n But that does not affect what should happen on the positive end\n of the ranges, which both \"git push\" and export/bundle need to\n specify anyway.\n"},{"id":"216838","messageId":"7vy5bn68eu.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"CAMP44s2RYPENwsPAcoaKjmHxkuVA52vvRrQ_stmT_qY-Pmt6HA@mail.gmail.com","subject":"Re: [PATCH 1/3] fast-export: improve argument parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-09T23:27:37Z","receivedAt":"2013-05-09T23:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> We don't want to pass arguments specific to fast-export to\n>>> setup_revisions.\n>>\n>> Interesting. What bad things happen with the current order?\n>>\n>> Does \"fast-export --export-marks=foo\" causes setup_revisions() to\n>> mistakenly eat --export-marks=foo and barf?\n>\n> No, apparently it skips them. But try 'git fast-export --export-marks\n> marks HEAD'.\n\nThat is the kind of thing that needs to be said, not in the\ndiscussion but in the history, either in the log or in a new test,\nor both.\n"},{"id":"216839","messageId":"CAMP44s1d_9iT7nqtu3gTy=kX-Kapnf0AUZJ-Z-MpLkBuWxBgSw@mail.gmail.com","threadId":"33758","inReplyTo":"7v38tv7n5y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T23:32:16Z","receivedAt":"2013-05-09T23:32:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Of course, but how do you implement that? That's mixing refspecs and\n>> revlist arguments, which AFAIK don't mix:\n>\n> Simple. You treat everything as refspecs and form revision ranges\n> out of them. Note that that is exactly the reason why \"git push\"\n> can take \"master\" as a short-hand for \"master:master\" [*1*].\n\nAnd how do you implement that?\n\n>> % git fast-export ^next:new-next master:new-master --not\n>> refs/tags/*:refs/tags/backup/*\n>\n> I thought you stopped mentioning the bottom of the range\n> (e.g. ^next) in the output from export stream at around 49266e8a11cf\n> (fast-export: don't handle uninteresting refs, 2012-11-28).\n\nThat doesn't prevent the rev-list parsing from working.\n\n> What does ^next:new-next (or mapping after \"--not\" in general) even\n> mean? They would not make sense, would they?\n\nThey don't, which is precisely my point.\n\n> So I would imagine you would be spelling that as:\n>\n> git fast-export master:new-master --not next refs/tags/*\n>\n> or something, no?\n\nrev-list doesn't accept 'refs/tags/*'.\n\n-- \nFelipe Contreras\n"},{"id":"216840","messageId":"CAMP44s1GAeu5v5xqLu8BgisprmWN=C8zoFYNidoyBPXmu-cvJQ@mail.gmail.com","threadId":"33758","inReplyTo":"7vy5bn68eu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] fast-export: improve argument parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-09T23:33:15Z","receivedAt":"2013-05-09T23:33:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 6:27 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> We don't want to pass arguments specific to fast-export to\n>>>> setup_revisions.\n>>>\n>>> Interesting. What bad things happen with the current order?\n>>>\n>>> Does \"fast-export --export-marks=foo\" causes setup_revisions() to\n>>> mistakenly eat --export-marks=foo and barf?\n>>\n>> No, apparently it skips them. But try 'git fast-export --export-marks\n>> marks HEAD'.\n>\n> That is the kind of thing that needs to be said, not in the\n> discussion but in the history, either in the log or in a new test,\n> or both.\n\nIf only I had known that when I wrote the commit message.\n\n-- \nFelipe Contreras\n"},{"id":"216841","messageId":"7vobcj65wp.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"CAMP44s1d_9iT7nqtu3gTy=kX-Kapnf0AUZJ-Z-MpLkBuWxBgSw@mail.gmail.com","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T00:21:42Z","receivedAt":"2013-05-10T00:21:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano wrote:\n>> Simple. You treat everything as refspecs and form revision ranges\n>> out of them. Note that that is exactly the reason why \"git push\"\n>> can take \"master\" as a short-hand for \"master:master\" [*1*].\n> ...\n>> So I would imagine you would be spelling that as:\n>>\n>> git fast-export master:new-master --not next refs/tags/*\n>>\n>> or something, no?\n>\n> rev-list doesn't accept 'refs/tags/*'.\n\nI think you misunderstood. That is meant to illustrate what your\nend users feed \"fast-export\". \"fast-export\" in turn expands that\ninto a revision range, which needs to happen anyway when it strips\n:new-master from the positive end of the range to make the range\ninto\n\n\tmaster ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...\n"},{"id":"216842","messageId":"CAMP44s101dmokgjwYxhtRsWWjhSZKxWFgnbkeDFHw+svDmTy9w@mail.gmail.com","threadId":"33758","inReplyTo":"7vobcj65wp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-10T00:44:24Z","receivedAt":"2013-05-10T00:44:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 7:21 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano wrote:\n>>> Simple. You treat everything as refspecs and form revision ranges\n>>> out of them. Note that that is exactly the reason why \"git push\"\n>>> can take \"master\" as a short-hand for \"master:master\" [*1*].\n>> ...\n>>> So I would imagine you would be spelling that as:\n>>>\n>>> git fast-export master:new-master --not next refs/tags/*\n>>>\n>>> or something, no?\n>>\n>> rev-list doesn't accept 'refs/tags/*'.\n>\n> I think you misunderstood. That is meant to illustrate what your\n> end users feed \"fast-export\". \"fast-export\" in turn expands that\n> into a revision range, which needs to happen anyway when it strips\n> :new-master from the positive end of the range to make the range\n> into\n>\n> master ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...\n\n'git fast-export' accepts rev-list arguments, to make it do something\nelse would not only break existing users, but would require massive\nwork. I'm not interested in doing that.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216843","messageId":"7vk3n763is.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"7vobcj65wp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T01:13:15Z","receivedAt":"2013-05-10T01:13:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Felipe Contreras writes:\n>\n>> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano wrote:\n>>> Simple. You treat everything as refspecs and form revision ranges\n>>> out of them. Note that that is exactly the reason why \"git push\"\n>>> can take \"master\" as a short-hand for \"master:master\" [*1*].\n>> ...\n>>> So I would imagine you would be spelling that as:\n>>>\n>>> git fast-export master:new-master --not next refs/tags/*\n>>>\n>>> or something, no?\n>>\n>> rev-list doesn't accept 'refs/tags/*'.\n>\n> I think you misunderstood. That is meant to illustrate what your\n> end users feed \"fast-export\". \"fast-export\" in turn expands that\n> into a revision range, which needs to happen anyway when it strips\n> :new-master from the positive end of the range to make the range\n> into\n>\n> \tmaster ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...\n\nA few corrections and a related random thought.\n\n * The original --not \"refs/tags/*:refs/tags/backup/*\" fooled me. As\n my primary point was that an earlier change to fast-export made\n the uninteresting bottom not appear as reset in the output\n stream, I omitted everything after colon without much thought.\n\n If the current fast-export does not let you say \"refs/tags/*\" to\n export all the tags, we do not have to extend it to allow it to\n do so.\n\n * On the other hand, \"git log 'fc/*'\" might be a handy thing for\n any command that wants to have multiple starting points for\n revision traversal, so in principle I would not mind such an\n enhancement to rev-list machinery.\n\n But that does not have to be part of this series.\n\n * I alluded to the similarity between fast-export and bundle,\n meaning to hint that \"git bundle\" might also benefit from being\n able to say \"record these refs under different name\", but after\n thinking about it a bit more, I think it is unnecessary.\n\n The user of the bundle's output, either fetch or clone, can use\n the usual refspec mapping to use the data in any way it wants.\n If the bundle says refs/heads/master is at one commit, the _user_\n of that bundle can decide to map it to refs/remotes/origin/master\n (or refs/heads/origin, for that matter).\n\n It is a very powerful concept that we can generate data once,\n cast it in stone, and delay the decision on _how_ it is used\n until the last minute, much better than mapping at export time.\n So bundle does not need a similar refspec mechanism to map what\n it exports. I haven't thought things through to see if the same\n logic applies to fast-export output (if so, it would mean it is\n better to allow the consumer of the stream take the refspec\n parameter and map the tips it finds in its input), though.\n"},{"id":"216847","messageId":"7v4neb5tj5.fsf@alter.siamese.dyndns.org","threadId":"33758","inReplyTo":"CAMP44s1GAeu5v5xqLu8BgisprmWN=C8zoFYNidoyBPXmu-cvJQ@mail.gmail.com","subject":"Re: [PATCH 1/3] fast-export: improve argument parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T04:49:02Z","receivedAt":"2013-05-10T04:49:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Thu, May 9, 2013 at 6:27 PM, Junio C Hamano wrote:\n> ...\n>> That is the kind of thing that needs to be said, not in the\n>> discussion but in the history, either in the log or in a new test,\n>> or both.\n>\n> If only I had known that when I wrote the commit message.\n\nThis is exactly what we review patches on the list for. There is no\nneed to feel bad. Nobody expects anybody to be perfect on the first\ntry.\n\nOther people who may not know what the thought process of the author\nwas when the patch was written (or those who may not be as familiar\nwith the area in question as the author of the patch) can sometimes\nspot a gap in the thought that is recorded in the patch.\n\nRegarding the text of the patch in question, I am of two minds. It\nmay solve the \"--import-marks foo\" to swap the orders, but I suspect\nthat may merely be shifting the problem and break rev-list options\n(e.g. \"--since 3.days\") for the same reason.\n\nIn the longer term, setup_revisions() may need to allow its callers\nto tell it what to do when it sees a parameter that it does not\nunderstand.\n\nThere already is a similar cascading mechanism in place from\nrevision argument parsing to diff argument parsing (look for the\nplace where diff_opt_parse() is called in revision.c). It may be\njust the matter of adding a \"struct option *\" to rev_info and\ncalling parse_options_step() after diff_opt_parse() says \"I dunno\nabout this option\".\n\nFor the mechanism to be more flexible, we may want to give the\ncustom option table the caller of setup_revisions() gives us\nprecedence over revision/diff options, though.\n"},{"id":"216852","messageId":"518C96DD.50304@viscovery.net","threadId":"33758","inReplyTo":"7vk3n763is.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-05-10T06:42:37Z","receivedAt":"2013-05-10T06:42:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/10/2013 3:13, schrieb Junio C Hamano:\n> * On the other hand, \"git log 'fc/*'\" might be a handy thing for\n> any command that wants to have multiple starting points for\n> revision traversal, so in principle I would not mind such an\n> enhancement to rev-list machinery.\n\nCurrently, we spell this as \"git log --branches=fc\" or\n\"git log --branches='*export*'\". But I do not mean to say that\n\"git log fc/\\*\" would be a bad thing to have.\n\n-- Hannes\n"},{"id":"217544","messageId":"CAMP44s1G=SFJ-5J8UR=mnoO_JXmuY+CC1FaaY7yJNQGuCi0Naw@mail.gmail.com","threadId":"33758","inReplyTo":"7vk3n763is.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-16T09:15:03Z","receivedAt":"2013-05-16T09:15:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 9, 2013 at 8:13 PM, Junio C Hamano wrote:\n\n> It is a very powerful concept that we can generate data once,\n> cast it in stone, and delay the decision on _how_ it is used\n> until the last minute, much better than mapping at export time.\n> So bundle does not need a similar refspec mechanism to map what\n> it exports. I haven't thought things through to see if the same\n> logic applies to fast-export output (if so, it would mean it is\n> better to allow the consumer of the stream take the refspec\n> parameter and map the tips it finds in its input), though.\n\nIt's possible to delay the renaming of refspecs to leave that duty to\nthe remote-helper, however, each and every remote-helper would need\nadvertise a new capability, and then implement the renaming. I think\nit makes much more sense to implement something that just works for\nfree in all remote-helpers.\n\nBut I wasn't even that interested in this patch, I implemented it\nbecause while I was helping the emacs folks I had to explain to them\nthat old:new didn't work, and I thought it would be trivial to make it\nwork.\n\nI'm dropping this series. Somebody should at least make it so a proper\nerror is displayed so that the users are not left in the dark\nwondering what the hell is going on.\n\n-- \nFelipe Contreras\n"},{"id":"217545","messageId":"CAMP44s1eNPQggd7UJ0XZ+JX0ywa6rDiN-HQQLx+bCp1mrmpEgA@mail.gmail.com","threadId":"33758","inReplyTo":"1368063095-22861-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/3] Support for old:new remote-helper push","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-16T09:15:59Z","receivedAt":"2013-05-16T09:15:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 8, 2013 at 8:31 PM, Felipe Contreras\n wrote:\n\n> In order to support pushing old:new refspecs in remote-helpers, the best way to\n> do what is to add an option to fast-export to rename the refs in the stream, so\n> here it is:\n\nI'm not going to work on this series any more.\n\n-- \nFelipe Contreras\n"}]}