{"thread":{"id":"35254","subject":"[PATCH v5 10/10] transport-helper: add support to delete branches","startedAt":"2013-10-31T09:36:27Z","lastAt":"2013-11-11T18:15:53Z","messageCount":45,"participants":["Felipe Contreras","Max Horn","Junio C Hamano","Eric Sunshine","Jeff King","Richard Hansen"],"isPatch":true,"patchVersion":5,"patchTotal":10},"messages":[{"id":"229896","messageId":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":null,"subject":"[PATCH v5 10/10] transport-helper: add support to delete branches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:27Z","receivedAt":"2013-10-31T09:36:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"For remote-helpers that use 'export' to push.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 8 ++++++++\n transport-helper.c | 11 ++++++-----\n 2 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 454337e..c667965 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -94,6 +94,14 @@ test_expect_success 'push new branch with old:new refspec' '\n \tcompare_refs local HEAD server refs/heads/new-refspec\n '\n \n+test_expect_success 'push delete branch' '\n+\t(cd local &&\n+\t git push origin :new-name\n+\t) &&\n+\ttest_must_fail git --git-dir=\"server/.git\" \\\n+\t rev-parse --verify refs/heads/new-name\n+'\n+\n test_expect_success 'forced push' '\n \t(cd local &&\n \tgit checkout -b force-test &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 84ff578..ef91882 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -875,9 +875,6 @@ 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\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n \t\tif (private && !get_sha1(private, sha1)) {\n \t\t\tstrbuf_addf(&buf, \"^%s\", private);\n@@ -889,12 +886,16 @@ static int push_refs_with_export(struct transport *transport,\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\tif (!ref->deletion)\n+\t\t\t\t\tstrbuf_addf(&buf, \"%s:%s\", ref->peer_ref->name, ref->name);\n+\t\t\t\telse\n+\t\t\t\t\tstrbuf_addf(&buf, \":%s\", 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\tif (!ref->deletion)\n+\t\t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n \t\t}\n \t}\n \n-- \n1.8.4.2+fc1\n"},{"id":"229899","messageId":"1383212197-14259-2-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 00/10] transport-helper: updates","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:28Z","receivedAt":"2013-10-31T09:36:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere are the patches that allow transport helpers to be completely transparent;\nrenaming branches, deleting them, custom refspecs, --force, --dry-run,\nreporting forced update, everything works.\n\nSome of these were were sent before and rejected without a reason, but here\nthey are again in case anybody is interested.\n\nDiff from v3:\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex f1f4ca9..e75699c 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option' capability.\n 'option check-connectivity' \\{'true'|'false'\\}::\n \tRequest the helper to check connectivity of a clone.\n \n+'option force' \\{'true'|'false'\\}::\n+\tRequest the helper to perform a force update. Defaults to\n+\t'false'.\n+\n SEE ALSO\n --------\n linkgit:git-remote[1]\ndiff --git a/git-remote-testgit.sh b/git-remote-testgit.sh\nindex 6d2f282..716aa4c 100755\n--- a/git-remote-testgit.sh\n+++ b/git-remote-testgit.sh\n@@ -39,6 +39,7 @@ do\n \t\tfi\n \t\ttest -n \"$GIT_REMOTE_TESTGIT_SIGNED_TAGS\" && echo \"signed-tags\"\n \t\ttest -n \"$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE\" && echo \"no-private-update\"\n+\t\techo 'option'\n \t\techo\n \t\t;;\n \tlist)\n@@ -93,6 +94,7 @@ do\n \t\tbefore=$(git for-each-ref --format=' %(refname) %(objectname) ')\n \n \t\tgit fast-import \\\n+\t\t\t${force:+--force} \\\n \t\t\t${testgitmarks:+\"--import-marks=$testgitmarks\"} \\\n \t\t\t${testgitmarks:+\"--export-marks=$testgitmarks\"} \\\n \t\t\t--quiet\n@@ -115,6 +117,20 @@ do\n \n \t\techo\n \t\t;;\n+\toption\\ *)\n+\t\tread cmd opt val <<-EOF\n+\t\t$line\n+\t\tEOF\n+\t\tcase $opt in\n+\t\tforce)\n+\t\t\ttest $val = \"true\" && force=\"true\" || force=\n+\t\t\techo \"ok\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\techo \"unsupported\"\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n \t'')\n \t\texit\n \t\t;;\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex be543c0..c667965 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -102,6 +102,19 @@ test_expect_success 'push delete branch' '\n \t rev-parse --verify refs/heads/new-name\n '\n \n+test_expect_success 'forced push' '\n+\t(cd local &&\n+\tgit checkout -b force-test &&\n+\techo content >> file &&\n+\tgit commit -a -m eight &&\n+\tgit push origin force-test &&\n+\techo content >> file &&\n+\tgit commit -a --amend -m eight-modified &&\n+\tgit push --force origin force-test\n+\t) &&\n+\tcompare_refs local refs/heads/force-test server refs/heads/force-test\n+'\n+\n test_expect_success 'cloning without refspec' '\n \tGIT_REMOTE_TESTGIT_REFSPEC=\"\" \\\n \tgit clone \"testgit::${PWD}/server\" local2 2>error &&\n\nFelipe Contreras (10):\n transport-helper: fix extra lines\n transport-helper: don't update refs in dry-run\n transport-helper: add 'force' to 'export' helpers\n transport-helper: check for 'forced update' message\n fast-export: improve argument parsing\n fast-export: add new --refspec option\n transport-helper: add support for old:new refspec\n fast-import: add support to delete refs\n fast-export: add support to delete refs\n transport-helper: add support to delete branches\n\n Documentation/git-fast-export.txt | 4 ++++\n Documentation/git-fast-import.txt | 3 +++\n Documentation/gitremote-helpers.txt | 4 ++++\n builtin/fast-export.c | 47 ++++++++++++++++++++++++++++++++++++-\n fast-import.c | 13 +++++++---\n git-remote-testgit.sh | 16 +++++++++++++\n t/t5801-remote-helpers.sh | 23 +++++++++++++++++-\n t/t9300-fast-import.sh | 18 ++++++++++++++\n t/t9350-fast-export.sh | 18 ++++++++++++++\n transport-helper.c | 47 +++++++++++++++++++++++++------------\n 10 files changed, 173 insertions(+), 20 deletions(-)\n\n-- \n1.8.4.2+fc1\n"},{"id":"229897","messageId":"1383212197-14259-3-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 01/10] transport-helper: fix extra lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:29Z","receivedAt":"2013-10-31T09:36:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Commit 9c51558 (transport-helper: trivial code shuffle) moved these\nlines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec')\nhad a wrong merge conflict and readded them.\n\nReported-by: Richard Hansen \nSigned-off-by: Felipe Contreras \n---\n transport-helper.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b32e2d6..985eeea 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -874,9 +874,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-\n \t\tif (ref->peer_ref) {\n \t\t\tif (strcmp(ref->peer_ref->name, ref->name))\n \t\t\t\tdie(\"remote-helpers do not support old:new syntax\");\n-- \n1.8.4.2+fc1\n"},{"id":"229900","messageId":"1383212197-14259-4-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 08/10] fast-import: add support to delete refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:30Z","receivedAt":"2013-10-31T09:36:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n Documentation/git-fast-import.txt | 3 +++\n fast-import.c | 13 ++++++++++---\n t/t9300-fast-import.sh | 18 ++++++++++++++++++\n 3 files changed, 31 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 73f9806..c49ede4 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.\n * Any valid Git SHA-1 expression that resolves to a commit. See\n ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.\n \n+* The special null SHA-1 (40 zeros) specifices that the branch is to be\n+ removed.\n+\n The special case of restarting an incremental import from the\n current branch value should be written as:\n ----\ndiff --git a/fast-import.c b/fast-import.c\nindex f4d9969..fdce0b7 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -248,6 +248,7 @@ struct branch {\n \tuintmax_t last_commit;\n \tuintmax_t num_notes;\n \tunsigned active : 1;\n+\tunsigned delete : 1;\n \tunsigned pack_id : PACK_ID_BITS;\n \tunsigned char sha1[20];\n };\n@@ -1690,10 +1691,13 @@ static int update_branch(struct branch *b)\n \tstruct ref_lock *lock;\n \tunsigned char old_sha1[20];\n \n-\tif (is_null_sha1(b->sha1))\n-\t\treturn 0;\n \tif (read_ref(b->name, old_sha1))\n \t\thashclr(old_sha1);\n+\tif (is_null_sha1(b->sha1)) {\n+\t\tif (b->delete)\n+\t\t\tdelete_ref(b->name, old_sha1, 0);\n+\t\treturn 0;\n+\t}\n \tlock = lock_any_ref_for_update(b->name, old_sha1, 0, NULL);\n \tif (!lock)\n \t\treturn error(\"Unable to lock %s\", b->name);\n@@ -2620,8 +2624,11 @@ static int parse_from(struct branch *b)\n \t\t\tfree(buf);\n \t\t} else\n \t\t\tparse_from_existing(b);\n-\t} else if (!get_sha1(from, b->sha1))\n+\t} else if (!get_sha1(from, b->sha1)) {\n \t\tparse_from_existing(b);\n+\t\tif (is_null_sha1(b->sha1))\n+\t\t\tb->delete = 1;\n+\t}\n \telse\n \t\tdie(\"Invalid ref name or SHA1 expression: %s\", from);\n \ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 88fc407..f453388 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2999,4 +2999,22 @@ test_expect_success 'T: ls root tree' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'T: delete branch' '\n+\tgit branch to-delete &&\n+\tgit fast-import <<-EOF &&\n+\treset refs/heads/to-delete\n+\tfrom 0000000000000000000000000000000000000000\n+\tEOF\n+\ttest_must_fail git rev-parse --verify refs/heads/to-delete\n+'\n+\n+test_expect_success 'T: empty reset doesnt delete branch' '\n+\tgit branch not-to-delete &&\n+\tgit fast-import <<-EOF &&\n+\treset refs/heads/not-to-delete\n+\tEOF\n+\tgit show-ref &&\n+\tgit rev-parse --verify refs/heads/not-to-delete\n+'\n+\n test_done\n-- \n1.8.4.2+fc1\n"},{"id":"229898","messageId":"1383212197-14259-5-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 05/10] fast-export: improve argument parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:31Z","receivedAt":"2013-10-31T09:36:31Z","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 78250ea..eea5b8c 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -701,8 +701,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.4.2+fc1\n"},{"id":"229906","messageId":"1383212197-14259-6-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:32Z","receivedAt":"2013-10-31T09:36:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that we can convert 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 85f1f30..221506b 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 \tA list of arguments, acceptable to 'git rev-parse' and\n \t'git rev-list', that specifies the specific objects and references\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex eea5b8c..b6f623e 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@@ -31,6 +32,8 @@ 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+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@@ -525,6 +528,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\tcommit = get_commit(e, full_name);\n \t\tif (!commit) {\n \t\t\twarning(\"%s: Unexpected object of type %s, skipping.\",\n@@ -668,6 +680,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 = STRING_LIST_INIT_NODUP;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"progress\", &progress,\n \t\t\t N_(\"show progress after objects\")),\n@@ -688,6 +701,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(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@@ -707,6 +722,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@@ -741,5 +769,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 34c2d8f..dc6666f 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -504,4 +504,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.4.2+fc1\n"},{"id":"229905","messageId":"1383212197-14259-7-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:33Z","receivedAt":"2013-10-31T09:36:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise they cannot know when to force the push or not (other than\nhacks).\n\nTests-by: Richard Hansen \nDocumentation-by: Richard Hansen \nSigned-off-by: Felipe Contreras \n---\n Documentation/gitremote-helpers.txt | 4 ++++\n git-remote-testgit.sh | 16 ++++++++++++++++\n t/t5801-remote-helpers.sh | 13 +++++++++++++\n transport-helper.c | 5 +++++\n 4 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex f1f4ca9..e75699c 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option' capability.\n 'option check-connectivity' \\{'true'|'false'\\}::\n \tRequest the helper to check connectivity of a clone.\n \n+'option force' \\{'true'|'false'\\}::\n+\tRequest the helper to perform a force update. Defaults to\n+\t'false'.\n+\n SEE ALSO\n --------\n linkgit:git-remote[1]\ndiff --git a/git-remote-testgit.sh b/git-remote-testgit.sh\nindex 6d2f282..716aa4c 100755\n--- a/git-remote-testgit.sh\n+++ b/git-remote-testgit.sh\n@@ -39,6 +39,7 @@ do\n \t\tfi\n \t\ttest -n \"$GIT_REMOTE_TESTGIT_SIGNED_TAGS\" && echo \"signed-tags\"\n \t\ttest -n \"$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE\" && echo \"no-private-update\"\n+\t\techo 'option'\n \t\techo\n \t\t;;\n \tlist)\n@@ -93,6 +94,7 @@ do\n \t\tbefore=$(git for-each-ref --format=' %(refname) %(objectname) ')\n \n \t\tgit fast-import \\\n+\t\t\t${force:+--force} \\\n \t\t\t${testgitmarks:+\"--import-marks=$testgitmarks\"} \\\n \t\t\t${testgitmarks:+\"--export-marks=$testgitmarks\"} \\\n \t\t\t--quiet\n@@ -115,6 +117,20 @@ do\n \n \t\techo\n \t\t;;\n+\toption\\ *)\n+\t\tread cmd opt val <<-EOF\n+\t\t$line\n+\t\tEOF\n+\t\tcase $opt in\n+\t\tforce)\n+\t\t\ttest $val = \"true\" && force=\"true\" || force=\n+\t\t\techo \"ok\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\techo \"unsupported\"\n+\t\t\t;;\n+\t\tesac\n+\t\t;;\n \t'')\n \t\texit\n \t\t;;\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 613f69a..c33cc25 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -94,6 +94,19 @@ test_expect_failure 'push new branch with old:new refspec' '\n \tcompare_refs local HEAD server refs/heads/new-refspec\n '\n \n+test_expect_success 'forced push' '\n+\t(cd local &&\n+\tgit checkout -b force-test &&\n+\techo content >> file &&\n+\tgit commit -a -m eight &&\n+\tgit push origin force-test &&\n+\techo content >> file &&\n+\tgit commit -a --amend -m eight-modified &&\n+\tgit push --force origin force-test\n+\t) &&\n+\tcompare_refs local refs/heads/force-test server refs/heads/force-test\n+'\n+\n test_expect_success 'cloning without refspec' '\n \tGIT_REMOTE_TESTGIT_REFSPEC=\"\" \\\n \tgit clone \"testgit::${PWD}/server\" local2 2>error &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex d05fc7c..ed238e5 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\tdie(\"helper %s does not support dry-run\", data->name);\n \t}\n \n+\tif (flags & TRANSPORT_PUSH_FORCE) {\n+\t\tif (set_helper_option(transport, \"force\", \"true\") != 0)\n+\t\t\tdie(\"helper %s does not support 'force'\", data->name);\n+\t}\n+\n \thelper = get_helper(transport);\n \n \twrite_constant(helper->in, \"export\\n\");\n-- \n1.8.4.2+fc1\n"},{"id":"229901","messageId":"1383212197-14259-8-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:34Z","receivedAt":"2013-10-31T09:36:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 14 ++++++++++++++\n t/t9350-fast-export.sh | 11 +++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex b6f623e..8ed41b4 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -673,6 +673,19 @@ static void import_marks(char *input_file)\n \tfclose(f);\n }\n \n+static void handle_deletes(void)\n+{\n+\tint i;\n+\tfor (i = 0; i < refspecs_nr; i++) {\n+\t\tstruct refspec *refspec = &refspecs[i];\n+\t\tif (*refspec->src)\n+\t\t\tcontinue;\n+\n+\t\tprintf(\"reset %s\\nfrom %s\\n\\n\",\n+\t\t\t\trefspec->dst, sha1_to_hex(null_sha1));\n+\t}\n+}\n+\n int cmd_fast_export(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -762,6 +775,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t}\n \n \thandle_tags_and_duplicates();\n+\thandle_deletes();\n \n \tif (export_filename && lastimportid != last_idnum)\n \t\texport_marks(export_filename);\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex dc6666f..ea6c96c 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -511,4 +511,15 @@ test_expect_success 'use refspec' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'delete refspec' '\n+\tgit branch to-delete &&\n+\tgit fast-export --refspec :refs/heads/to-delete to-delete ^to-delete > actual &&\n+\tcat > expected <<-EOF &&\n+\treset refs/heads/to-delete\n+\tfrom 0000000000000000000000000000000000000000\n+\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.4.2+fc1\n"},{"id":"229903","messageId":"1383212197-14259-9-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 07/10] transport-helper: add support for old:new refspec","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:35Z","receivedAt":"2013-10-31T09:36: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 | 13 ++++++++++---\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex c33cc25..454337e 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -87,7 +87,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 5aba15c..84ff578 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -849,7 +849,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 \tif (!data->refspecs)\n@@ -887,8 +887,13 @@ static int push_refs_with_export(struct transport *transport,\n \t\tfree(private);\n \n \t\tif (ref->peer_ref) {\n-\t\t\tif (strcmp(ref->peer_ref->name, ref->name))\n-\t\t\t\tdie(\"remote-helpers do not support old:new syntax\");\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 \t}\n@@ -896,6 +901,8 @@ static int push_refs_with_export(struct transport *transport,\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, flags);\n-- \n1.8.4.2+fc1\n"},{"id":"229902","messageId":"1383212197-14259-10-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 04/10] transport-helper: check for 'forced update' message","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:36Z","receivedAt":"2013-10-31T09:36:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So the remote-helpers can tell us when a forced push was needed.\n\nSigned-off-by: Felipe Contreras \n---\n transport-helper.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex ed238e5..5aba15c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -643,7 +643,7 @@ static int push_update_ref_status(struct strbuf *buf,\n \t\t\t\t struct ref *remote_refs)\n {\n \tchar *refname, *msg;\n-\tint status;\n+\tint status, forced = 0;\n \n \tif (!prefixcmp(buf->buf, \"ok \")) {\n \t\tstatus = REF_STATUS_OK;\n@@ -701,6 +701,11 @@ static int push_update_ref_status(struct strbuf *buf,\n \t\t\tfree(msg);\n \t\t\tmsg = NULL;\n \t\t}\n+\t\telse if (!strcmp(msg, \"forced update\")) {\n+\t\t\tforced = 1;\n+\t\t\tfree(msg);\n+\t\t\tmsg = NULL;\n+\t\t}\n \t}\n \n \tif (*ref)\n@@ -722,6 +727,7 @@ static int push_update_ref_status(struct strbuf *buf,\n \t}\n \n \t(*ref)->status = status;\n+\t(*ref)->forced_update = forced;\n \t(*ref)->remote_status = msg;\n \treturn !(status == REF_STATUS_OK);\n }\n-- \n1.8.4.2+fc1\n"},{"id":"229904","messageId":"1383212197-14259-11-git-send-email-felipe.contreras@gmail.com","threadId":"35254","inReplyTo":"1383212197-14259-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 02/10] transport-helper: don't update refs in dry-run","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:36:37Z","receivedAt":"2013-10-31T09:36:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The remote helper namespace should not be updated.\n\nSigned-off-by: Felipe Contreras \n---\n transport-helper.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 985eeea..d05fc7c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -727,7 +727,8 @@ static int push_update_ref_status(struct strbuf *buf,\n }\n \n static void push_update_refs_status(struct helper_data *data,\n-\t\t\t\t struct ref *remote_refs)\n+\t\t\t\t struct ref *remote_refs,\n+\t\t\t\t int flags)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct ref *ref = remote_refs;\n@@ -741,7 +742,7 @@ static void push_update_refs_status(struct helper_data *data,\n \t\tif (push_update_ref_status(&buf, &ref, remote_refs))\n \t\t\tcontinue;\n \n-\t\tif (!data->refspecs || data->no_private_update)\n+\t\tif (flags & TRANSPORT_PUSH_DRY_RUN || !data->refspecs || data->no_private_update)\n \t\t\tcontinue;\n \n \t\t/* propagate back the update to the remote namespace */\n@@ -832,7 +833,7 @@ static int push_refs_with_push(struct transport *transport,\n \tsendline(data, &buf);\n \tstrbuf_release(&buf);\n \n-\tpush_update_refs_status(data, remote_refs);\n+\tpush_update_refs_status(data, remote_refs, flags);\n \treturn 0;\n }\n \n@@ -886,7 +887,7 @@ static int push_refs_with_export(struct transport *transport,\n \n \tif (finish_command(&exporter))\n \t\tdie(\"Error while running fast-export\");\n-\tpush_update_refs_status(data, remote_refs);\n+\tpush_update_refs_status(data, remote_refs, flags);\n \treturn 0;\n }\n \n-- \n1.8.4.2+fc1\n"},{"id":"229927","messageId":"BBA75EFD-E68A-4E7F-A886-8878650D58A8@quendi.de","threadId":"35254","inReplyTo":"1383212197-14259-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 00/10] transport-helper: updates","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T17:59:08Z","receivedAt":"2013-10-31T17:59:08Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 10:36, Felipe Contreras wrote:\n\n> Hi,\n> \n> Here are the patches that allow transport helpers to be completely transparent;\n> renaming branches, deleting them, custom refspecs, --force, --dry-run,\n> reporting forced update, everything works.\n\nI looked through this patch series in detail, and it looks fine to me. Indeed, it fixes several nuisances when using remote-helpers, and as such would be a definite win. In other words: Would be really nice to see these applied!\n\n\nCheers,\nMax\n\n"},{"id":"229932","messageId":"xmqq4n7x8g6e.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"BBA75EFD-E68A-4E7F-A886-8878650D58A8@quendi.de","subject":"Re: [PATCH v5 00/10] transport-helper: updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:10:33Z","receivedAt":"2013-10-31T18:10:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Horn writes:\n\n> On 31.10.2013, at 10:36, Felipe Contreras \n> wrote:\n>\n>> Hi,\n>> \n>> Here are the patches that allow transport helpers to be completely\n> transparent;\n>> renaming branches, deleting them, custom refspecs, --force,\n> --dry-run,\n>> reporting forced update, everything works.\n>\n> I looked through this patch series in detail, and it looks fine to\n> me. Indeed, it fixes several nuisances when using remote-helpers, and\n> as such would be a definite win.\n\nNice.\n\n> In other words: Would be really nice to see these applied!\n\nThe series is sitting on the 'pu' branch, and I think there were\nsome \"fixup\" suggestions during the review, so it may need to be\nrerolled before advancing to 'next'.\n"},{"id":"229937","messageId":"xmqqeh7171bv.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"1383212197-14259-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 01/10] transport-helper: fix extra lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:16:36Z","receivedAt":"2013-10-31T18:16:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Commit 9c51558 (transport-helper: trivial code shuffle) moved these\n> lines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec')\n> had a wrong merge conflict and readded them.\n>\n> Reported-by: Richard Hansen \n> Signed-off-by: Felipe Contreras \n> ---\n\nAhh, sorry about that. Will apply with a retitle.\n\n> transport-helper.c | 3 ---\n> 1 file changed, 3 deletions(-)\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index b32e2d6..985eeea 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -874,9 +874,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> -\n> \t\tif (ref->peer_ref) {\n> \t\t\tif (strcmp(ref->peer_ref->name, ref->name))\n> \t\t\t\tdie(\"remote-helpers do not support old:new syntax\");\n"},{"id":"229938","messageId":"xmqqa9hp714d.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"1383212197-14259-7-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:21:06Z","receivedAt":"2013-10-31T18:21:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Otherwise they cannot know when to force the push or not (other than\n> hacks).\n> ...\n> diff --git a/transport-helper.c b/transport-helper.c\n> index d05fc7c..ed238e5 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n> \t\t\tdie(\"helper %s does not support dry-run\", data->name);\n> \t}\n> \n> +\tif (flags & TRANSPORT_PUSH_FORCE) {\n> +\t\tif (set_helper_option(transport, \"force\", \"true\") != 0)\n> +\t\t\tdie(\"helper %s does not support 'force'\", data->name);\n> +\t}\n> +\n\nDoes this cause a \"git push --force $there A:B\" to fail when $there\nis a destination that goes via an existing helper does not suport\n\"force\" option?\n\nShould it fail even when the current value of B is an ancestor of A\n(i.e. when an unforced push would succeed)?\n"},{"id":"229939","messageId":"xmqq61sd70vw.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"1383212197-14259-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:26:11Z","receivedAt":"2013-10-31T18:26:11Z","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\nIt feels somewhat redundant that you have to twice say that you are\npushing your \"master\", once with --refspec and then the branch\nname. Is this the best we can do?\n\nIf the answer is \"yes\", I guess it is OK to leave the external\ninterface to fast-export of this patch as-is, but if we can do\nbetter, then we'd be better off without redundancy.\n\nThere are quite a few shell script style violations in this test, by\nthe way. You do not want the trailing \" \\\" after a pipe (the shell,\nwhen seeing a line that ends with \"|\", knows you haven't finished\ngiving it the pipeline). Also we do not put extra space between a\nredirection operator and the target filename.\n\n\n\n> +\techo \"commit refs/heads/foobar\" > expected &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_done\n"},{"id":"229940","messageId":"CAMP44s2dtGdLy=yhNva126nbzoHyeKA_AOY0xNZu-Qyq4zKf3A@mail.gmail.com","threadId":"35254","inReplyTo":"xmqq4n7x8g6e.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 00/10] transport-helper: updates","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T18:30:03Z","receivedAt":"2013-10-31T18:30:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 12:10 PM, Junio C Hamano wrote:\n> Max Horn writes:\n\n>> In other words: Would be really nice to see these applied!\n>\n> The series is sitting on the 'pu' branch, and I think there were\n> some \"fixup\" suggestions during the review, so it may need to be\n> rerolled before advancing to 'next'.\n\nThe suggestions are applied, as you can see in the diff.\n\n-- \nFelipe Contreras\n"},{"id":"229942","messageId":"xmqqwqkt5lxe.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"xmqq4n7x8g6e.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 00/10] transport-helper: updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:34:37Z","receivedAt":"2013-10-31T18:34:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Max Horn writes:\n>\n>> On 31.10.2013, at 10:36, Felipe Contreras \n>> wrote:\n>>\n>>> Hi,\n>>> \n>>> Here are the patches that allow transport helpers to be completely\n>> transparent;\n>>> renaming branches, deleting them, custom refspecs, --force,\n>> --dry-run,\n>>> reporting forced update, everything works.\n>>\n>> I looked through this patch series in detail, and it looks fine to\n>> me. Indeed, it fixes several nuisances when using remote-helpers, and\n>> as such would be a definite win.\n>\n> Nice.\n>\n>> In other words: Would be really nice to see these applied!\n>\n> The series is sitting on the 'pu' branch, and I think there were\n> some \"fixup\" suggestions during the review, so it may need to be\n> rerolled before advancing to 'next'.\n\nAh, I spoke too early; this is v5, updated from v4.\n"},{"id":"229945","messageId":"B997286F-D346-464D-9710-BA80BC17A570@quendi.de","threadId":"35254","inReplyTo":"xmqq4n7x8g6e.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 00/10] transport-helper: updates","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T18:41:15Z","receivedAt":"2013-10-31T18:41:15Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 19:10, Junio C Hamano wrote:\n\n> Max Horn writes:\n> \n>> On 31.10.2013, at 10:36, Felipe Contreras \n>> wrote:\n>> \n>>> Hi,\n>>> \n>>> Here are the patches that allow transport helpers to be completely\n>> transparent;\n>>> renaming branches, deleting them, custom refspecs, --force,\n>> --dry-run,\n>>> reporting forced update, everything works.\n>> \n>> I looked through this patch series in detail, and it looks fine to\n>> me. Indeed, it fixes several nuisances when using remote-helpers, and\n>> as such would be a definite win.\n> \n> Nice.\n> \n>> In other words: Would be really nice to see these applied!\n> \n> The series is sitting on the 'pu' branch, and I think there were\n> some \"fixup\" suggestions during the review, so it may need to be\n> rerolled before advancing to 'next'.\n\nYou are of course right. Next time I comment, I'll make sure to check whether previous review suggestions have been picked up.\n\n"},{"id":"229946","messageId":"CAMP44s246M5DaeX80tqzfjRWnbE4vKh-vp_tLW-qAQUFFPsP0Q@mail.gmail.com","threadId":"35254","inReplyTo":"xmqq61sd70vw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T18:41:31Z","receivedAt":"2013-10-31T18:41:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 12:26 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> It feels somewhat redundant that you have to twice say that you are\n> pushing your \"master\", once with --refspec and then the branch\n> name. Is this the best we can do?\n\nAs this has been discussed before and no other solution came forward, yes.\n\n-- \nFelipe Contreras\n"},{"id":"229947","messageId":"08E1EB05-D3E1-4393-A950-60AB46930BB1@quendi.de","threadId":"35254","inReplyTo":"1383212197-14259-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 08/10] fast-import: add support to delete refs","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T18:41:52Z","receivedAt":"2013-10-31T18:41:52Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 10:36, Felipe Contreras wrote:\n\n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/git-fast-import.txt | 3 +++\n> fast-import.c | 13 ++++++++++---\n> t/t9300-fast-import.sh | 18 ++++++++++++++++++\n> 3 files changed, 31 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index 73f9806..c49ede4 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.\n> * Any valid Git SHA-1 expression that resolves to a commit. See\n> ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.\n> \n> +* The special null SHA-1 (40 zeros) specifices that the branch is to be\n\ns/specifices/specifies/\n\n(this was previously pointed out by Eric Sunshine for patch 7 of v4 of this series).\n\n"},{"id":"229949","messageId":"CAMP44s39H8tASO_OvK+C+XSG-2pm09RJY6Xeg4AqDZBAU7GPaw@mail.gmail.com","threadId":"35254","inReplyTo":"xmqqa9hp714d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T18:55:06Z","receivedAt":"2013-10-31T18:55:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 12:21 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Otherwise they cannot know when to force the push or not (other than\n>> hacks).\n>> ...\n>> diff --git a/transport-helper.c b/transport-helper.c\n>> index d05fc7c..ed238e5 100644\n>> --- a/transport-helper.c\n>> +++ b/transport-helper.c\n>> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n>> die(\"helper %s does not support dry-run\", data->name);\n>> }\n>>\n>> + if (flags & TRANSPORT_PUSH_FORCE) {\n>> + if (set_helper_option(transport, \"force\", \"true\") != 0)\n>> + die(\"helper %s does not support 'force'\", data->name);\n>> + }\n>> +\n>\n> Does this cause a \"git push --force $there A:B\" to fail when $there\n> is a destination that goes via an existing helper does not suport\n> \"force\" option?\n\nYes.\n\n> Should it fail even when the current value of B is an ancestor of A\n> (i.e. when an unforced push would succeed)?\n\nIt might make sense to fail only when the push is non-fast-forward,\nbut it's not so straight-forward to implement.\n\n-- \nFelipe Contreras\n"},{"id":"229953","messageId":"xmqqfvrh5kev.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"CAMP44s39H8tASO_OvK+C+XSG-2pm09RJY6Xeg4AqDZBAU7GPaw@mail.gmail.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T19:07:20Z","receivedAt":"2013-10-31T19:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Thu, Oct 31, 2013 at 12:21 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> Otherwise they cannot know when to force the push or not (other than\n>>> hacks).\n>>> ...\n>>> diff --git a/transport-helper.c b/transport-helper.c\n>>> index d05fc7c..ed238e5 100644\n>>> --- a/transport-helper.c\n>>> +++ b/transport-helper.c\n>>> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n>>> die(\"helper %s does not support dry-run\", data->name);\n>>> }\n>>>\n>>> + if (flags & TRANSPORT_PUSH_FORCE) {\n>>> + if (set_helper_option(transport, \"force\", \"true\") != 0)\n>>> + die(\"helper %s does not support 'force'\", data->name);\n>>> + }\n>>> +\n>>\n>> Does this cause a \"git push --force $there A:B\" to fail when $there\n>> is a destination that goes via an existing helper does not suport\n>> \"force\" option?\n>\n> Yes.\n>\n>> Should it fail even when the current value of B is an ancestor of A\n>> (i.e. when an unforced push would succeed)?\n>\n> It might make sense to fail only when the push is non-fast-forward,\n> but it's not so straight-forward to implement.\n\nOK; let's see if anybody screams by cooking the series in 'next'\n(that is, when other issues people may discover in the series are\naddressed).\n\nThanks.\n"},{"id":"229956","messageId":"6F276334-DFDB-40B8-8B24-38FFB6DBED9B@quendi.de","threadId":"35254","inReplyTo":"1383212197-14259-8-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T19:29:02Z","receivedAt":"2013-10-31T19:29:02Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"Actually, I just noticed one thing that I *do* have a question about:\n\nOn 31.10.2013, at 10:36, Felipe Contreras wrote:\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/fast-export.c | 14 ++++++++++++++\n> t/t9350-fast-export.sh | 11 +++++++++++\n> 2 files changed, 25 insertions(+)\n> \n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> index b6f623e..8ed41b4 100644\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)\n> \tfclose(f);\n> }\n> \n> +static void handle_deletes(void)\n> +{\n> +\tint i;\n> +\tfor (i = 0; i < refspecs_nr; i++) {\n> +\t\tstruct refspec *refspec = &refspecs[i];\n> +\t\tif (*refspec->src)\n> +\t\t\tcontinue;\n> +\n> +\t\tprintf(\"reset %s\\nfrom %s\\n\\n\",\n> +\t\t\t\trefspec->dst, sha1_to_hex(null_sha1));\n\nIf I understand it right, this issues a \"reset\" command in the fast-import stream, resetting a ref to an all-zero SHA1. I had a look at the git-fast-import documentation, but I found that it does not explicitly cover this case. In particular, the \"reset\" command does not specify that an all-zero SHA1 should be treated as \"delete this ref\". On the other hand, the docs for \"reset\" seem to indicate that one can omit the \"from\" part, although I couldn't tell for sure what that would mean, either.\n\nI have not yet tried to dive into the code to figure out what actually happens, but I assume Felipe did that resp. tested it, and so supposedly doing this works, at least for git resp. existing transport helpers. But I wonder if other implementers of the fast-import format would handle it properly?\n\nIn any case, it seems to me that this is a gap in the documentation, isn't it? Or am I just being dense?\n\n"},{"id":"229960","messageId":"CAMP44s3vxfN5Wc+S+VjimisUgZGQMzR7Z2YkO8J4UEk7+cPy_Q@mail.gmail.com","threadId":"35254","inReplyTo":"6F276334-DFDB-40B8-8B24-38FFB6DBED9B@quendi.de","subject":"Re: [PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:41:15Z","receivedAt":"2013-10-31T19:41:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 1:29 PM, Max Horn wrote:\n> Actually, I just noticed one thing that I *do* have a question about:\n>\n> On 31.10.2013, at 10:36, Felipe Contreras wrote:\n>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> builtin/fast-export.c | 14 ++++++++++++++\n>> t/t9350-fast-export.sh | 11 +++++++++++\n>> 2 files changed, 25 insertions(+)\n>>\n>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n>> index b6f623e..8ed41b4 100644\n>> --- a/builtin/fast-export.c\n>> +++ b/builtin/fast-export.c\n>> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)\n>> fclose(f);\n>> }\n>>\n>> +static void handle_deletes(void)\n>> +{\n>> + int i;\n>> + for (i = 0; i < refspecs_nr; i++) {\n>> + struct refspec *refspec = &refspecs[i];\n>> + if (*refspec->src)\n>> + continue;\n>> +\n>> + printf(\"reset %s\\nfrom %s\\n\\n\",\n>> + refspec->dst, sha1_to_hex(null_sha1));\n>\n> If I understand it right, this issues a \"reset\" command in the fast-import stream, resetting a ref to an all-zero SHA1. I had a look at the git-fast-import documentation, but I found that it does not explicitly cover this case. In particular, the \"reset\" command does not specify that an all-zero SHA1 should be treated as \"delete this ref\".\n\nThat's what the previous patch does.\n\n> On the other hand, the docs for \"reset\" seem to indicate that one can omit the \"from\" part, although I couldn't tell for sure what that would mean, either.\n\nIt means something different.\n\n> I have not yet tried to dive into the code to figure out what actually happens, but I assume Felipe did that resp. tested it, and so supposedly doing this works, at least for git resp. existing transport helpers. But I wonder if other implementers of the fast-import format would handle it properly?\n\nTry this:\n\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -519,7 +519,9 @@ test_expect_success 'delete refspec' '\n from 0000000000000000000000000000000000000000\n\n EOF\n- test_cmp expected actual\n+ test_cmp expected actual &&\n+ git fast-import < actual &&\n+ test_must_fail git rev-parse -q --verify refs/heads/to-delete\n '\n\n test_done\n\n-- \nFelipe Contreras\n"},{"id":"229963","messageId":"78D9AAB6-09EC-4E81-A7BE-D36B9A9869D1@quendi.de","threadId":"35254","inReplyTo":"CAMP44s3vxfN5Wc+S+VjimisUgZGQMzR7Z2YkO8J4UEk7+cPy_Q@mail.gmail.com","subject":"Re: [PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T19:47:25Z","receivedAt":"2013-10-31T19:47:25Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 20:41, Felipe Contreras wrote:\n\n> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn wrote:\n>> Actually, I just noticed one thing that I *do* have a question about:\n>> \n>> On 31.10.2013, at 10:36, Felipe Contreras wrote:\n>> \n>>> Signed-off-by: Felipe Contreras \n>>> ---\n>>> builtin/fast-export.c | 14 ++++++++++++++\n>>> t/t9350-fast-export.sh | 11 +++++++++++\n>>> 2 files changed, 25 insertions(+)\n>>> \n>>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n>>> index b6f623e..8ed41b4 100644\n>>> --- a/builtin/fast-export.c\n>>> +++ b/builtin/fast-export.c\n>>> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)\n>>> fclose(f);\n>>> }\n>>> \n>>> +static void handle_deletes(void)\n>>> +{\n>>> + int i;\n>>> + for (i = 0; i < refspecs_nr; i++) {\n>>> + struct refspec *refspec = &refspecs[i];\n>>> + if (*refspec->src)\n>>> + continue;\n>>> +\n>>> + printf(\"reset %s\\nfrom %s\\n\\n\",\n>>> + refspec->dst, sha1_to_hex(null_sha1));\n>> \n>> If I understand it right, this issues a \"reset\" command in the fast-import stream, resetting a ref to an all-zero SHA1. I had a look at the git-fast-import documentation, but I found that it does not explicitly cover this case. In particular, the \"reset\" command does not specify that an all-zero SHA1 should be treated as \"delete this ref\".\n> \n> That's what the previous patch does.\n\nRight *facepalm*.\n\nBut then this should be documented in git-fast-import.txt, shouldn't it?\n\n> \n>> On the other hand, the docs for \"reset\" seem to indicate that one can omit the \"from\" part, although I couldn't tell for sure what that would mean, either.\n> \n> It means something different.\n\nYeah, I figured that -- just wanted to point out that this, too, is not very clear in the documentation and should be improved (not saying that I expect you to do that, just pointing it out).\n\n"},{"id":"229966","messageId":"CAMP44s2n-dDEcY8Ds+0z0sys1q_Y60s7KOCs5VvO0tAWG3iLpQ@mail.gmail.com","threadId":"35254","inReplyTo":"78D9AAB6-09EC-4E81-A7BE-D36B9A9869D1@quendi.de","subject":"Re: [PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:53:15Z","receivedAt":"2013-10-31T19:53:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 1:47 PM, Max Horn wrote:\n>\n> On 31.10.2013, at 20:41, Felipe Contreras wrote:\n>\n>> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn wrote:\n>>> Actually, I just noticed one thing that I *do* have a question about:\n>>>\n>>> On 31.10.2013, at 10:36, Felipe Contreras wrote:\n>>>\n>>>> Signed-off-by: Felipe Contreras \n>>>> ---\n>>>> builtin/fast-export.c | 14 ++++++++++++++\n>>>> t/t9350-fast-export.sh | 11 +++++++++++\n>>>> 2 files changed, 25 insertions(+)\n>>>>\n>>>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n>>>> index b6f623e..8ed41b4 100644\n>>>> --- a/builtin/fast-export.c\n>>>> +++ b/builtin/fast-export.c\n>>>> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)\n>>>> fclose(f);\n>>>> }\n>>>>\n>>>> +static void handle_deletes(void)\n>>>> +{\n>>>> + int i;\n>>>> + for (i = 0; i < refspecs_nr; i++) {\n>>>> + struct refspec *refspec = &refspecs[i];\n>>>> + if (*refspec->src)\n>>>> + continue;\n>>>> +\n>>>> + printf(\"reset %s\\nfrom %s\\n\\n\",\n>>>> + refspec->dst, sha1_to_hex(null_sha1));\n>>>\n>>> If I understand it right, this issues a \"reset\" command in the fast-import stream, resetting a ref to an all-zero SHA1. I had a look at the git-fast-import documentation, but I found that it does not explicitly cover this case. In particular, the \"reset\" command does not specify that an all-zero SHA1 should be treated as \"delete this ref\".\n>>\n>> That's what the previous patch does.\n>\n> Right *facepalm*.\n>\n> But then this should be documented in git-fast-import.txt, shouldn't it?\n\nIt is... in the previous patch.\n\n-- \nFelipe Contreras\n"},{"id":"229982","messageId":"DB0125AA-9F4D-497A-B0F6-9933231BCEA6@quendi.de","threadId":"35254","inReplyTo":"CAMP44s2n-dDEcY8Ds+0z0sys1q_Y60s7KOCs5VvO0tAWG3iLpQ@mail.gmail.com","subject":"Re: [PATCH v5 09/10] fast-export: add support to delete refs","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T22:38:03Z","receivedAt":"2013-10-31T22:38:03Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 20:53, Felipe Contreras wrote:\n\n> On Thu, Oct 31, 2013 at 1:47 PM, Max Horn wrote:\n>> \n>> On 31.10.2013, at 20:41, Felipe Contreras wrote:\n>> \n>>> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn wrote:\n[...]\n>>> \n>>> That's what the previous patch does.\n>> \n>> Right *facepalm*.\n>> \n>> But then this should be documented in git-fast-import.txt, shouldn't it?\n> \n> It is... in the previous patch.\n\nIndeed, there it is. So that answer to my initial question is: Yes, I am being dense :-).\n\nSo, I am (still) happy with the patch series :-)\n\n"},{"id":"230004","messageId":"xmqqppqk2n3d.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"xmqqfvrh5kev.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T14:49:58Z","receivedAt":"2013-11-01T14:49:58Z","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, Oct 31, 2013 at 12:21 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> Otherwise they cannot know when to force the push or not (other than\n>>>> hacks).\n>>>> ...\n>>>> diff --git a/transport-helper.c b/transport-helper.c\n>>>> index d05fc7c..ed238e5 100644\n>>>> --- a/transport-helper.c\n>>>> +++ b/transport-helper.c\n>>>> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n>>>> die(\"helper %s does not support dry-run\", data->name);\n>>>> }\n>>>>\n>>>> + if (flags & TRANSPORT_PUSH_FORCE) {\n>>>> + if (set_helper_option(transport, \"force\", \"true\") != 0)\n>>>> + die(\"helper %s does not support 'force'\", data->name);\n>>>> + }\n>>>> +\n>>>\n>>> Does this cause a \"git push --force $there A:B\" to fail when $there\n>>> is a destination that goes via an existing helper does not suport\n>>> \"force\" option?\n>>\n>> Yes.\n>>\n>>> Should it fail even when the current value of B is an ancestor of A\n>>> (i.e. when an unforced push would succeed)?\n>>\n>> It might make sense to fail only when the push is non-fast-forward,\n>> but it's not so straight-forward to implement.\n>\n> OK; let's see if anybody screams by cooking the series in 'next'\n> (that is, when other issues people may discover in the series are\n> addressed).\n>\n> Thanks.\n\nActually, thinking about it again, would it be a better alternative\nto issue an error message and continue, instead of dying here\n(i.e. replace the above die() with error() or even warning())?\n\nThat way, if the helper has not been updated to support 'force', but\nthe user has been happily accepting the result he gets from \"git\npush $there +master\" (perhaps because he has never pushed a non-ff\nhistory so far), he now gets a warning that essentially says that\nhis push has been working by accident, even when the particular\nff-push that showed the error message goes through successfully.\n\nIf his push through the helper that has not been updated does need\n'force', he gets the same old behaviour, possibly a broken one, out\nof the helper, and again he does get the warning.\n\nBecause updated helpers know 'force' option, such a s/die/warning/\nchange would not affect them at all.\n\nAm I missing something?\n"},{"id":"230006","messageId":"CAMP44s1wfk3zmiof6E474vNi72zTo_QFj=56G3POjopukGK-5Q@mail.gmail.com","threadId":"35254","inReplyTo":"xmqqppqk2n3d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-01T15:37:14Z","receivedAt":"2013-11-01T15:37:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 1, 2013 at 8:49 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Felipe Contreras writes:\n>>\n>>> On Thu, Oct 31, 2013 at 12:21 PM, Junio C Hamano wrote:\n>>>> Felipe Contreras writes:\n>>>>\n>>>>> Otherwise they cannot know when to force the push or not (other than\n>>>>> hacks).\n>>>>> ...\n>>>>> diff --git a/transport-helper.c b/transport-helper.c\n>>>>> index d05fc7c..ed238e5 100644\n>>>>> --- a/transport-helper.c\n>>>>> +++ b/transport-helper.c\n>>>>> @@ -854,6 +854,11 @@ static int push_refs_with_export(struct transport *transport,\n>>>>> die(\"helper %s does not support dry-run\", data->name);\n>>>>> }\n>>>>>\n>>>>> + if (flags & TRANSPORT_PUSH_FORCE) {\n>>>>> + if (set_helper_option(transport, \"force\", \"true\") != 0)\n>>>>> + die(\"helper %s does not support 'force'\", data->name);\n>>>>> + }\n>>>>> +\n>>>>\n>>>> Does this cause a \"git push --force $there A:B\" to fail when $there\n>>>> is a destination that goes via an existing helper does not suport\n>>>> \"force\" option?\n>>>\n>>> Yes.\n>>>\n>>>> Should it fail even when the current value of B is an ancestor of A\n>>>> (i.e. when an unforced push would succeed)?\n>>>\n>>> It might make sense to fail only when the push is non-fast-forward,\n>>> but it's not so straight-forward to implement.\n>>\n>> OK; let's see if anybody screams by cooking the series in 'next'\n>> (that is, when other issues people may discover in the series are\n>> addressed).\n>>\n>> Thanks.\n>\n> Actually, thinking about it again, would it be a better alternative\n> to issue an error message and continue, instead of dying here\n> (i.e. replace the above die() with error() or even warning())?\n>\n> That way, if the helper has not been updated to support 'force', but\n> the user has been happily accepting the result he gets from \"git\n> push $there +master\" (perhaps because he has never pushed a non-ff\n> history so far), he now gets a warning that essentially says that\n> his push has been working by accident, even when the particular\n> ff-push that showed the error message goes through successfully.\n>\n> If his push through the helper that has not been updated does need\n> 'force', he gets the same old behaviour, possibly a broken one, out\n> of the helper, and again he does get the warning.\n>\n> Because updated helpers know 'force' option, such a s/die/warning/\n> change would not affect them at all.\n\nYeah, that makes sense.\n\n-- \nFelipe Contreras\n"},{"id":"230008","messageId":"xmqqa9ho2i6v.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"CAMP44s1wfk3zmiof6E474vNi72zTo_QFj=56G3POjopukGK-5Q@mail.gmail.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T16:35:52Z","receivedAt":"2013-11-01T16:35:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Fri, Nov 1, 2013 at 8:49 AM, Junio C Hamano wrote:\n>\n>> Because updated helpers know 'force' option, such a s/die/warning/\n>> change would not affect them at all.\n>\n> Yeah, that makes sense.\n\nOK, then let's queue this separately on top, so that we can revert\nit more easily when our \"s/die/warning/ should be safe and more\nfriendly to existing users\" reasoning we discussed is found to be\nfaulty later.\n\n-- >8 --\nSubject: [PATCH] transport-helper: demote lack of \"force\" option to a warning\n\nIt would have been a good conservative position to take, if there\nwere no existing helpers, to die when the user asked to force a push\nthrough a transport helper mechanism and the helper script hasn't\nbeen updated to handle the \"force\" option.\n\nThere however are existing helpers in the field and none of them\nobviously has been taught about the option yet. If a helper has not\nbeen updated to understand \"force\", but the user has happily been\naccepting the result of \"git push $there +master\" (perhaps because\nhe has never pushed a non-ff history so far), the change made\npreviously in this series will fail the push, which would be a minor\nregression for such a user.\n\nDemote lack of \"force\" option from a \"die()\" to a \"warning()\". By\ndoing so, such a user now gets a warning that notifies him that his\npush has been working by accident, even when the particular ff-push\nthat showed the error message goes through successfully.\n\nIf his push does need \"force\", he gets the same old behaviour,\npossibly a broken one, out of the helper that has not been update,\nbut again he does see the warning.\n\nHelped-by: Felipe Contreras \nSigned-off-by: Junio C Hamano \n---\n transport-helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex ef91882..6b167ea 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -862,7 +862,7 @@ static int push_refs_with_export(struct transport *transport,\n \n \tif (flags & TRANSPORT_PUSH_FORCE) {\n \t\tif (set_helper_option(transport, \"force\", \"true\") != 0)\n-\t\t\tdie(\"helper %s does not support 'force'\", data->name);\n+\t\t\twarning(\"helper %s does not support 'force'\", data->name);\n \t}\n \n \thelper = get_helper(transport);\n-- \n1.8.5-rc0-205-g5b7460b\n"},{"id":"230010","messageId":"CAPig+cSZJoTJbgWHZfoWvtzsePUq=KbNoxapwA+59fnT+KoUPA@mail.gmail.com","threadId":"35254","inReplyTo":"xmqqa9ho2i6v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 03/10] transport-helper: add 'force' to 'export' helpers","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-11-01T17:28:59Z","receivedAt":"2013-11-01T17:28:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 1, 2013 at 12:35 PM, Junio C Hamano wrote:\n> Subject: [PATCH] transport-helper: demote lack of \"force\" option to a warning\n>\n> It would have been a good conservative position to take, if there\n> were no existing helpers, to die when the user asked to force a push\n> through a transport helper mechanism and the helper script hasn't\n> been updated to handle the \"force\" option.\n>\n> There however are existing helpers in the field and none of them\n> obviously has been taught about the option yet. If a helper has not\n> been updated to understand \"force\", but the user has happily been\n> accepting the result of \"git push $there +master\" (perhaps because\n> he has never pushed a non-ff history so far), the change made\n> previously in this series will fail the push, which would be a minor\n> regression for such a user.\n>\n> Demote lack of \"force\" option from a \"die()\" to a \"warning()\". By\n> doing so, such a user now gets a warning that notifies him that his\n> push has been working by accident, even when the particular ff-push\n> that showed the error message goes through successfully.\n>\n> If his push does need \"force\", he gets the same old behaviour,\n> possibly a broken one, out of the helper that has not been update,\n\ns/update/updated/\n\n> but again he does see the warning.\n>\n> Helped-by: Felipe Contreras \n> Signed-off-by: Junio C Hamano \n"},{"id":"230156","messageId":"xmqq38n98cud.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"CAMP44s246M5DaeX80tqzfjRWnbE4vKh-vp_tLW-qAQUFFPsP0Q@mail.gmail.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-06T21:00:42Z","receivedAt":"2013-11-06T21:00: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, Oct 31, 2013 at 12:26 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>> It feels somewhat redundant that you have to twice say that you are\n>> pushing your \"master\", once with --refspec and then the branch\n>> name. Is this the best we can do?\n>\n> As this has been discussed before and no other solution came forward, yes.\n\nWe need to take that \"no other solution came forward\" with a grain\nof salt. After all, this is your itch, and if nobody was interested\nin helping you (which I think that we both understand entirely\nplausible, given the recent history), it only means you didn't think\nof any other solution.\n\nI didn't think things through, but at the external UI level, I see a\npossibility of a nicer way to express the above.\n\nIn our \"push\" refspec (and export is about pushing what we have), a\ncolonless refspec A is a short-hand for A:A, so the traditional\n\n\tgit fast-export master\n\ncan be thought of, in a new world order with a patch that lets you\ndo a ref mapping, a short-hand for an identical mapping:\n\n\tgit fast-export master:master\n\nIt follows that the syntax naturally support\n\n\tgit fast-export refs/heads/master:refs/heads/foobar\n\nI would think.\n\nThat approach lets you express ref mapping without a new option\n--refspec, which goes contrary to existing UI for any commands in\nGit (I think nobody takes refspec as a value to a dashed-option in\nthe transport; both in fetch and push, they are proper operands,\ni.e. command line arguments to the command), no?\n"},{"id":"230160","messageId":"20131106221427.GB13258@sigill.intra.peff.net","threadId":"35254","inReplyTo":"xmqq38n98cud.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-06T22:14:27Z","receivedAt":"2013-11-06T22:14:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 06, 2013 at 01:00:42PM -0800, Junio C Hamano wrote:\n\n> I didn't think things through, but at the external UI level, I see a\n> possibility of a nicer way to express the above.\n> \n> In our \"push\" refspec (and export is about pushing what we have), a\n> colonless refspec A is a short-hand for A:A, so the traditional\n> \n> \tgit fast-export master\n> \n> can be thought of, in a new world order with a patch that lets you\n> do a ref mapping, a short-hand for an identical mapping:\n> \n> \tgit fast-export master:master\n> \n> It follows that the syntax naturally support\n> \n> \tgit fast-export refs/heads/master:refs/heads/foobar\n> \n> I would think.\n> \n> That approach lets you express ref mapping without a new option\n> --refspec, which goes contrary to existing UI for any commands in\n> Git (I think nobody takes refspec as a value to a dashed-option in\n> the transport; both in fetch and push, they are proper operands,\n> i.e. command line arguments to the command), no?\n\nI think that is much nicer for the simple cases, but how do we handle\nmore complex rev expressions? One can say:\n\n git fast-export master ^origin\n\nor even:\n\n git fast-export origin..master\n\nThe \"^origin\" is not a refspec, and finding the refspec in the\ndot-expression would involve parsing it into two components. I think you\ncan come up with a workable system by parsing the arguments as revision\nspecifiers and then applying some rules. E.g., a positive ref \"foo\" is a\nrefspec \"foo:foo\", but negative \"^bar\" does not impact refspecs at all,\nand the same rules are applied for \"bar..foo\". There is a syntactic\nconflict where \"foo:bar\" would be interpreted as a tree:path by the\nrevision code, though, whereas in this context it means a refspec.\n\nSo I think it is possible to go that route (and perhaps preferable,\neven, because it keeps the simple and common cases easy for the user),\nbut the implementation is not as simple as just treating the arguments\nas refspecs.\n\n-Peff\n"},{"id":"230163","messageId":"xmqqr4at6u24.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"20131106221427.GB13258@sigill.intra.peff.net","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-06T22:31:47Z","receivedAt":"2013-11-06T22:31:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think that is much nicer for the simple cases, but how do we handle\n> more complex rev expressions? One can say:\n>\n> git fast-export master ^origin\n>\n> or even:\n>\n> git fast-export origin..master\n>\n> The \"^origin\" is not a refspec, and finding the refspec in the\n> dot-expression would involve parsing it into two components. I think you\n> can come up with a workable system by parsing the arguments as revision\n> specifiers and then applying some rules. E.g., a positive ref \"foo\" is a\n> refspec \"foo:foo\", but negative \"^bar\" does not impact refspecs at all,\n> and the same rules are applied for \"bar..foo\". There is a syntactic\n> conflict where \"foo:bar\" would be interpreted as a tree:path by the\n> revision code, though, whereas in this context it means a refspec.\n>\n> So I think it is possible to go that route (and perhaps preferable,\n> even, because it keeps the simple and common cases easy for the user),\n> but the implementation is not as simple as just treating the arguments\n> as refspecs.\n\nI forgot about the ranges, but thinking about it further, I think\nwhat we really need to worry about is the positive end.\n\n\tgit fast-export origin..master\n\nis (by the usual definition of A..B range) the same as\n\n\tgit fast-export ^origin master\n\nIf we realize that the bottom ends of ranges do not participate in\nthe \"update the ref at the receiving end\" phase of object/history\ntransfer at all, that command line can still be thought of as a\nshort-hand for\n\n\tgit fast-export ^origin refs/heads/master:refs/heads/master\n\nThat is, anything negative is used only to specify the bottom of the\nrange, and positive ones (e.g. 'master' in both of your examples) tell\nus what ref to update in addition to where the history ends.\n"},{"id":"230164","messageId":"xmqqmwlh6tlh.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"xmqqr4at6u24.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-06T22:41:46Z","receivedAt":"2013-11-06T22:41:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> I think that is much nicer for the simple cases, but how do we handle\n>> more complex rev expressions? One can say:\n>>\n>> git fast-export master ^origin\n>>\n>> or even:\n>>\n>> git fast-export origin..master\n>>\n>> The \"^origin\" is not a refspec, and finding the refspec in the\n>> dot-expression would involve parsing it into two components. I think you\n>> can come up with a workable system by parsing the arguments as revision\n>> specifiers and then applying some rules. E.g., a positive ref \"foo\" is a\n>> refspec \"foo:foo\", but negative \"^bar\" does not impact refspecs at all,\n>> and the same rules are applied for \"bar..foo\". There is a syntactic\n>> conflict where \"foo:bar\" would be interpreted as a tree:path by the\n>> revision code, though, whereas in this context it means a refspec.\n>>\n>> So I think it is possible to go that route (and perhaps preferable,\n>> even, because it keeps the simple and common cases easy for the user),\n>> but the implementation is not as simple as just treating the arguments\n>> as refspecs.\n>\n> I forgot about the ranges, but thinking about it further, I think ...\n\nAnd I am an idiot; you've spelled out everything I wanted to say, so\nI should have just dropped this ;-)\n"},{"id":"230176","messageId":"CAMP44s2dCFWyaP3tBQ1SZf8222c58JT19o7bQM7+EobN9KJ2jQ@mail.gmail.com","threadId":"35254","inReplyTo":"xmqq38n98cud.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-07T01:00:12Z","receivedAt":"2013-11-07T01:00:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 6, 2013 at 3:00 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Thu, Oct 31, 2013 at 12:26 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>>> It feels somewhat redundant that you have to twice say that you are\n>>> pushing your \"master\", once with --refspec and then the branch\n>>> name. Is this the best we can do?\n>>\n>> As this has been discussed before and no other solution came forward, yes.\n>\n> We need to take that \"no other solution came forward\" with a grain\n> of salt. After all, this is your itch, and if nobody was interested\n> in helping you (which I think that we both understand entirely\n> plausible, given the recent history), it only means you didn't think\n> of any other solution.\n\nThinking is easy, the problem is not thinking of a solution, the\nproblem is actually doing it. I think we should end world hunger. What\na great idea!\n\n> I didn't think things through, but at the external UI level, I see a\n> possibility of a nicer way to express the above.\n\nYou already said that before.\n\nTalk is cheap. Show me the code.\n\n-- \nFelipe Contreras\n"},{"id":"230255","messageId":"xmqqwqkimpas.fsf@gitster.dls.corp.google.com","threadId":"35254","inReplyTo":"20131106221427.GB13258@sigill.intra.peff.net","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-08T23:44:43Z","receivedAt":"2013-11-08T23:44:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 06, 2013 at 01:00:42PM -0800, Junio C Hamano wrote:\n>\n>> It follows that the syntax naturally support\n>> \n>> \tgit fast-export refs/heads/master:refs/heads/foobar\n>> \n>> I would think.\n>> \n>> That approach lets you express ref mapping without a new option\n>> --refspec, which goes contrary to existing UI for any commands in\n>> Git (I think nobody takes refspec as a value to a dashed-option in\n>> the transport; both in fetch and push, they are proper operands,\n>> i.e. command line arguments to the command), no?\n>\n> I think that is much nicer for the simple cases, but how do we handle\n> more complex rev expressions? ...\n>\n> The \"^origin\" is not a refspec, and finding the refspec in the\n> dot-expression would involve parsing it into two components. I think you\n> can come up with a workable system by parsing the arguments as revision\n> specifiers and then applying some rules. E.g....\n\nI was thinking about this a bit more today. It is more or less\ntrivial to actually teach the setup_revisions() infrastructure to\noptionally allow A:B to mean \"We want a revision A, but with an\nextra twist\", and leave that \"extra twist\" information in the\nrev_cmdline machinery. After all, rev_cmdline was introduced for\ndoing exactly this kind of thing.\n\nEarlier I said that all the existing transport commands take refspec\nas proper operands, not a value to a dashed-option, but I can imagine\nthat we may in the future want to update \"git push\" in a way similar\nto what Felipe did to \"git fast-export\" so that it allows something\nlike this:\n\n $ git push mothership \\\n > --refspec refs/heads/*:refs/remotes/satellite/* master\n\nwhich would mean \"I am pushing out 'master', but anything I push out\nto the mothership from my refs/heads/ hierarchy should be used to\nupdate the refs/remotes/satellite/ hierarchy over there\". The same\nthing can be done in the reverse direction for \"git fetch\".\n\nBut such a wildcard refspec cannot be supported naturally by\nextending the setup_revisions(); what the wildcarded refspec expands\nto will depend on what other things are on the command line (in this\ncase, 'master'). So I stopped there (I'll attach a toy patch at the\nend, but I'll discard it once I send this message out).\n\nIf you set remote.*.fetch (or remote.*.push) refspec with the\ncurrent system, it tells us two logically separate/separable things:\n\n (1) what is the set of refs fetched (or pushed); and\n\n (2) what refs at the receiving end are updated.\n\n\"git push\" and \"git fetch\" could borrow the independent ref-mapping\nUI from \"git fast-export\" to allow us to dissociate these two\nconcepts. In the above \"mothership-satelite\" example, the \"master\"\nspecifies what is pushed out, while the value of \"--refspec\" option\nspecifies the mapping. It would open a door to even make the\nmapping a configuration variable. In short, \"nobody uses an\nindependent refspec mapping parameter\" does not necessarily mean \"we\nshould not have such an independent refspec mapping parameter\".\n\nIf we were to go that route, however, I would be strongly against\ncalling that option --refspec; perhaps calling it --refmap would\navoid confusion.\n\n remote.c | 5 +++++\n remote.h | 2 ++\n revision.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n revision.h | 10 +++++++++-\n 4 files changed, 82 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 9f1a8aa..26b86a0 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -653,6 +653,11 @@ static struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)\n \treturn parse_refspec_internal(nr_refspec, refspec, 0, 0);\n }\n \n+struct refspec *parse_push_refspec_verify(int nr_refspec, const char **refspec)\n+{\n+\treturn parse_refspec_internal(nr_refspec, refspec, 0, 1);\n+}\n+\n void free_refspec(int nr_refspec, struct refspec *refspec)\n {\n \tint i;\ndiff --git a/remote.h b/remote.h\nindex 131130a..2bc0a7e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -156,6 +156,8 @@ void ref_remove_duplicates(struct ref *ref_map);\n int valid_fetch_refspec(const char *refspec);\n struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);\n \n+struct refspec *parse_push_refspec_verify(int nr_refspec, const char **refspec);\n+\n void free_refspec(int nr_refspec, struct refspec *refspec);\n \n char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\ndiff --git a/revision.c b/revision.c\nindex 956040c..17e7b3d 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -16,6 +16,7 @@\n #include \"line-log.h\"\n #include \"mailmap.h\"\n #include \"commit-slab.h\"\n+#include \"remote.h\"\n \n volatile show_early_output_fn_t show_early_output;\n \n@@ -1377,6 +1378,58 @@ static void prepare_show_merge(struct rev_info *revs)\n \trevs->limited = 1;\n }\n \n+static int handle_revision_refspec(const char *arg, struct rev_info *revs, unsigned revarg_opt)\n+{\n+\tint cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;\n+\tstruct refspec *pair;\n+\tstatic struct ref *local_refs;\n+\tstruct ref *remote_refs = NULL;\n+\tstruct object *object;\n+\tint retval = 0;\n+\n+\tpair = parse_push_refspec_verify(1, &arg);\n+\tif (!pair)\n+\t\treturn -1;\n+\n+\tif (pair->matching || pair->pattern) {\n+\t\t/* \":\" or \"refs/heads/:refs/heads/\" are not revs */\n+\t\tretval = -1;\n+\t\tgoto cleanup_return;\n+\t}\n+\n+\t/* The source side is what we are pushing out */\n+\tif (!local_refs)\n+\t\tlocal_refs = get_local_heads();\n+\tif (match_push_refs(local_refs, &remote_refs, 1, &arg, 0)) {\n+\t\tretval = -1;\n+\t\tgoto cleanup_return;\n+\t}\n+\n+\t/*\n+\t * Now, remote_refs should have a single element that tells\n+\t * us what object we are pushing.\n+\t */\n+\tif (!remote_refs || remote_refs->next || !remote_refs->peer_ref ||\n+\t !(object = parse_object(remote_refs->peer_ref->new_sha1))) {\n+\t\tretval = -1;\n+\t\tgoto cleanup_return;\n+\t}\n+\n+\tif (!cant_be_filename)\n+\t\tverify_non_filename(revs->prefix, arg);\n+\n+\tretval = 0;\n+\tadd_rev_cmdline(revs, object, arg, REV_CMD_REFSPEC, 0);\n+\tadd_pending_object(revs, object, arg);\n+\n+cleanup_return:\n+\tfree_refs(remote_refs);\n+\tfree(pair->src);\n+\tfree(pair->dst);\n+\tfree(pair);\n+\treturn retval;\n+}\n+\n int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)\n {\n \tstruct object_context oc;\n@@ -1500,8 +1553,20 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi\n \tif (revarg_opt & REVARG_COMMITTISH)\n \t\tget_sha1_flags = GET_SHA1_COMMITTISH;\n \n-\tif (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc))\n+\t/*\n+\t * Are we allowed to interpret A:B refspec as a revision\n+\t * specifying A? ^A:B nor --not A:B would make any sense, so\n+\t * do not route such cases to handle_revision_refspec().\n+\t */\n+\tif (!local_flags && !flags &&\n+\t (revarg_opt & REVARG_ALLOW_REFSPEC) &&\n+\t !handle_revision_refspec(arg, revs, revarg_opt))\n+\t\treturn 0;\n+\n+\tif (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc)) {\n \t\treturn revs->ignore_missing ? 0 : -1;\n+\t}\n+\n \tif (!cant_be_filename)\n \t\tverify_non_filename(revs->prefix, arg);\n \tobject = get_reference(revs, arg, sha1, flags ^ local_flags);\ndiff --git a/revision.h b/revision.h\nindex 89132df..c97805e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -40,7 +40,8 @@ struct rev_cmdline_info {\n \t\t\tREV_CMD_LEFT,\n \t\t\tREV_CMD_RIGHT,\n \t\t\tREV_CMD_MERGE_BASE,\n-\t\t\tREV_CMD_REV\n+\t\t\tREV_CMD_REV,\n+\t\t\tREV_CMD_REFSPEC\n \t\t} whence;\n \t\tunsigned flags;\n \t} *rev;\n@@ -205,7 +206,13 @@ extern volatile show_early_output_fn_t show_early_output;\n \n struct setup_revision_opt {\n \tconst char *def;\n+\t/* hook to call after parsing the command line */\n \tvoid (*tweak)(struct rev_info *, struct setup_revision_opt *);\n+\t/*\n+\t * hook to call for a command line argument that is not a rev\n+\t */\n+\tint (*parse_extended_rev)(struct rev_info *, struct setup_revision_opt *, const char *);\n+\n \tconst char *submodule;\n \tint assume_dashdash;\n \tunsigned revarg_opt;\n@@ -219,6 +226,7 @@ extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ct\n \t\t\t const char * const usagestr[]);\n #define REVARG_CANNOT_BE_FILENAME 01\n #define REVARG_COMMITTISH 02\n+#define REVARG_ALLOW_REFSPEC 04\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\t\t int flags, unsigned revarg_opt);\n \n"},{"id":"230257","messageId":"20131109000653.GB22906@sigill.intra.peff.net","threadId":"35254","inReplyTo":"xmqqwqkimpas.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 06/10] fast-export: add new --refspec option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-09T00:06:53Z","receivedAt":"2013-11-09T00:06:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 08, 2013 at 03:44:43PM -0800, Junio C Hamano wrote:\n\n> > The \"^origin\" is not a refspec, and finding the refspec in the\n> > dot-expression would involve parsing it into two components. I think you\n> > can come up with a workable system by parsing the arguments as revision\n> > specifiers and then applying some rules. E.g....\n> \n> I was thinking about this a bit more today. It is more or less\n> trivial to actually teach the setup_revisions() infrastructure to\n> optionally allow A:B to mean \"We want a revision A, but with an\n> extra twist\", and leave that \"extra twist\" information in the\n> rev_cmdline machinery. After all, rev_cmdline was introduced for\n> doing exactly this kind of thing.\n\nI had wondered at first whether it would be a problem to be\nsyntactically ambiguous with :. But I don't think so. Doing:\n\n git fast-export HEAD:Documentation\n\ndoes not produce any output currently. And the fast-import stream cannot\nactually represent trees outside of commits. Something like:\n\n git fast-export HEAD:Makefile\n\ncould produce a blob entry, but it currently does nothing. So I don't\nthink it's a big deal, as nobody is clamoring for the feature. And\ncertainly \"git push\" suffers from the same problem, and you can work\naround it with:\n\n git push $(git rev-parse HEAD:Makefile):refs/tag/makefile-blob\n\n> Earlier I said that all the existing transport commands take refspec\n> as proper operands, not a value to a dashed-option, but I can imagine\n> that we may in the future want to update \"git push\" in a way similar\n> to what Felipe did to \"git fast-export\" so that it allows something\n> like this:\n> \n> $ git push mothership \\\n> > --refspec refs/heads/*:refs/remotes/satellite/* master\n> \n> which would mean \"I am pushing out 'master', but anything I push out\n> to the mothership from my refs/heads/ hierarchy should be used to\n> update the refs/remotes/satellite/ hierarchy over there\". The same\n> thing can be done in the reverse direction for \"git fetch\".\n\nYes, though I would expect the \"--refspec\" bit would end up in config\n(else why would you not just apply it directly to master). And you would\nwant to be able to override it, like:\n\n git push mothership \\\n --refspec refs/heads/*:refs/remotes/satellite/* master:refs/other/master\n\nSo I think even with such wildcard magic, you'd still want refspecs,\nboth for \"push\" and for \"fast-export\".\n\n> But such a wildcard refspec cannot be supported naturally by\n> extending the setup_revisions(); what the wildcarded refspec expands\n> to will depend on what other things are on the command line (in this\n> case, 'master'). So I stopped there (I'll attach a toy patch at the\n> end, but I'll discard it once I send this message out).\n\nYes, I think applying such a refspec would be the duty of whoever\nexpands \"master\" into \"master:master\". And that is not a job for\nsetup_revisions, which should be dealing mostly with the syntax and\ntelling us \"I got just 'master'\" or \"I got 'master:foo'\". The\ninterpretation is up to the caller.\n\n> If we were to go that route, however, I would be strongly against\n> calling that option --refspec; perhaps calling it --refmap would\n> avoid confusion.\n\nAgreed.\n\n-Peff\n"},{"id":"230386","messageId":"1384146067-9575-1-git-send-email-rhansen@bbn.com","threadId":"35254","inReplyTo":"1383212197-14259-7-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH] fixup! transport-helper: add 'force' to 'export' helpers","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2013-11-11T05:01:07Z","receivedAt":"2013-11-11T05:01:07Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"defend against force=foo in the user's environment\n\nSigned-off-by: Richard Hansen \n---\n git-remote-testgit.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-remote-testgit.sh b/git-remote-testgit.sh\nindex 6d2f282..1cfdea2 100755\n--- a/git-remote-testgit.sh\n+++ b/git-remote-testgit.sh\n@@ -15,6 +15,8 @@ test -z \"$refspec\" && prefix=\"refs\"\n \n export GIT_DIR=\"$url/.git\"\n \n+force=\n+\n mkdir -p \"$dir\"\n \n if test -z \"$GIT_REMOTE_TESTGIT_NO_MARKS\"\n-- \n1.8.5.rc1.207.gc17dd22\n"},{"id":"230387","messageId":"1384146541-11607-1-git-send-email-rhansen@bbn.com","threadId":"35254","inReplyTo":"1383212197-14259-2-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 11/10] test-hg.sh: tests are now expected to pass","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2013-11-11T05:09:01Z","receivedAt":"2013-11-11T05:09:01Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"Signed-off-by: Richard Hansen \n---\n contrib/remote-helpers/test-hg.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh\nindex 9f5066b..e6e3fed 100755\n--- a/contrib/remote-helpers/test-hg.sh\n+++ b/contrib/remote-helpers/test-hg.sh\n@@ -604,7 +604,7 @@ test_expect_success 'remote big push fetch first' '\n \t)\n '\n \n-test_expect_failure 'remote big push force' '\n+test_expect_success 'remote big push force' '\n \ttest_when_finished \"rm -rf hgrepo gitrepo*\" &&\n \n \tsetup_big_push\n@@ -634,7 +634,7 @@ test_expect_failure 'remote big push force' '\n \tcheck_bookmark hgrepo new_bmark six\n '\n \n-test_expect_failure 'remote big push dry-run' '\n+test_expect_success 'remote big push dry-run' '\n \ttest_when_finished \"rm -rf hgrepo gitrepo*\" &&\n \n \tsetup_big_push\n-- \n1.8.5.rc1.207.gc17dd22\n"},{"id":"230388","messageId":"1384146656-11811-1-git-send-email-rhansen@bbn.com","threadId":"35254","inReplyTo":"1383212197-14259-2-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 12/10] remote-bzr: support the new 'force' option","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2013-11-11T05:10:56Z","receivedAt":"2013-11-11T05:10:56Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"Signed-off-by: Richard Hansen \n---\n contrib/remote-helpers/git-remote-bzr | 34 +++++++++++++++++++++++++++++++++-\n contrib/remote-helpers/test-bzr.sh | 22 +++++++++++++++++++++-\n 2 files changed, 54 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\nindex 7e34532..ba693d1 100755\n--- a/contrib/remote-helpers/git-remote-bzr\n+++ b/contrib/remote-helpers/git-remote-bzr\n@@ -42,6 +42,7 @@ import json\n import re\n import StringIO\n import atexit, shutil, hashlib, urlparse, subprocess\n+import types\n \n NAME_RE = re.compile('^([^<>]+)')\n AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')\n@@ -684,7 +685,8 @@ def do_export(parser):\n peer = bzrlib.branch.Branch.open(peers[name],\n possible_transports=transports)\n try:\n- peer.bzrdir.push_branch(branch, revision_id=revid)\n+ peer.bzrdir.push_branch(branch, revision_id=revid,\n+ overwrite=force)\n except bzrlib.errors.DivergedBranches:\n print \"error %s non-fast forward\" % ref\n continue\n@@ -718,8 +720,34 @@ def do_capabilities(parser):\n print \"*import-marks %s\" % path\n print \"*export-marks %s\" % path\n \n+ print \"option\"\n print\n \n+class InvalidOptionValue(Exception):\n+ pass\n+\n+def do_option(parser):\n+ (opt, val) = parser[1:3]\n+ handler = globals().get('do_option_' + opt)\n+ if handler and type(handler) == types.FunctionType:\n+ try:\n+ handler(val)\n+ except InvalidOptionValue:\n+ print \"error '%s' is not a valid value for option '%s'\" % (val, opt)\n+ else:\n+ print \"unsupported\"\n+\n+def do_bool_option(val):\n+ if val == 'true': ret = True\n+ elif val == 'false': ret = False\n+ else: raise InvalidOptionValue()\n+ print \"ok\"\n+ return ret\n+\n+def do_option_force(val):\n+ global force\n+ force = do_bool_option(val)\n+\n def ref_is_valid(name):\n return not True in [c in name for c in '~^: \\\\']\n \n@@ -882,6 +910,7 @@ def main(args):\n global is_tmp\n global branches, peers\n global transports\n+ global force\n \n alias = args[1]\n url = args[2]\n@@ -895,6 +924,7 @@ def main(args):\n branches = {}\n peers = {}\n transports = []\n+ force = False\n \n if alias[5:] == url:\n is_tmp = True\n@@ -930,6 +960,8 @@ def main(args):\n do_import(parser)\n elif parser.check('export'):\n do_export(parser)\n+ elif parser.check('option'):\n+ do_option(parser)\n else:\n die('unhandled command: %s' % line)\n sys.stdout.flush()\ndiff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh\nindex ea597b0..7d7778f 100755\n--- a/contrib/remote-helpers/test-bzr.sh\n+++ b/contrib/remote-helpers/test-bzr.sh\n@@ -69,13 +69,33 @@ test_expect_success 'pushing' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'forced pushing' '\n+\t(\n+\tcd gitrepo &&\n+\techo three-new >content &&\n+\tgit commit -a --amend -m three-new &&\n+\tgit push -f\n+\t) &&\n+\n+\t(\n+\tcd bzrrepo &&\n+\t# the forced update overwrites the bzr branch but not the bzr\n+\t# working directory (it tries to merge instead)\n+\tbzr revert\n+\t) &&\n+\n+\techo three-new >expected &&\n+\tcat bzrrepo/content >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'roundtrip' '\n \t(\n \tcd gitrepo &&\n \tgit pull &&\n \tgit log --format=\"%s\" -1 origin/master >actual\n \t) &&\n-\techo three >expected &&\n+\techo three-new >expected &&\n \ttest_cmp expected actual &&\n \n \t(cd gitrepo && git push && git pull) &&\n-- \n1.8.5.rc1.207.gc17dd22\n"},{"id":"230396","messageId":"5280c4bc4354a_36a51059e74ea@nysa.notmuch","threadId":"35254","inReplyTo":"1384146656-11811-1-git-send-email-rhansen@bbn.com","subject":"RE: [PATCH v5 12/10] remote-bzr: support the new 'force' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-11T11:51:24Z","receivedAt":"2013-11-11T11:51:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Richard Hansen wrote:\n> Signed-off-by: Richard Hansen \n> ---\n> contrib/remote-helpers/git-remote-bzr | 34 +++++++++++++++++++++++++++++++++-\n> contrib/remote-helpers/test-bzr.sh | 22 +++++++++++++++++++++-\n> 2 files changed, 54 insertions(+), 2 deletions(-)\n> \n> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\n> index 7e34532..ba693d1 100755\n> --- a/contrib/remote-helpers/git-remote-bzr\n> +++ b/contrib/remote-helpers/git-remote-bzr\n> @@ -42,6 +42,7 @@ import json\n> import re\n> import StringIO\n> import atexit, shutil, hashlib, urlparse, subprocess\n> +import types\n> \n> NAME_RE = re.compile('^([^<>]+)')\n> AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')\n> @@ -684,7 +685,8 @@ def do_export(parser):\n> peer = bzrlib.branch.Branch.open(peers[name],\n> possible_transports=transports)\n> try:\n> - peer.bzrdir.push_branch(branch, revision_id=revid)\n> + peer.bzrdir.push_branch(branch, revision_id=revid,\n> + overwrite=force)\n> except bzrlib.errors.DivergedBranches:\n> print \"error %s non-fast forward\" % ref\n> continue\n> @@ -718,8 +720,34 @@ def do_capabilities(parser):\n> print \"*import-marks %s\" % path\n> print \"*export-marks %s\" % path\n> \n> + print \"option\"\n> print\n> \n> +class InvalidOptionValue(Exception):\n> + pass\n> +\n> +def do_option(parser):\n> + (opt, val) = parser[1:3]\n> + handler = globals().get('do_option_' + opt)\n> + if handler and type(handler) == types.FunctionType:\n> + try:\n> + handler(val)\n> + except InvalidOptionValue:\n> + print \"error '%s' is not a valid value for option '%s'\" % (val, opt)\n> + else:\n> + print \"unsupported\"\n> +\n> +def do_bool_option(val):\n> + if val == 'true': ret = True\n> + elif val == 'false': ret = False\n> + else: raise InvalidOptionValue()\n> + print \"ok\"\n> + return ret\n> +\n> +def do_option_force(val):\n> + global force\n> + force = do_bool_option(val)\n> +\n\nWhile this organization has merit, I think it's overkill for a single option,\nor just a couple of them. If in the future we add more, we might revisit this,\nfor the moment something like this would suffice:\n\n class InvalidOptionValue(Exception):\n\tpass\n\n def get_bool_option(val):\n\tif val == 'true':\n\t return True\n\telif val == 'false':\n\t return False\n\telse:\n\t raise InvalidOptionValue()\n\n def do_option(parser):\n\tglobal force\n\t_, key, value = parser.line.split(' ')\n\ttry:\n\t if key == 'force':\n\t\tforce = get_bool_option(value)\n\t\tprint 'ok'\n\t else:\n\t\tprint 'unsupported'\n\texcept InvalidOptionValue:\n\t print \"error '%s' is not a valid value for option '%s'\" % (value, key)\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"230404","messageId":"52811E1F.4090500@bbn.com","threadId":"35254","inReplyTo":"5280c4bc4354a_36a51059e74ea@nysa.notmuch","subject":"Re: [PATCH v5 12/10] remote-bzr: support the new 'force' option","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2013-11-11T18:12:47Z","receivedAt":"2013-11-11T18:12:47Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"On 2013-11-11 06:51, Felipe Contreras wrote:\n> Richard Hansen wrote:\n>> Signed-off-by: Richard Hansen \n>> ---\n>> contrib/remote-helpers/git-remote-bzr | 34 +++++++++++++++++++++++++++++++++-\n>> contrib/remote-helpers/test-bzr.sh | 22 +++++++++++++++++++++-\n>> 2 files changed, 54 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\n>> index 7e34532..ba693d1 100755\n>> --- a/contrib/remote-helpers/git-remote-bzr\n>> +++ b/contrib/remote-helpers/git-remote-bzr\n>> @@ -42,6 +42,7 @@ import json\n>> import re\n>> import StringIO\n>> import atexit, shutil, hashlib, urlparse, subprocess\n>> +import types\n>> \n>> NAME_RE = re.compile('^([^<>]+)')\n>> AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')\n>> @@ -684,7 +685,8 @@ def do_export(parser):\n>> peer = bzrlib.branch.Branch.open(peers[name],\n>> possible_transports=transports)\n>> try:\n>> - peer.bzrdir.push_branch(branch, revision_id=revid)\n>> + peer.bzrdir.push_branch(branch, revision_id=revid,\n>> + overwrite=force)\n>> except bzrlib.errors.DivergedBranches:\n>> print \"error %s non-fast forward\" % ref\n>> continue\n>> @@ -718,8 +720,34 @@ def do_capabilities(parser):\n>> print \"*import-marks %s\" % path\n>> print \"*export-marks %s\" % path\n>> \n>> + print \"option\"\n>> print\n>> \n>> +class InvalidOptionValue(Exception):\n>> + pass\n>> +\n>> +def do_option(parser):\n>> + (opt, val) = parser[1:3]\n>> + handler = globals().get('do_option_' + opt)\n>> + if handler and type(handler) == types.FunctionType:\n>> + try:\n>> + handler(val)\n>> + except InvalidOptionValue:\n>> + print \"error '%s' is not a valid value for option '%s'\" % (val, opt)\n>> + else:\n>> + print \"unsupported\"\n>> +\n>> +def do_bool_option(val):\n>> + if val == 'true': ret = True\n>> + elif val == 'false': ret = False\n>> + else: raise InvalidOptionValue()\n>> + print \"ok\"\n>> + return ret\n>> +\n>> +def do_option_force(val):\n>> + global force\n>> + force = do_bool_option(val)\n>> +\n> \n> While this organization has merit, I think it's overkill for a single option,\n> or just a couple of them. If in the future we add more, we might revisit this,\n> for the moment something like this would suffice:\n\nOK, I'll reroll.\n\n> \n> class InvalidOptionValue(Exception):\n> \tpass\n> \n> def get_bool_option(val):\n> \tif val == 'true':\n> \t return True\n> \telif val == 'false':\n> \t return False\n> \telse:\n> \t raise InvalidOptionValue()\n> \n> def do_option(parser):\n> \tglobal force\n> \t_, key, value = parser.line.split(' ')\n\nI'm surprised you prefer this over 'key, val = parser[1:3]' or even\n'_, key, val = parser[:]'. Are you intending to eventually remove\nParser.__getitem__()?\n\nThanks,\nRichard\n\n\n> \ttry:\n> \t if key == 'force':\n> \t\tforce = get_bool_option(value)\n> \t\tprint 'ok'\n> \t else:\n> \t\tprint 'unsupported'\n> \texcept InvalidOptionValue:\n> \t print \"error '%s' is not a valid value for option '%s'\" % (value, key)\n> \n> Cheers.\n"},{"id":"230405","messageId":"CAMP44s1SC0SQLKWG13-FRwvX6AB6pZmn+z0f1ovnuawC6wcb+w@mail.gmail.com","threadId":"35254","inReplyTo":"52811E1F.4090500@bbn.com","subject":"Re: [PATCH v5 12/10] remote-bzr: support the new 'force' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-11T18:15:53Z","receivedAt":"2013-11-11T18:15:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 11, 2013 at 12:12 PM, Richard Hansen wrote:\n> On 2013-11-11 06:51, Felipe Contreras wrote:\n\n>> def do_option(parser):\n>> global force\n>> _, key, value = parser.line.split(' ')\n>\n> I'm surprised you prefer this over 'key, val = parser[1:3]' or even\n> '_, key, val = parser[:]'. Are you intending to eventually remove\n> Parser.__getitem__()?\n\nI don't, actually. I'm fine with either way.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}