{"thread":{"id":"27656","subject":"[PATCH v2 v2 00/20] remote-helper improvements","startedAt":"2011-06-19T15:18:25Z","lastAt":"2011-07-04T22:37:59Z","messageCount":44,"participants":["Sverre Rabbelier","Jonathan Nieder","Dmitry Ivankov","Brandon Casey","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":20},"messages":[{"id":"170232","messageId":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":null,"subject":"[PATCH v2 v2 00/20] remote-helper improvements","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:25Z","receivedAt":"2011-06-19T15:18:25Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This is v2 of the series, mostly minor changes.\n\nJeff King (6):\n transport-helper: fix minor leak in push_refs_with_export\n t5800: factor out some ref tests\n t5800: document some non-functional parts of remote helpers\n teach remote-testgit to import non-HEAD refs\n transport-helper: don't feed bogus refs to export push\n git_remote_helpers: push all refs during a non-local export\n\nSverre Rabbelier (14):\n t5800: use skip_all instead of prereq\n remote-curl: accept empty line as terminator\n git-remote-testgit: only push for non-local repositories\n git-remote-testgit: fix error handling\n fast-import: introduce 'done' command\n fast-export: support done feature\n transport-helper: factor out push_update_refs_status\n transport-helper: check status code of finish_command\n transport-helper: use the new done feature where possible\n transport-helper: update ref status after push with export\n transport-helper: change import semantics\n transport-helper: export is no longer always the last command\n transport-helper: Use capname for gitdir capability too\n transport-helper: implement marks location as capability\n\n Documentation/git-fast-export.txt | 4 +\n Documentation/git-fast-import.txt | 25 ++++\n builtin/fast-export.c | 9 ++\n fast-import.c | 8 ++\n git-remote-testgit.py | 57 ++++++---\n git_remote_helpers/git/exporter.py | 15 ++-\n git_remote_helpers/git/importer.py | 32 +++++-\n git_remote_helpers/git/non_local.py | 20 +--\n git_remote_helpers/git/repo.py | 7 +-\n git_remote_helpers/util.py | 66 ++++++++++\n remote-curl.c | 3 +\n t/t5800-remote-helpers.sh | 93 +++++++++++---\n t/t9300-fast-import.sh | 42 ++++++\n transport-helper.c | 238 ++++++++++++++++++----------------\n 14 files changed, 445 insertions(+), 174 deletions(-)\n\n-- \n1.7.5.1.292.g728120\n"},{"id":"170233","messageId":"1308496725-22329-2-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 01/20] transport-helper: fix minor leak in push_refs_with_export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:26Z","receivedAt":"2011-06-19T15:18:26Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n transport-helper.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 660147f..b560b64 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -728,6 +728,7 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\tstrbuf_addf(&buf, \"^%s\", private);\n \t\t\tstring_list_append(&revlist_args, strbuf_detach(&buf, NULL));\n \t\t}\n+\t\tfree(private);\n \n \t\tstring_list_append(&revlist_args, ref->name);\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170235","messageId":"1308496725-22329-3-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 02/20] t5800: factor out some ref tests","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:27Z","receivedAt":"2011-06-19T15:18:27Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\nThese are a little hard to read, and I'm about to add more\njust like them. Plus the failure output is nicer if we use\ntest_cmp than a comparison with \"test\".\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n t/t5800-remote-helpers.sh | 12 ++++++++----\n 1 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex 1fb6380..3a37ad0 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -17,6 +17,12 @@ then\n \ttest_set_prereq PYTHON_24\n fi\n \n+compare_refs() {\n+\tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n+\tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success PYTHON_24 'setup repository' '\n \tgit init --bare server/.git &&\n \tgit clone server public &&\n@@ -59,8 +65,7 @@ test_expect_success PYTHON_24 'pushing to local repo' '\n \techo content >>file &&\n \tgit commit -a -m three &&\n \tgit push) &&\n-\tHEAD=$(git --git-dir=localclone/.git rev-parse --verify HEAD) &&\n-\ttest $HEAD = $(git --git-dir=server/.git rev-parse --verify HEAD)\n+\tcompare_refs localclone HEAD server HEAD\n '\n \n test_expect_success PYTHON_24 'synch with changes from localclone' '\n@@ -73,8 +78,7 @@ test_expect_success PYTHON_24 'pushing remote local repo' '\n \techo content >>file &&\n \tgit commit -a -m four &&\n \tgit push) &&\n-\tHEAD=$(git --git-dir=clone/.git rev-parse --verify HEAD) &&\n-\ttest $HEAD = $(git --git-dir=server/.git rev-parse --verify HEAD)\n+\tcompare_refs clone HEAD server HEAD\n '\n \n test_done\n-- \n1.7.5.1.292.g728120\n"},{"id":"170236","messageId":"1308496725-22329-4-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 03/20] t5800: use skip_all instead of prereq","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:28Z","receivedAt":"2011-06-19T15:18:28Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"All tests require python 2.4 or higher.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n New in this series.\n\n t/t5800-remote-helpers.sh | 34 +++++++++++++++++++---------------\n 1 files changed, 19 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex 3a37ad0..f6796e3 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -7,15 +7,19 @@ test_description='Test remote-helper import and export commands'\n \n . ./test-lib.sh\n \n-if test_have_prereq PYTHON && \"$PYTHON_PATH\" -c '\n+if ! test_have_prereq PYTHON ; then\n+\tskip_all='skipping git-remote-hg tests, python not available'\n+\ttest_done\n+fi\n+\n+\"$PYTHON_PATH\" -c '\n import sys\n if sys.hexversion < 0x02040000:\n sys.exit(1)\n-'\n-then\n- # Requires Python 2.4 or newer\n-\ttest_set_prereq PYTHON_24\n-fi\n+' || {\n+\tskip_all='skipping git-remote-hg tests, python version < 2.4'\n+\ttest_done\n+}\n \n compare_refs() {\n \tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n@@ -23,7 +27,7 @@ compare_refs() {\n \ttest_cmp expect actual\n }\n \n-test_expect_success PYTHON_24 'setup repository' '\n+test_expect_success 'setup repository' '\n \tgit init --bare server/.git &&\n \tgit clone server public &&\n \t(cd public &&\n@@ -33,34 +37,34 @@ test_expect_success PYTHON_24 'setup repository' '\n \t git push origin master)\n '\n \n-test_expect_success PYTHON_24 'cloning from local repo' '\n+test_expect_success 'cloning from local repo' '\n \tgit clone \"testgit::${PWD}/server\" localclone &&\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success PYTHON_24 'cloning from remote repo' '\n+test_expect_success 'cloning from remote repo' '\n \tgit clone \"testgit::file://${PWD}/server\" clone &&\n \ttest_cmp public/file clone/file\n '\n \n-test_expect_success PYTHON_24 'create new commit on remote' '\n+test_expect_success 'create new commit on remote' '\n \t(cd public &&\n \t echo content >>file &&\n \t git commit -a -m two &&\n \t git push)\n '\n \n-test_expect_success PYTHON_24 'pulling from local repo' '\n+test_expect_success 'pulling from local repo' '\n \t(cd localclone && git pull) &&\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success PYTHON_24 'pulling from remote remote' '\n+test_expect_success 'pulling from remote remote' '\n \t(cd clone && git pull) &&\n \ttest_cmp public/file clone/file\n '\n \n-test_expect_success PYTHON_24 'pushing to local repo' '\n+test_expect_success 'pushing to local repo' '\n \t(cd localclone &&\n \techo content >>file &&\n \tgit commit -a -m three &&\n@@ -68,12 +72,12 @@ test_expect_success PYTHON_24 'pushing to local repo' '\n \tcompare_refs localclone HEAD server HEAD\n '\n \n-test_expect_success PYTHON_24 'synch with changes from localclone' '\n+test_expect_success 'synch with changes from localclone' '\n \t(cd clone &&\n \t git pull)\n '\n \n-test_expect_success PYTHON_24 'pushing remote local repo' '\n+test_expect_success 'pushing remote local repo' '\n \t(cd clone &&\n \techo content >>file &&\n \tgit commit -a -m four &&\n-- \n1.7.5.1.292.g728120\n"},{"id":"170234","messageId":"1308496725-22329-5-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 04/20] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:29Z","receivedAt":"2011-06-19T15:18:29Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\nThese are all things one might expect to work in a helper\nthat is capable of handling multiple branches (which our\ntestgit helper in theory should be able to do, as it is\nbacked by git). All of these bugs are specific to the\nimport/export codepaths, so they don't affect helpers like\ngit-remote-curl that use fetch/push commands.\n\nThe first and fourth tests are about fetching and pushing\nnew refs, and demonstrate bugs in the git_remote_helpers\nlibrary (so they would be most likely to impact helpers for\nother VCSs which import/export git).\n\nThe second test is about importing multiple refs; it\ndemonstrates a bug in git-remote-testgit, which is mostly\nfor exercising the test code. Therefore it probably doesn't\naffect anyone in practice.\n\nThe third test demonstrates a bug in git's side of the\nhelper code when the upstream has added refs that we do not\nhave locally. This could impact git users who use remote\nhelpers to access foreign VCSs.\n\nAll of those bugs have fixes later in this series.\n\nThe fifth test is the most complex, and does not have a fix\nin this series. It tests pushing a ref via the export\nmechanism to a new name on the remote side (i.e.,\n\"git push $remote old:new\").\n\nThe problem is that we push all of the work of generating\nthe export stream onto fast-export, but we have no way of\ncommunicating to fast-export that this name mapping is\nhappening. So we tell fast-export to generate a stream with\nthe commits for \"old\", but we can't tell it to label them\nall as \"new\".\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Culled the part of the commit message that is discussion of a\n possible implementation, but kept the parts describing the tests\n and why the last one is failing.\n\n t/t5800-remote-helpers.sh | 47 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 47 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex f6796e3..9db8ca8 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -85,4 +85,51 @@ test_expect_success 'pushing remote local repo' '\n \tcompare_refs clone HEAD server HEAD\n '\n \n+test_expect_failure 'fetch new branch' '\n+\t(cd public &&\n+\t git checkout -b new &&\n+\t echo content >>file &&\n+\t git commit -a -m five &&\n+\t git push origin new\n+\t) &&\n+\t(cd localclone &&\n+\t git fetch origin new\n+\t) &&\n+\tcompare_refs public HEAD localclone FETCH_HEAD\n+'\n+\n+test_expect_failure 'fetch multiple branches' '\n+\t(cd localclone &&\n+\t git fetch\n+\t) &&\n+\tcompare_refs server master localclone refs/remotes/origin/master &&\n+\tcompare_refs server new localclone refs/remotes/origin/new\n+'\n+\n+test_expect_failure 'push when remote has extra refs' '\n+\t(cd clone &&\n+\t echo content >>file &&\n+\t git commit -a -m six &&\n+\t git push\n+\t) &&\n+\tcompare_refs clone master server master\n+'\n+\n+test_expect_failure 'push new branch by name' '\n+\t(cd clone &&\n+\t git checkout -b new-name &&\n+\t echo content >>file &&\n+\t git commit -a -m seven &&\n+\t git push origin new-name\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-name\n+'\n+\n+test_expect_failure 'push new branch with old:new refspec' '\n+\t(cd clone &&\n+\t git push origin new-name:new-refspec\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-refspec\n+'\n+\n test_done\n-- \n1.7.5.1.292.g728120\n"},{"id":"170238","messageId":"1308496725-22329-6-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 05/20] teach remote-testgit to import non-HEAD refs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:30Z","receivedAt":"2011-06-19T15:18:30Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\nUpon receiving an \"import\" command, the testgit remote\nhelper would ignore the ref asked for by git and generate a\nfast-export stream based on HEAD. Instead, we should\nactually give git the ref it asked for.\n\nThis requires adding a new parameter to the export_repo\nmethod in the remote-helpers python library, which may be\nused by code outside of git.git. We use a default parameter\nso that callers without the new parameter will get the same\nbehavior as before.\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Fixed spurious appending of \"HEAD\" noticed by Junio.\n\n git-remote-testgit.py | 2 +-\n git_remote_helpers/git/exporter.py | 9 +++++++--\n t/t5800-remote-helpers.sh | 2 +-\n 3 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex df9d512..e4a99a3 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -122,7 +122,7 @@ def do_import(repo, args):\n die(\"Need gitdir to import\")\n \n repo = update_local_repo(repo)\n- repo.exporter.export_repo(repo.gitdir)\n+ repo.exporter.export_repo(repo.gitdir, args)\n \n \n def do_export(repo, args):\ndiff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py\nindex f40f9d6..bc39163 100644\n--- a/git_remote_helpers/git/exporter.py\n+++ b/git_remote_helpers/git/exporter.py\n@@ -15,7 +15,7 @@ class GitExporter(object):\n \n self.repo = repo\n \n- def export_repo(self, base):\n+ def export_repo(self, base, refs=None):\n \"\"\"Exports a fast-export stream for the given directory.\n \n Simply delegates to git fast-epxort and pipes it through sed\n@@ -23,8 +23,13 @@ class GitExporter(object):\n default refs/heads. This is to demonstrate how the export\n data can be stored under it's own ref (using the refspec\n capability).\n+\n+ If None, refs defaults to [\"HEAD\"].\n \"\"\"\n \n+ if not refs:\n+ refs = [\"HEAD\"]\n+\n dirname = self.repo.get_base_path(base)\n path = os.path.abspath(os.path.join(dirname, 'testgit.marks'))\n \n@@ -42,7 +47,7 @@ class GitExporter(object):\n if os.path.exists(path):\n args.append(\"--import-marks=\" + path)\n \n- args.append(\"HEAD\")\n+ args.extend(refs)\n \n p1 = subprocess.Popen(args, stdout=subprocess.PIPE)\n \ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex 9db8ca8..ca115cc 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -85,7 +85,7 @@ test_expect_success 'pushing remote local repo' '\n \tcompare_refs clone HEAD server HEAD\n '\n \n-test_expect_failure 'fetch new branch' '\n+test_expect_success 'fetch new branch' '\n \t(cd public &&\n \t git checkout -b new &&\n \t echo content >>file &&\n-- \n1.7.5.1.292.g728120\n"},{"id":"170239","messageId":"1308496725-22329-7-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 06/20] transport-helper: don't feed bogus refs to export push","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:31Z","receivedAt":"2011-06-19T15:18:31Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\nWhen we want to push to a remote helper that has the\n\"export\" capability, we collect all of the refs we want to\npush and then feed them to fast-export.\n\nHowever, the list of refs is actually a list of remote refs,\nnot local refs. The mapped local refs are included via the\npeer_ref pointer. So when we add an argument to our\nfast-export command line, we must be sure to use the local\npeer_ref name (and if there is no local name, it is because\nwe are not actually sending that ref, or we may not even\nhave the ref at all).\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n t/t5800-remote-helpers.sh | 2 +-\n transport-helper.c | 3 ++-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex ca115cc..ceb0010 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -106,7 +106,7 @@ test_expect_failure 'fetch multiple branches' '\n \tcompare_refs server new localclone refs/remotes/origin/new\n '\n \n-test_expect_failure 'push when remote has extra refs' '\n+test_expect_success 'push when remote has extra refs' '\n \t(cd clone &&\n \t echo content >>file &&\n \t git commit -a -m six &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b560b64..34d18aa 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -730,7 +730,8 @@ static int push_refs_with_export(struct transport *transport,\n \t\t}\n \t\tfree(private);\n \n-\t\tstring_list_append(&revlist_args, ref->name);\n+\t\tif (ref->peer_ref)\n+\t\t\tstring_list_append(&revlist_args, ref->peer_ref->name);\n \n \t}\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170237","messageId":"1308496725-22329-8-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 07/20] git_remote_helpers: push all refs during a non-local export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:32Z","receivedAt":"2011-06-19T15:18:32Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Jeff King \n\nWhen a remote helper exports to a non-local git repo, the\nsteps are roughly:\n\n 1. fast-export into a local staging area; the set of\n interesting refs is defined by what is in the fast-export\n stream\n\n 2. git push from the staging area to the non-local repo\n\nIn the second step, we should explicitly push all refs, not\njust matching ones. This will let us push refs that do not\nyet exist in the remote repo.\n\nThis does not deal with forced (not-fast-forward) pushes.\n\nSigned-off-by: Jeff King \nSigned-off-by: Sverre Rabbelier \n---\n\n Added last line to the commit message.\n\n git_remote_helpers/git/non_local.py | 2 +-\n t/t5800-remote-helpers.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py\nindex f27389b..c53e074 100644\n--- a/git_remote_helpers/git/non_local.py\n+++ b/git_remote_helpers/git/non_local.py\n@@ -63,7 +63,7 @@ class NonLocalGit(object):\n if not os.path.exists(path):\n die(\"could not find repo at %s\", path)\n \n- args = [\"git\", \"--git-dir=\" + path, \"push\", \"--quiet\", self.repo.gitpath]\n+ args = [\"git\", \"--git-dir=\" + path, \"push\", \"--quiet\", self.repo.gitpath, \"--all\"]\n child = subprocess.Popen(args)\n if child.wait() != 0:\n raise CalledProcessError\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex ceb0010..12f471c 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -115,7 +115,7 @@ test_expect_success 'push when remote has extra refs' '\n \tcompare_refs clone master server master\n '\n \n-test_expect_failure 'push new branch by name' '\n+test_expect_success 'push new branch by name' '\n \t(cd clone &&\n \t git checkout -b new-name &&\n \t echo content >>file &&\n-- \n1.7.5.1.292.g728120\n"},{"id":"170243","messageId":"1308496725-22329-9-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:33Z","receivedAt":"2011-06-19T15:18:33Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This went unnoticed because the transport helper infrastructore did\nnot check the return value of the helper, nor did the helper print\nanything before exiting.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n remote-curl.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 17d8a9b..7e8d50f 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -857,6 +857,8 @@ int main(int argc, const char **argv)\n \tdo {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n \t\t\tbreak;\n+\t\tif (buf.len == 0)\n+\t\t\tbreak;\n \t\tif (!prefixcmp(buf.buf, \"fetch \")) {\n \t\t\tif (nongit)\n \t\t\t\tdie(\"Fetch attempted without a local repo\");\n@@ -895,6 +897,7 @@ int main(int argc, const char **argv)\n \t\t\tprintf(\"\\n\");\n \t\t\tfflush(stdout);\n \t\t} else {\n+\t\t\tfprintf(stderr, \"Unknown command '%s'\\n\", buf.buf);\n \t\t\treturn 1;\n \t\t}\n \t\tstrbuf_reset(&buf);\n-- \n1.7.5.1.292.g728120\n"},{"id":"170240","messageId":"1308496725-22329-10-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 09/20] git-remote-testgit: only push for non-local repositories","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:34Z","receivedAt":"2011-06-19T15:18:34Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Trying to push for local repositories will fail since there is no\nlocal checkout in .git/info/... to push from as that is only used for\nnon-local repositories (local repositories are pushed to directly).\n\nThis went unnoticed because the transport helper infrastructure does\nnot check the return value of the helper.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n git-remote-testgit.py | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex e4a99a3..9658355 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -147,7 +147,9 @@ def do_export(repo, args):\n \n update_local_repo(repo)\n repo.importer.do_import(repo.gitdir)\n- repo.non_local.push(repo.gitdir)\n+\n+ if not repo.local:\n+ repo.non_local.push(repo.gitdir)\n \n \n def do_gitdir(repo, args):\n-- \n1.7.5.1.292.g728120\n"},{"id":"170242","messageId":"1308496725-22329-11-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 10/20] git-remote-testgit: fix error handling","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:35Z","receivedAt":"2011-06-19T15:18:35Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"If fast-export did not complete successfully the error handling code\nitself would error out.\n\nThis was broken in commit 23b093ee0 (Brandon Casey, Wed Jun 9 2010,\nRemove python 2.5'isms). Revert that commit an introduce our own copy\nof check_call in util.py instead.\n\nTested by changing 'if retcode' to 'if not retcode' temporarily.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n This was broken in many places, not just the one place I fixed it\n in the last version of this commit.\n\n git_remote_helpers/git/exporter.py | 6 +++---\n git_remote_helpers/git/importer.py | 6 +++---\n git_remote_helpers/git/non_local.py | 18 +++++-------------\n git_remote_helpers/git/repo.py | 7 ++++---\n git_remote_helpers/util.py | 32 ++++++++++++++++++++++++++++++++\n 5 files changed, 47 insertions(+), 22 deletions(-)\n\ndiff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py\nindex bc39163..9ee5f96 100644\n--- a/git_remote_helpers/git/exporter.py\n+++ b/git_remote_helpers/git/exporter.py\n@@ -2,6 +2,8 @@ import os\n import subprocess\n import sys\n \n+from git_remote_helpers.util import check_call\n+\n \n class GitExporter(object):\n \"\"\"An exporter for testgit repositories.\n@@ -53,6 +55,4 @@ class GitExporter(object):\n \n args = [\"sed\", \"s_refs/heads/_\" + self.repo.prefix + \"_g\"]\n \n- child = subprocess.Popen(args, stdin=p1.stdout)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ check_call(args, stdin=p1.stdout)\ndiff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py\nindex 70a7127..02a719a 100644\n--- a/git_remote_helpers/git/importer.py\n+++ b/git_remote_helpers/git/importer.py\n@@ -1,6 +1,8 @@\n import os\n import subprocess\n \n+from git_remote_helpers.util import check_call\n+\n \n class GitImporter(object):\n \"\"\"An importer for testgit repositories.\n@@ -35,6 +37,4 @@ class GitImporter(object):\n if os.path.exists(path):\n args.append(\"--import-marks=\" + path)\n \n- child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ check_call(args)\ndiff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py\nindex c53e074..e700250 100644\n--- a/git_remote_helpers/git/non_local.py\n+++ b/git_remote_helpers/git/non_local.py\n@@ -1,7 +1,7 @@\n import os\n import subprocess\n \n-from git_remote_helpers.util import die, warn\n+from git_remote_helpers.util import check_call, die, warn\n \n \n class NonLocalGit(object):\n@@ -29,9 +29,7 @@ class NonLocalGit(object):\n os.makedirs(path)\n args = [\"git\", \"clone\", \"--bare\", \"--quiet\", self.repo.gitpath, path]\n \n- child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ check_call(args)\n \n return path\n \n@@ -45,14 +43,10 @@ class NonLocalGit(object):\n die(\"could not find repo at %s\", path)\n \n args = [\"git\", \"--git-dir=\" + path, \"fetch\", \"--quiet\", self.repo.gitpath]\n- child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ check_call(args)\n \n args = [\"git\", \"--git-dir=\" + path, \"update-ref\", \"refs/heads/master\", \"FETCH_HEAD\"]\n- child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ child = check_call(args)\n \n def push(self, base):\n \"\"\"Pushes from the non-local repo to base.\n@@ -64,6 +58,4 @@ class NonLocalGit(object):\n die(\"could not find repo at %s\", path)\n \n args = [\"git\", \"--git-dir=\" + path, \"push\", \"--quiet\", self.repo.gitpath, \"--all\"]\n- child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ child = check_call(args)\ndiff --git a/git_remote_helpers/git/repo.py b/git_remote_helpers/git/repo.py\nindex 58e1cdb..acbf8d7 100644\n--- a/git_remote_helpers/git/repo.py\n+++ b/git_remote_helpers/git/repo.py\n@@ -1,6 +1,9 @@\n import os\n import subprocess\n \n+from git_remote_helpers.util import check_call\n+\n+\n def sanitize(rev, sep='\\t'):\n \"\"\"Converts a for-each-ref line to a name/value pair.\n \"\"\"\n@@ -53,9 +56,7 @@ class GitRepo(object):\n path = \".cached_revs\"\n ofile = open(path, \"w\")\n \n- child = subprocess.Popen(args, stdout=ofile)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ check_call(args, stdout=ofile)\n output = open(path).readlines()\n self.revmap = dict(sanitize(i) for i in output)\n if \"HEAD\" in self.revmap:\ndiff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py\nindex dce83e6..8b9f302 100644\n--- a/git_remote_helpers/util.py\n+++ b/git_remote_helpers/util.py\n@@ -128,6 +128,38 @@ def run_command (args, cwd = None, shell = False, add_env = None,\n return (exit_code, output, errors)\n \n \n+# from python2.7:subprocess.py\n+def call(*popenargs, **kwargs):\n+ \"\"\"Run command with arguments. Wait for command to complete, then\n+ return the returncode attribute.\n+\n+ The arguments are the same as for the Popen constructor. Example:\n+\n+ retcode = call([\"ls\", \"-l\"])\n+ \"\"\"\n+ return subprocess.Popen(*popenargs, **kwargs).wait()\n+\n+\n+# from python2.7:subprocess.py\n+def check_call(*popenargs, **kwargs):\n+ \"\"\"Run command with arguments. Wait for command to complete. If\n+ the exit code was zero then return, otherwise raise\n+ CalledProcessError. The CalledProcessError object will have the\n+ return code in the returncode attribute.\n+\n+ The arguments are the same as for the Popen constructor. Example:\n+\n+ check_call([\"ls\", \"-l\"])\n+ \"\"\"\n+ retcode = call(*popenargs, **kwargs)\n+ if retcode:\n+ cmd = kwargs.get(\"args\")\n+ if cmd is None:\n+ cmd = popenargs[0]\n+ raise subprocess.CalledProcessError(retcode, cmd)\n+ return 0\n+\n+\n def file_reader_method (missing_ok = False):\n \"\"\"Decorator for simplifying reading of files.\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170241","messageId":"1308496725-22329-12-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 11/20] fast-import: introduce 'done' command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:36Z","receivedAt":"2011-06-19T15:18:36Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Add a 'done' command that causes fast-import to stop reading from the\nstream and exit.\n\nIf the new --done command line flag was passed on the command line\n(or a \"feature done\" declaration included at the start of the stream),\nmake the 'done' command mandatory. So \"git fast-import --done\"'s\ninput format will be prefix-free, making errors easier to detect when\nthey show up as early termination at some convenient time of the\nupstream of a pipe writing to fast-import.\n\nAnother possible application of the 'done' command would to be allow a\nfast-import stream that is only a small part of a larger encapsulating\nstream to be easily parsed, leaving the file offset after the \"done\\n\"\nso the other application can pick up from there. This patch does not\nteach fast-import to do that --- fast-import still uses buffered input\n(stdio).\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n Documentation/git-fast-import.txt | 25 ++++++++++++++++++++++\n fast-import.c | 8 +++++++\n t/t9300-fast-import.sh | 42 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 75 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 249249a..0fc68a9 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -101,6 +101,12 @@ OPTIONS\n \twhen the `cat-blob` command is encountered in the stream.\n \tThe default behaviour is to write to `stdout`.\n \n+--done::\n+\tRequire a `done` command at the end of the stream.\n+\tThis option might be useful for detecting errors that\n+\tcause the frontend to terminate before it has started to\n+\twrite a stream.\n+\n --export-pack-edges=::\n \tAfter creating a packfile, print a line of data to\n \t listing the filename of the packfile and the last\n@@ -330,6 +336,11 @@ and control the current import process. More detailed discussion\n \tstandard output. This command is optional and is not needed\n \tto perform an import.\n \n+`done`::\n+\tMarks the end of the stream. This command is optional\n+\tunless the `done` feature was requested using the\n+\t`--done` command line option or `feature done` command.\n+\n `cat-blob`::\n \tCauses fast-import to print a blob in 'cat-file --batch'\n \tformat to the file descriptor set with `--cat-blob-fd` or\n@@ -1015,6 +1026,11 @@ notes::\n \tVersions of fast-import not supporting notes will exit\n \twith a message indicating so.\n \n+done::\n+\tError out if the stream ends without a 'done' command.\n+\tWithout this feature, errors causing the frontend to end\n+\tabruptly at a convenient point in the stream can go\n+\tundetected.\n \n `option`\n ~~~~~~~~\n@@ -1044,6 +1060,15 @@ not be passed as option:\n * cat-blob-fd\n * force\n \n+`done`\n+~~~~~~\n+If the `done` feature is not in use, treated as if EOF was read.\n+This can be used to tell fast-import to finish early.\n+\n+If the `--done` command line option or `feature done` command is\n+in use, the `done` command is mandatory and marks the end of the\n+stream.\n+\n Crash Reports\n -------------\n If fast-import is supplied invalid input it will terminate with a\ndiff --git a/fast-import.c b/fast-import.c\nindex 78d9786..8a8a915 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -354,6 +354,7 @@ static unsigned int cmd_save = 100;\n static uintmax_t next_mark;\n static struct strbuf new_data = STRBUF_INIT;\n static int seen_data_command;\n+static int require_explicit_termination;\n \n /* Signal handling */\n static volatile sig_atomic_t checkpoint_requested;\n@@ -3139,6 +3140,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\trelative_marks_paths = 1;\n \t} else if (!strcmp(feature, \"no-relative-marks\")) {\n \t\trelative_marks_paths = 0;\n+\t} else if (!strcmp(feature, \"done\")) {\n+\t\trequire_explicit_termination = 1;\n \t} else if (!strcmp(feature, \"force\")) {\n \t\tforce_update = 1;\n \t} else if (!strcmp(feature, \"notes\") || !strcmp(feature, \"ls\")) {\n@@ -3288,6 +3291,8 @@ int main(int argc, const char **argv)\n \t\t\tparse_reset_branch();\n \t\telse if (!strcmp(\"checkpoint\", command_buf.buf))\n \t\t\tparse_checkpoint();\n+\t\telse if (!strcmp(\"done\", command_buf.buf))\n+\t\t\tbreak;\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\n \t\t\tparse_progress();\n \t\telse if (!prefixcmp(command_buf.buf, \"feature \"))\n@@ -3307,6 +3312,9 @@ int main(int argc, const char **argv)\n \tif (!seen_data_command)\n \t\tparse_argv();\n \n+\tif (require_explicit_termination && feof(stdin))\n+\t\tdie(\"stream ends early\");\n+\n \tend_packfile();\n \n \tdump_branches();\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 6b1ba6c..526231b 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2197,6 +2197,48 @@ test_expect_success 'R: quiet option results in no stats being output' '\n test_cmp empty output\n '\n \n+test_expect_success 'R: feature done means terminating \"done\" is mandatory' '\n+\techo feature done | test_must_fail git fast-import &&\n+\ttest_must_fail git fast-import --done expect <<-\\EOF &&\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\thello.c\n+\t:000000 100644 OBJID OBJID A\thello2.c\n+\tEOF\n+\tgit fast-import <<-EOF &&\n+\tcommit refs/heads/done-ends\n+\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\tdata <actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >input <\n---\n\n Unchanged.\n\n Documentation/git-fast-export.txt | 4 ++++\n builtin/fast-export.c | 9 +++++++++\n 2 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 781bd6e..e3f8453 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -82,6 +82,10 @@ marks the same across runs.\n \tallow that. So fake a tagger to be able to fast-import the\n \toutput.\n \n+--use-done-feature::\n+\tStart the stream with a 'feature done' stanza, and terminate\n+\tit with a 'done' command.\n+\n --no-data::\n \tSkip output of blob objects and instead refer to blobs via\n \ttheir original SHA-1 hash. This is useful when rewriting the\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex daf1945..becef85 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -26,6 +26,7 @@ static int progress;\n static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;\n static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;\n static int fake_missing_tagger;\n+static int use_done_feature;\n static int no_data;\n static int full_tree;\n \n@@ -627,6 +628,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t\t \"Fake a tagger when tags lack one\"),\n \t\tOPT_BOOLEAN(0, \"full-tree\", &full_tree,\n \t\t\t \"Output full tree for each commit\"),\n+\t\tOPT_BOOLEAN(0, \"use-done-feature\", &use_done_feature,\n+\t\t\t \"Use the done feature to terminate the stream\"),\n \t\t{ OPTION_NEGBIT, 0, \"data\", &no_data, NULL,\n \t\t\t\"Skip output of blob data\",\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },\n@@ -648,6 +651,9 @@ 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 (use_done_feature)\n+\t\tprintf(\"feature done\\n\");\n+\n \tif (import_filename)\n \t\timport_marks(import_filename);\n \n@@ -675,5 +681,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (export_filename)\n \t\texport_marks(export_filename);\n \n+\tif (use_done_feature)\n+\t\tprintf(\"done\\n\");\n+\n \treturn 0;\n }\n-- \n1.7.5.1.292.g728120\n"},{"id":"170248","messageId":"1308496725-22329-14-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 13/20] transport-helper: factor out push_update_refs_status","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:38Z","receivedAt":"2011-06-19T15:18:38Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"The update ref status part of push is useful for the export command\nas well, factor it out into it's own function.\n\nAlso factor out push_update_ref_status to avoid a long loop without\nan explicit condition with a non-trivial body.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n transport-helper.c | 153 ++++++++++++++++++++++++++++-----------------------\n 1 files changed, 84 insertions(+), 69 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 34d18aa..ecb44f6 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -554,6 +554,88 @@ static int fetch(struct transport *transport,\n \treturn -1;\n }\n \n+static void push_update_ref_status(struct strbuf *buf,\n+\t\t\t\t struct ref **ref,\n+\t\t\t\t struct ref *remote_refs)\n+{\n+\tchar *refname, *msg;\n+\tint status;\n+\n+\tif (!prefixcmp(buf->buf, \"ok \")) {\n+\t\tstatus = REF_STATUS_OK;\n+\t\trefname = buf->buf + 3;\n+\t} else if (!prefixcmp(buf->buf, \"error \")) {\n+\t\tstatus = REF_STATUS_REMOTE_REJECT;\n+\t\trefname = buf->buf + 6;\n+\t} else\n+\t\tdie(\"expected ok/error, helper said '%s'\\n\", buf->buf);\n+\n+\tmsg = strchr(refname, ' ');\n+\tif (msg) {\n+\t\tstruct strbuf msg_buf = STRBUF_INIT;\n+\t\tconst char *end;\n+\n+\t\t*msg++ = '\\0';\n+\t\tif (!unquote_c_style(&msg_buf, msg, &end))\n+\t\t\tmsg = strbuf_detach(&msg_buf, NULL);\n+\t\telse\n+\t\t\tmsg = xstrdup(msg);\n+\t\tstrbuf_release(&msg_buf);\n+\n+\t\tif (!strcmp(msg, \"no match\")) {\n+\t\t\tstatus = REF_STATUS_NONE;\n+\t\t\tfree(msg);\n+\t\t\tmsg = NULL;\n+\t\t}\n+\t\telse if (!strcmp(msg, \"up to date\")) {\n+\t\t\tstatus = REF_STATUS_UPTODATE;\n+\t\t\tfree(msg);\n+\t\t\tmsg = NULL;\n+\t\t}\n+\t\telse if (!strcmp(msg, \"non-fast forward\")) {\n+\t\t\tstatus = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\t\tfree(msg);\n+\t\t\tmsg = NULL;\n+\t\t}\n+\t}\n+\n+\tif (*ref)\n+\t\t*ref = find_ref_by_name(*ref, refname);\n+\tif (!*ref)\n+\t\t*ref = find_ref_by_name(remote_refs, refname);\n+\tif (!*ref) {\n+\t\twarning(\"helper reported unexpected status of %s\", refname);\n+\t\treturn;\n+\t}\n+\n+\tif ((*ref)->status != REF_STATUS_NONE) {\n+\t\t/*\n+\t\t * Earlier, the ref was marked not to be pushed, so ignore the ref\n+\t\t * status reported by the remote helper if the latter is 'no match'.\n+\t\t */\n+\t\tif (status == REF_STATUS_NONE)\n+\t\t\treturn;\n+\t}\n+\n+\t(*ref)->status = status;\n+\t(*ref)->remote_status = msg;\n+}\n+\n+static void push_update_refs_status(struct helper_data *data,\n+\t\t\t\t struct ref *remote_refs)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct ref *ref = remote_refs;\n+\tfor (;;) {\n+\t\trecvline(data, &buf);\n+\t\tif (!buf.len)\n+\t\t\tbreak;\n+\n+\t\tpush_update_ref_status(&buf, &ref, remote_refs);\n+\t}\n+\tstrbuf_release(&buf);\n+}\n+\n static int push_refs_with_push(struct transport *transport,\n \t\tstruct ref *remote_refs, int flags)\n {\n@@ -608,76 +690,9 @@ static int push_refs_with_push(struct transport *transport,\n \n \tstrbuf_addch(&buf, '\\n');\n \tsendline(data, &buf);\n-\n-\tref = remote_refs;\n-\twhile (1) {\n-\t\tchar *refname, *msg;\n-\t\tint status;\n-\n-\t\trecvline(data, &buf);\n-\t\tif (!buf.len)\n-\t\t\tbreak;\n-\n-\t\tif (!prefixcmp(buf.buf, \"ok \")) {\n-\t\t\tstatus = REF_STATUS_OK;\n-\t\t\trefname = buf.buf + 3;\n-\t\t} else if (!prefixcmp(buf.buf, \"error \")) {\n-\t\t\tstatus = REF_STATUS_REMOTE_REJECT;\n-\t\t\trefname = buf.buf + 6;\n-\t\t} else\n-\t\t\tdie(\"expected ok/error, helper said '%s'\\n\", buf.buf);\n-\n-\t\tmsg = strchr(refname, ' ');\n-\t\tif (msg) {\n-\t\t\tstruct strbuf msg_buf = STRBUF_INIT;\n-\t\t\tconst char *end;\n-\n-\t\t\t*msg++ = '\\0';\n-\t\t\tif (!unquote_c_style(&msg_buf, msg, &end))\n-\t\t\t\tmsg = strbuf_detach(&msg_buf, NULL);\n-\t\t\telse\n-\t\t\t\tmsg = xstrdup(msg);\n-\t\t\tstrbuf_release(&msg_buf);\n-\n-\t\t\tif (!strcmp(msg, \"no match\")) {\n-\t\t\t\tstatus = REF_STATUS_NONE;\n-\t\t\t\tfree(msg);\n-\t\t\t\tmsg = NULL;\n-\t\t\t}\n-\t\t\telse if (!strcmp(msg, \"up to date\")) {\n-\t\t\t\tstatus = REF_STATUS_UPTODATE;\n-\t\t\t\tfree(msg);\n-\t\t\t\tmsg = NULL;\n-\t\t\t}\n-\t\t\telse if (!strcmp(msg, \"non-fast forward\")) {\n-\t\t\t\tstatus = REF_STATUS_REJECT_NONFASTFORWARD;\n-\t\t\t\tfree(msg);\n-\t\t\t\tmsg = NULL;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (ref)\n-\t\t\tref = find_ref_by_name(ref, refname);\n-\t\tif (!ref)\n-\t\t\tref = find_ref_by_name(remote_refs, refname);\n-\t\tif (!ref) {\n-\t\t\twarning(\"helper reported unexpected status of %s\", refname);\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (ref->status != REF_STATUS_NONE) {\n-\t\t\t/*\n-\t\t\t * Earlier, the ref was marked not to be pushed, so ignore the ref\n-\t\t\t * status reported by the remote helper if the latter is 'no match'.\n-\t\t\t */\n-\t\t\tif (status == REF_STATUS_NONE)\n-\t\t\t\tcontinue;\n-\t\t}\n-\n-\t\tref->status = status;\n-\t\tref->remote_status = msg;\n-\t}\n \tstrbuf_release(&buf);\n+\n+\tpush_update_refs_status(data, remote_refs);\n \treturn 0;\n }\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170249","messageId":"1308496725-22329-15-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 14/20] transport-helper: check status code of finish_command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:39Z","receivedAt":"2011-06-19T15:18:39Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Previously the status code of all helpers were ignored, allowing\nerrors that occur to go unnoticed if the error text output by the\nhelper is not noticed.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n transport-helper.c | 23 +++++++++++++++--------\n 1 files changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex ecb44f6..7f3c6c6 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -204,6 +204,7 @@ static int disconnect_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tint res = 0;\n \n \tif (data->helper) {\n \t\tif (debug)\n@@ -215,13 +216,13 @@ static int disconnect_helper(struct transport *transport)\n \t\tclose(data->helper->in);\n \t\tclose(data->helper->out);\n \t\tfclose(data->out);\n-\t\tfinish_command(data->helper);\n+\t\tres = finish_command(data->helper);\n \t\tfree((char *)data->helper->argv[0]);\n \t\tfree(data->helper->argv);\n \t\tfree(data->helper);\n \t\tdata->helper = NULL;\n \t}\n-\treturn 0;\n+\treturn res;\n }\n \n static const char *unsupported_options[] = {\n@@ -299,12 +300,13 @@ static void standard_options(struct transport *t)\n \n static int release_helper(struct transport *transport)\n {\n+\tint res = 0;\n \tstruct helper_data *data = transport->data;\n \tfree_refspec(data->refspec_nr, data->refspecs);\n \tdata->refspecs = NULL;\n-\tdisconnect_helper(transport);\n+\tres = disconnect_helper(transport);\n \tfree(transport->data);\n-\treturn 0;\n+\treturn res;\n }\n \n static int fetch_with_fetch(struct transport *transport,\n@@ -410,8 +412,11 @@ static int fetch_with_import(struct transport *transport,\n \t\tsendline(data, &buf);\n \t\tstrbuf_reset(&buf);\n \t}\n-\tdisconnect_helper(transport);\n-\tfinish_command(&fastimport);\n+\tif (disconnect_helper(transport))\n+\t\tdie(\"Error while disconnecting helper\");\n+\tif (finish_command(&fastimport))\n+\t\tdie(\"Error while running fast-import\");\n+\n \tfree(fastimport.argv);\n \tfastimport.argv = NULL;\n \n@@ -755,8 +760,10 @@ static int push_refs_with_export(struct transport *transport,\n \t\tdie(\"Couldn't run fast-export\");\n \n \tdata->no_disconnect_req = 1;\n-\tfinish_command(&exporter);\n-\tdisconnect_helper(transport);\n+\tif (finish_command(&exporter))\n+\t\tdie(\"Error while running fast-export\");\n+\tif (disconnect_helper(transport))\n+\t\tdie(\"Error while disconnecting helper\");\n \treturn 0;\n }\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170246","messageId":"1308496725-22329-16-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 15/20] transport-helper: use the new done feature where possible","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:40Z","receivedAt":"2011-06-19T15:18:40Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"In other words, use fast-export --use-done-feature to add a 'done'\ncommand at the end of streams passed to remote helpers' \"import\"\ncommands, and teach the remote helpers implementing \"export\" to use\nthe 'done' command in turn when producing their streams.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n git-remote-testgit.py | 2 ++\n transport-helper.c | 8 ++------\n 2 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex 9658355..a8e47d9 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -124,6 +124,8 @@ def do_import(repo, args):\n repo = update_local_repo(repo)\n repo.exporter.export_repo(repo.gitdir, args)\n \n+ print \"done\"\n+\n \n def do_export(repo, args):\n \"\"\"Imports a fast-import stream from git to testgit.\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 7f3c6c6..b0361c2 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -375,8 +375,9 @@ static int get_exporter(struct transport *transport,\n \t/* we need to duplicate helper->in because we want to use it after\n \t * fastexport is done with it. */\n \tfastexport->out = dup(helper->in);\n-\tfastexport->argv = xcalloc(4 + revlist_args->nr, sizeof(*fastexport->argv));\n+\tfastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv));\n \tfastexport->argv[argc++] = \"fast-export\";\n+\tfastexport->argv[argc++] = \"--use-done-feature\";\n \tif (export_marks)\n \t\tfastexport->argv[argc++] = export_marks;\n \tif (import_marks)\n@@ -412,11 +413,8 @@ static int fetch_with_import(struct transport *transport,\n \t\tsendline(data, &buf);\n \t\tstrbuf_reset(&buf);\n \t}\n-\tif (disconnect_helper(transport))\n-\t\tdie(\"Error while disconnecting helper\");\n \tif (finish_command(&fastimport))\n \t\tdie(\"Error while running fast-import\");\n-\n \tfree(fastimport.argv);\n \tfastimport.argv = NULL;\n \n@@ -762,8 +760,6 @@ static int push_refs_with_export(struct transport *transport,\n \tdata->no_disconnect_req = 1;\n \tif (finish_command(&exporter))\n \t\tdie(\"Error while running fast-export\");\n-\tif (disconnect_helper(transport))\n-\t\tdie(\"Error while disconnecting helper\");\n \treturn 0;\n }\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170245","messageId":"1308496725-22329-17-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 16/20] transport-helper: update ref status after push with export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:41Z","receivedAt":"2011-06-19T15:18:41Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"\nSigned-off-by: Sverre Rabbelier \n---\n\n This time a proper implementation of this patch.\n\n git-remote-testgit.py | 6 +++++-\n git_remote_helpers/git/importer.py | 28 +++++++++++++++++++++++++++-\n git_remote_helpers/util.py | 34 ++++++++++++++++++++++++++++++++++\n transport-helper.c | 1 +\n 4 files changed, 67 insertions(+), 2 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex a8e47d9..854e27e 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -148,11 +148,15 @@ def do_export(repo, args):\n sys.stdout.flush()\n \n update_local_repo(repo)\n- repo.importer.do_import(repo.gitdir)\n+ changed = repo.importer.do_import(repo.gitdir)\n \n if not repo.local:\n repo.non_local.push(repo.gitdir)\n \n+ for ref in changed:\n+ print \"ok %s\" % ref\n+ print\n+\n \n def do_gitdir(repo, args):\n \"\"\"Stores the location of the gitdir.\ndiff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py\nindex 02a719a..5c6b595 100644\n--- a/git_remote_helpers/git/importer.py\n+++ b/git_remote_helpers/git/importer.py\n@@ -1,7 +1,7 @@\n import os\n import subprocess\n \n-from git_remote_helpers.util import check_call\n+from git_remote_helpers.util import check_call, check_output\n \n \n class GitImporter(object):\n@@ -16,6 +16,18 @@ class GitImporter(object):\n \n self.repo = repo\n \n+ def get_refs(self, gitdir):\n+ \"\"\"Returns a dictionary with refs.\n+ \"\"\"\n+ args = [\"git\", \"--git-dir=\" + gitdir, \"for-each-ref\", \"refs/heads\"]\n+ lines = check_output(args).strip().split('\\n')\n+ refs = {}\n+ for line in lines:\n+ value, name = line.split(' ')\n+ name = name.strip('commit\\t')\n+ refs[name] = value\n+ return refs\n+\n def do_import(self, base):\n \"\"\"Imports a fast-import stream to the given directory.\n \n@@ -32,9 +44,23 @@ class GitImporter(object):\n if not os.path.exists(dirname):\n os.makedirs(dirname)\n \n+ refs_before = self.get_refs(gitdir)\n+\n args = [\"git\", \"--git-dir=\" + gitdir, \"fast-import\", \"--quiet\", \"--export-marks=\" + path]\n \n if os.path.exists(path):\n args.append(\"--import-marks=\" + path)\n \n check_call(args)\n+\n+ refs_after = self.get_refs(gitdir)\n+\n+ changed = {}\n+\n+ for name, value in refs_after.iteritems():\n+ if refs_before.get(name) == value:\n+ continue\n+\n+ changed[name] = value\n+\n+ return changed\ndiff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py\nindex 8b9f302..6a9ec5c 100644\n--- a/git_remote_helpers/util.py\n+++ b/git_remote_helpers/util.py\n@@ -160,6 +160,40 @@ def check_call(*popenargs, **kwargs):\n return 0\n \n \n+# from python2.7:subprocess.py\n+def check_output(*popenargs, **kwargs):\n+ r\"\"\"Run command with arguments and return its output as a byte string.\n+\n+ If the exit code was non-zero it raises a CalledProcessError. The\n+ CalledProcessError object will have the return code in the returncode\n+ attribute and output in the output attribute.\n+\n+ The arguments are the same as for the Popen constructor. Example:\n+\n+ >>> check_output([\"ls\", \"-l\", \"/dev/null\"])\n+ 'crw-rw-rw- 1 root root 1, 3 Oct 18 2007 /dev/null\\n'\n+\n+ The stdout argument is not allowed as it is used internally.\n+ To capture standard error in the result, use stderr=STDOUT.\n+\n+ >>> check_output([\"/bin/sh\", \"-c\",\n+ ... \"ls -l non_existent_file ; exit 0\"],\n+ ... stderr=STDOUT)\n+ 'ls: non_existent_file: No such file or directory\\n'\n+ \"\"\"\n+ if 'stdout' in kwargs:\n+ raise ValueError('stdout argument not allowed, it will be overridden.')\n+ process = subprocess.Popen(stdout=subprocess.PIPE, *popenargs, **kwargs)\n+ output, unused_err = process.communicate()\n+ retcode = process.poll()\n+ if retcode:\n+ cmd = kwargs.get(\"args\")\n+ if cmd is None:\n+ cmd = popenargs[0]\n+ raise subprocess.CalledProcessError(retcode, cmd)\n+ return output\n+\n+\n def file_reader_method (missing_ok = False):\n \"\"\"Decorator for simplifying reading of files.\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex b0361c2..bb1b97f 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -760,6 +760,7 @@ static int push_refs_with_export(struct transport *transport,\n \tdata->no_disconnect_req = 1;\n \tif (finish_command(&exporter))\n \t\tdie(\"Error while running fast-export\");\n+\tpush_update_refs_status(data, remote_refs);\n \treturn 0;\n }\n \n-- \n1.7.5.1.292.g728120\n"},{"id":"170247","messageId":"1308496725-22329-18-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 17/20] transport-helper: change import semantics","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:42Z","receivedAt":"2011-06-19T15:18:42Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Currently the helper must somehow guess how many import statements to\nread before it starts outputting its fast-export stream. This is\nbecause the remote helper infrastructure runs fast-import only once,\nso the helper is forced to output one stream for all import commands\nit will receive. The only reason this worked in the past was because\nonly one ref was imported at a time.\n\nChange the semantics of the import statement such that it matches\nthat of the list statement. That is, 'import\\n' is followed by a list\nof refs that should be exported, followed by '\\n'.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n git-remote-testgit.py | 14 +++++++++++---\n t/t5800-remote-helpers.sh | 2 +-\n transport-helper.c | 7 ++++++-\n 3 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex 854e27e..4939853 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -115,14 +115,22 @@ def do_import(repo, args):\n \"\"\"Exports a fast-import stream from testgit for git to import.\n \"\"\"\n \n- if len(args) != 1:\n- die(\"Import needs exactly one ref\")\n+ if args:\n+ die(\"Import expects its ref seperately\")\n \n if not repo.gitdir:\n die(\"Need gitdir to import\")\n \n+ refs = []\n+\n+ while True:\n+ line = sys.stdin.readline()\n+ if line == '\\n':\n+ break\n+ refs.append(line.strip())\n+\n repo = update_local_repo(repo)\n- repo.exporter.export_repo(repo.gitdir, args)\n+ repo.exporter.export_repo(repo.gitdir, refs)\n \n print \"done\"\n \ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex 12f471c..1c62001 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -98,7 +98,7 @@ test_expect_success 'fetch new branch' '\n \tcompare_refs public HEAD localclone FETCH_HEAD\n '\n \n-test_expect_failure 'fetch multiple branches' '\n+test_expect_success 'fetch multiple branches' '\n \t(cd localclone &&\n \t git fetch\n \t) &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex bb1b97f..c089928 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -404,15 +404,20 @@ static int fetch_with_import(struct transport *transport,\n \tif (get_importer(transport, &fastimport))\n \t\tdie(\"Couldn't run fast-import\");\n \n+\twrite_constant(data->helper->in, \"import\\n\");\n+\n \tfor (i = 0; i < nr_heads; i++) {\n \t\tposn = to_fetch[i];\n \t\tif (posn->status & REF_STATUS_UPTODATE)\n \t\t\tcontinue;\n \n-\t\tstrbuf_addf(&buf, \"import %s\\n\", posn->name);\n+\t\tstrbuf_addf(&buf, \"%s\\n\", posn->name);\n \t\tsendline(data, &buf);\n \t\tstrbuf_reset(&buf);\n \t}\n+\n+\twrite_constant(data->helper->in, \"\\n\");\n+\n \tif (finish_command(&fastimport))\n \t\tdie(\"Error while running fast-import\");\n \tfree(fastimport.argv);\n-- \n1.7.5.1.292.g728120\n"},{"id":"170251","messageId":"1308496725-22329-19-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 18/20] transport-helper: export is no longer always the last command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:43Z","receivedAt":"2011-06-19T15:18:43Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"The trailing \\n in the protocol signals the helper that the\nconnection is about to close, allowing it to do whatever cleanup\nneccesary.\n\nPreviously, the connection would already be closed by the\ntime the trailing \\n was to be written. Now that the remote-helper\nprotocol uses the new done command in its fast-import streams, this\nis no longer the case and we can safely write the trailing \\n.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Updated commit message.\n\n transport-helper.c | 1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex c089928..f78b670 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -762,7 +762,6 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\t export_marks, import_marks, &revlist_args))\n \t\tdie(\"Couldn't run fast-export\");\n \n-\tdata->no_disconnect_req = 1;\n \tif (finish_command(&exporter))\n \t\tdie(\"Error while running fast-export\");\n \tpush_update_refs_status(data, remote_refs);\n-- \n1.7.5.1.292.g728120\n"},{"id":"170250","messageId":"1308496725-22329-20-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 19/20] transport-helper: Use capname for gitdir capability too","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:44Z","receivedAt":"2011-06-19T15:18:44Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Also properly use capname in the refspec capability.\n\nPreviously the gitdir and refspec capabilities could not be listed as\nrequired or their parsing would break.\n\nMost likely the reason the second hunk wasn't caught is because the\nseries that added 'gitdir' as capability, and the one that added\nrequired capabilities were done in parallel.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Updated commit message.\n\n transport-helper.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex f78b670..ddf0ffa 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -171,10 +171,10 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tALLOC_GROW(refspecs,\n \t\t\t\t refspec_nr + 1,\n \t\t\t\t refspec_alloc);\n-\t\t\trefspecs[refspec_nr++] = strdup(buf.buf + strlen(\"refspec \"));\n+\t\t\trefspecs[refspec_nr++] = strdup(capname + strlen(\"refspec \"));\n \t\t} else if (!strcmp(capname, \"connect\")) {\n \t\t\tdata->connect = 1;\n-\t\t} else if (!strcmp(buf.buf, \"gitdir\")) {\n+\t\t} else if (!strcmp(capname, \"gitdir\")) {\n \t\t\tstruct strbuf gitdir = STRBUF_INIT;\n \t\t\tstrbuf_addf(&gitdir, \"gitdir %s\\n\", get_git_dir());\n \t\t\tsendline(data, &gitdir);\n-- \n1.7.5.1.292.g728120\n"},{"id":"170252","messageId":"1308496725-22329-21-git-send-email-srabbelier@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 20/20] transport-helper: implement marks location as capability","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-19T15:18:45Z","receivedAt":"2011-06-19T15:18:45Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"While this requires the helper to flush stdout after listing 'gitdir'\nas capability, and read a command (the 'gitdir' response from the\nremote helper infrastructure) right after that, this is more elegant\nand does not require an ad-hoc exchange of values.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged.\n\n git-remote-testgit.py | 31 ++++++++++++++++++-------------\n transport-helper.c | 47 ++++++++++++++++++-----------------------------\n 2 files changed, 36 insertions(+), 42 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex 4939853..30f775e 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -71,8 +71,26 @@ def do_capabilities(repo, args):\n print \"import\"\n print \"export\"\n print \"gitdir\"\n+\n+ sys.stdout.flush()\n+ if not read_one_line(repo):\n+ die(\"Expected gitdir, got empty line\")\n+ if not repo.gitdir:\n+ die(\"Expected gitdir, got something else\")\n+\n print \"refspec refs/heads/*:%s*\" % repo.prefix\n \n+ dirname = repo.get_base_path(repo.gitdir)\n+\n+ if not os.path.exists(dirname):\n+ os.makedirs(dirname)\n+\n+ path = os.path.join(dirname, 'testgit.marks')\n+\n+ print \"*export-marks %s\" % path\n+ if os.path.exists(path):\n+ print \"*import-marks %s\" % path\n+\n print # end capabilities\n \n \n@@ -142,19 +160,6 @@ def do_export(repo, args):\n if not repo.gitdir:\n die(\"Need gitdir to export\")\n \n- dirname = repo.get_base_path(repo.gitdir)\n-\n- if not os.path.exists(dirname):\n- os.makedirs(dirname)\n-\n- path = os.path.join(dirname, 'testgit.marks')\n- print path\n- if os.path.exists(path):\n- print path\n- else:\n- print \"\"\n- sys.stdout.flush()\n-\n update_local_repo(repo)\n changed = repo.importer.do_import(repo.gitdir)\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex ddf0ffa..12f08a1 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -23,6 +23,8 @@ struct helper_data {\n \t\tpush : 1,\n \t\tconnect : 1,\n \t\tno_disconnect_req : 1;\n+\tchar *export_marks;\n+\tchar *import_marks;\n \t/* These go from remote name (as in \"list\") to private name */\n \tstruct refspec *refspecs;\n \tint refspec_nr;\n@@ -179,6 +181,16 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tstrbuf_addf(&gitdir, \"gitdir %s\\n\", get_git_dir());\n \t\t\tsendline(data, &gitdir);\n \t\t\tstrbuf_release(&gitdir);\n+\t\t} else if (!prefixcmp(capname, \"export-marks \")) {\n+\t\t\tstruct strbuf arg = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&arg, \"--export-marks=\");\n+\t\t\tstrbuf_addstr(&arg, capname + strlen(\"export-marks \"));\n+\t\t\tdata->export_marks = strbuf_detach(&arg, NULL);\n+\t\t} else if (!prefixcmp(capname, \"import-marks\")) {\n+\t\t\tstruct strbuf arg = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&arg, \"--import-marks=\");\n+\t\t\tstrbuf_addstr(&arg, capname + strlen(\"import-marks \"));\n+\t\t\tdata->import_marks = strbuf_detach(&arg, NULL);\n \t\t} else if (mandatory) {\n \t\t\tdie(\"Unknown mandatory capability %s. This remote \"\n \t\t\t \"helper probably needs newer version of Git.\\n\",\n@@ -364,10 +376,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti\n \n static int get_exporter(struct transport *transport,\n \t\t\tstruct child_process *fastexport,\n-\t\t\tconst char *export_marks,\n-\t\t\tconst char *import_marks,\n \t\t\tstruct string_list *revlist_args)\n {\n+\tstruct helper_data *data = transport->data;\n \tstruct child_process *helper = get_helper(transport);\n \tint argc = 0, i;\n \tmemset(fastexport, 0, sizeof(*fastexport));\n@@ -378,10 +389,10 @@ static int get_exporter(struct transport *transport,\n \tfastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv));\n \tfastexport->argv[argc++] = \"fast-export\";\n \tfastexport->argv[argc++] = \"--use-done-feature\";\n-\tif (export_marks)\n-\t\tfastexport->argv[argc++] = export_marks;\n-\tif (import_marks)\n-\t\tfastexport->argv[argc++] = import_marks;\n+\tif (data->export_marks)\n+\t\tfastexport->argv[argc++] = data->export_marks;\n+\tif (data->import_marks)\n+\t\tfastexport->argv[argc++] = data->import_marks;\n \n \tfor (i = 0; i < revlist_args->nr; i++)\n \t\tfastexport->argv[argc++] = revlist_args->items[i].string;\n@@ -710,7 +721,6 @@ 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-\tchar *export_marks = NULL, *import_marks = NULL;\n \tstruct string_list revlist_args = STRING_LIST_INIT_NODUP;\n \tstruct strbuf buf = STRBUF_INIT;\n \n@@ -718,26 +728,6 @@ static int push_refs_with_export(struct transport *transport,\n \n \twrite_constant(helper->in, \"export\\n\");\n \n-\trecvline(data, &buf);\n-\tif (debug)\n-\t\tfprintf(stderr, \"Debug: Got export_marks '%s'\\n\", buf.buf);\n-\tif (buf.len) {\n-\t\tstruct strbuf arg = STRBUF_INIT;\n-\t\tstrbuf_addstr(&arg, \"--export-marks=\");\n-\t\tstrbuf_addbuf(&arg, &buf);\n-\t\texport_marks = strbuf_detach(&arg, NULL);\n-\t}\n-\n-\trecvline(data, &buf);\n-\tif (debug)\n-\t\tfprintf(stderr, \"Debug: Got import_marks '%s'\\n\", buf.buf);\n-\tif (buf.len) {\n-\t\tstruct strbuf arg = STRBUF_INIT;\n-\t\tstrbuf_addstr(&arg, \"--import-marks=\");\n-\t\tstrbuf_addbuf(&arg, &buf);\n-\t\timport_marks = strbuf_detach(&arg, NULL);\n-\t}\n-\n \tstrbuf_reset(&buf);\n \n \tfor (ref = remote_refs; ref; ref = ref->next) {\n@@ -758,8 +748,7 @@ static int push_refs_with_export(struct transport *transport,\n \n \t}\n \n-\tif (get_exporter(transport, &exporter,\n-\t\t\t export_marks, import_marks, &revlist_args))\n+\tif (get_exporter(transport, &exporter, &revlist_args))\n \t\tdie(\"Couldn't run fast-export\");\n \n \tif (finish_command(&exporter))\n-- \n1.7.5.1.292.g728120\n"},{"id":"170258","messageId":"20110619220246.GE23893@elie","threadId":"27656","inReplyTo":"1308496725-22329-5-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 04/20] t5800: document some non-functional parts of remote helpers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-19T22:02:46Z","receivedAt":"2011-06-19T22:02:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Culled the part of the commit message that is discussion of a\n> possible implementation, but kept the parts describing the tests\n> and why the last one is failing.\n\nLooks good. What happened to the extra tests (e.g., push tag)\nmentioned in the last round?\n"},{"id":"170265","messageId":"20110619224216.GF23893@elie","threadId":"27656","inReplyTo":"1308496725-22329-9-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-19T22:42:16Z","receivedAt":"2011-06-19T22:42:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -857,6 +857,8 @@ int main(int argc, const char **argv)\n> \tdo {\n> \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n> \t\t\tbreak;\n> +\t\tif (buf.len == 0)\n> +\t\t\tbreak;\n\nThanks. I wonder if that first \"if\" should be something like\n\n\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n\t\tif (ferror(stdin))\n\t\t\tfprintf(stderr, \"Error reading command stream\\n\");\n\t\telse\n\t\t\tfprintf(stderr, \"Unexpected end of command stream\\n\");\n\t\treturn 1;\n\t}\n\nto catch I/O errors (e.g., the transport-helper exiting early).\n"},{"id":"170268","messageId":"20110619225810.GG23893@elie","threadId":"27656","inReplyTo":"1308496725-22329-11-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 10/20] git-remote-testgit: fix error handling","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-19T22:58:10Z","receivedAt":"2011-06-19T22:58:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSverre Rabbelier wrote:\n\n> If fast-export did not complete successfully the error handling code\n> itself would error out.\n\nThat sounds like a problem indeed. What error message does it produce?\n\n> This was broken in commit 23b093ee0 (Brandon Casey, Wed Jun 9 2010,\n> Remove python 2.5'isms). Revert that commit an introduce our own copy\n> of check_call in util.py instead.\n\nThe \"an\" should probably be \"and\". More importantly, it's probably\nworth mentioning that this is only a partial revert or rephrasing some\nother way.\n\nHave you checked if this still works with python 2.4? Cc-ing Brandon\nin case he has advice.\n\n[...]\n> +++ b/git_remote_helpers/git/exporter.py\n[...]\n> @@ -53,6 +55,4 @@ class GitExporter(object):\n> \n> args = [\"sed\", \"s_refs/heads/_\" + self.repo.prefix + \"_g\"]\n> \n> - child = subprocess.Popen(args, stdin=p1.stdout)\n> - if child.wait() != 0:\n> - raise CalledProcessError\n> + check_call(args, stdin=p1.stdout)\n[...]\n> +++ b/git_remote_helpers/util.py\n> @@ -128,6 +128,38 @@ def run_command (args, cwd = None, shell = False, add_env = None,\n> return (exit_code, output, errors)\n> \n> \n> +# from python2.7:subprocess.py\n> +def call(*popenargs, **kwargs):\n[...]\n> + return subprocess.Popen(*popenargs, **kwargs).wait()\n> +\n> +\n> +# from python2.7:subprocess.py\n> +def check_call(*popenargs, **kwargs):\n[...]\n> + retcode = call(*popenargs, **kwargs)\n> + if retcode:\n> + cmd = kwargs.get(\"args\")\n> + if cmd is None:\n> + cmd = popenargs[0]\n> + raise subprocess.CalledProcessError(retcode, cmd)\n> + return 0\n\nSo IIUC the existing code is not providing arguments to the\nCalledProcessError constructor, and your patch fixes that. Good.\n\nBased on , Python 2.4 doesn't\nseem to have a CalledProcessError type. Maybe these code paths\nweren't being exercised before.\n\nRegards,\nJonathan\n"},{"id":"170269","messageId":"20110619232525.GH23893@elie","threadId":"27656","inReplyTo":"1308496725-22329-17-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 16/20] transport-helper: update ref status after push with export","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-19T23:25:25Z","receivedAt":"2011-06-19T23:25:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n>\n> Signed-off-by: Sverre Rabbelier \n> ---\n>\n> This time a proper implementation of this patch.\n\nNice. :)\n\nI assume this still does not return \"error \" responses,\ninstead disconnecting the helper on error, which is fine but probably\nworth mentioning.\n\n> --- a/git-remote-testgit.py\n> +++ b/git-remote-testgit.py\n> @@ -148,11 +148,15 @@ def do_export(repo, args):\n> sys.stdout.flush()\n> \n> update_local_repo(repo)\n> - repo.importer.do_import(repo.gitdir)\n> + changed = repo.importer.do_import(repo.gitdir)\n> \n> if not repo.local:\n> repo.non_local.push(repo.gitdir)\n> \n> + for ref in changed:\n> + print \"ok %s\" % ref\n> + print\n> +\n\nThe remote-helpers(1) manual says, concerning \"push\":\n\n\tWhen the push is complete, outputs one or more ok or\n\terror ? lines to indicate success or failure of\n\teach pushed ref. The status report output is terminated by a\n\tblank line. The option field may be quoted in a C style\n\tstring if it contains an LF.\n\nIdeally we would want \"export\" to also print all refs intended for\nexport, even if they did not actually change, but there's no obvious\nway to get that information. In the \"not repo.local\" case, seeing\nwhich refs changed in .git/info/fast-import/.git like you do\nhere seems like a reasonable enough heuristic.\n\nWhat happens if no ref changed (the \"already up to date\" case)?\n"},{"id":"170270","messageId":"20110619233822.GJ23893@elie","threadId":"27656","inReplyTo":"1308496725-22329-18-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 17/20] transport-helper: change import semantics","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-19T23:38:23Z","receivedAt":"2011-06-19T23:38:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Change the semantics of the import statement such that it matches\n> that of the list statement. That is, 'import\\n' is followed by a list\n> of refs that should be exported, followed by '\\n'.\n\nI think that for learnability, it would be better to use\n\n\timport \n\timport \n\t...\n\t[blank line]\n\nsince that is consistent with \"fetch\" and \"push\". I'll try mocking up\na patch for that tonight.\n"},{"id":"170275","messageId":"20110620012937.GB25836@elie","threadId":"27656","inReplyTo":"1308496725-22329-21-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 20/20] transport-helper: implement marks location as capability","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-20T01:29:37Z","receivedAt":"2011-06-20T01:29:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> [Subject: transport-helper: implement marks location as capability]\n>\n> While this requires the helper to flush stdout after listing 'gitdir'\n> as capability, and read a command (the 'gitdir' response from the\n> remote helper infrastructure) right after that, this is more elegant\n> and does not require an ad-hoc exchange of values.\n\nProbably I'm not thinking straight, but it's not obvious how the subject\nconnects to the content. I suppose the pieces I'm missing are that:\n\n - this patch introduces new undocumented \"import-marks\" and\n \"export-marks\" capabilities that take a pathname as parameter;\n\n - for remote-testgit, that pathname is based on $GIT_DIR, so to\n retrieve that value we flush stdout and read the gitdir immediately\n rather than hoping the gitdir arrives on stdin in time for it to\n be used later.\n\nBut that still leaves a couple of other questions unanswered:\n\nWhy do we use a \"gitdir\" capability for this at all, instead of\nexporting the GIT_DIR environment variable as Tomas has suggested?\n(Not about this patch, but a separate patch explaining that in the\ndocumentation would be nice.)\n\nHow does this interact with fast-import's --relative-marks feature\n(if at all)?\n\n[...]\n> +++ b/transport-helper.c\n[...]\n> @@ -718,26 +728,6 @@ static int push_refs_with_export(struct transport *transport,\n> \n> \twrite_constant(helper->in, \"export\\n\");\n> \n> -\trecvline(data, &buf);\n> -\tif (debug)\n> -\t\tfprintf(stderr, \"Debug: Got export_marks '%s'\\n\", buf.buf);\n> -\tif (buf.len) {\n> -\t\tstruct strbuf arg = STRBUF_INIT;\n> -\t\tstrbuf_addstr(&arg, \"--export-marks=\");\n> -\t\tstrbuf_addbuf(&arg, &buf);\n> -\t\texport_marks = strbuf_detach(&arg, NULL);\n> -\t}\n> -\n> -\trecvline(data, &buf);\n> -\tif (debug)\n> -\t\tfprintf(stderr, \"Debug: Got import_marks '%s'\\n\", buf.buf);\n\nHm, it seems there is a silent (but good) change in the behavior of\nthe \"export\" command, too.\n\nExcept where noted already, the patches look good, and the series\nseems to make the remote helper infrastructure a little saner.\nThanks.\n"},{"id":"170276","messageId":"1308537351-23209-1-git-send-email-divanorama@gmail.com","threadId":"27656","inReplyTo":"1308496725-22329-9-git-send-email-srabbelier@gmail.com","subject":"[PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-06-20T02:35:51Z","receivedAt":"2011-06-20T02:35:51Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"As a next step, won't it be better to use \"exit\\n\" or \"quit\\n\"\nas a terminator? I don't see a convention of terminating on a\nblank line in docs, only on EOF. Also I can imagine a blank\nline being read in a case of communication error, I mean a\nhelper can miss the error and see \"exit normally\" terminator.\n"},{"id":"170286","messageId":"20110620075541.GA28282@elie","threadId":"27656","inReplyTo":"1308537351-23209-1-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-20T07:55:41Z","receivedAt":"2011-06-20T07:55:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDmitry Ivankov wrote:\n\n> As a next step, won't it be better to use \"exit\\n\" or \"quit\\n\"\n> as a terminator?\n\nIf we were starting over, a \"done\" command (or \"quit\" or some other\nself-descriptive name) would be a no-brainer. Unfortunately, there\nare existing remote helpers out there --- based on a web search:\n\n - mediawiki\n - ccnx\n - couch\n - cvs\n - bzr\n - hg\n\nPresumably most already ignore a final blank line and do not check for\n\"quit\".\n\nOne possibility would be to emit \"done\" instead of a blank line if the\nremote helper has declared a \"done\" capability. But what does it buy?\n\nSide note: a \"done\" capability doesn't sound like a bad idea, though,\nfor another reason. The transport-helper could tell fast-import to\nexpect a \"done\" command at the end when importing from a remote helper\ndeclaring it, to catch situations in which the pipe prematurely closes\n(for example, because the remote helper has segfaulted).\n\n> I don't see a convention of terminating on a\n> blank line in docs,\n\nYes, this would be nice to document.\n\n> only on EOF. Also I can imagine a blank\n> line being read in a case of communication error\n\nA spurious NL, NL, EOF sequence does not sound likely to me. If the\ncommand stream is passing through a noisy channel, there are worse\ncorruptions to worry about (e.g., fetching to the wrong ref).\n"},{"id":"170293","messageId":"20110620114556.GA29818@elie","threadId":"27656","inReplyTo":"1308496725-22329-16-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 15/20] transport-helper: use the new done feature where possible","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-20T11:45:56Z","receivedAt":"2011-06-20T11:45:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSverre Rabbelier wrote:\n\n> In other words, use fast-export --use-done-feature to add a 'done'\n> command at the end of streams passed to remote helpers' \"import\"\n> commands, and teach the remote helpers implementing \"export\" to use\n> the 'done' command in turn when producing their streams.\n\nSounds like a reasonable thing to do. One puzzle:\n\n[...]\n> +++ b/transport-helper.c\n> @@ -375,8 +375,9 @@ static int get_exporter(struct transport *transport,\n> \t/* we need to duplicate helper->in because we want to use it after\n> \t * fastexport is done with it. */\n> \tfastexport->out = dup(helper->in);\n> -\tfastexport->argv = xcalloc(4 + revlist_args->nr, sizeof(*fastexport->argv));\n> +\tfastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv));\n> \tfastexport->argv[argc++] = \"fast-export\";\n> +\tfastexport->argv[argc++] = \"--use-done-feature\";\n> \tif (export_marks)\n> \t\tfastexport->argv[argc++] = export_marks;\n> \tif (import_marks)\n> @@ -412,11 +413,8 @@ static int fetch_with_import(struct transport *transport,\n> \t\tsendline(data, &buf);\n> \t\tstrbuf_reset(&buf);\n> \t}\n> -\tif (disconnect_helper(transport))\n> -\t\tdie(\"Error while disconnecting helper\");\n> \tif (finish_command(&fastimport))\n> \t\tdie(\"Error while running fast-import\");\n> -\n\nWhat is this change about? Is the plan to allow other commands after\na fetch_with_import? Sounds reasonable; I think it should be\nadvertised in the log message, though.\n\nWhen does the disconnect_helper call happen (to avoid leaks)? Ah, in\nrelease_helper; phew.\n\nThe disconnect_helper call writes the blank line that terminates the\nlist of \"import %s\" commands to start the import, so there would need\nto be a\n\n\tstrbuf_reset(&buf);\n\tstrbuf_addf(&buf, \"\\n\");\n\tsendline(data, &buf);\n\nin its place.\n"},{"id":"170310","messageId":"jfzVoEhsVKgfY8Vp692jfTk1lNp2lhg9JCJc8MiY6N6th3PzBJoHzw@cipher.nrlssc.navy.mil","threadId":"27656","inReplyTo":"20110619225810.GG23893@elie","subject":"Re: [PATCH v2 10/20] git-remote-testgit: fix error handling","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2011-06-20T17:50:39Z","receivedAt":"2011-06-20T17:50:39Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 06/19/2011 05:58 PM, Jonathan Nieder wrote:\n\n> Sverre Rabbelier wrote:\n> \n> Have you checked if this still works with python 2.4? Cc-ing Brandon\n> in case he has advice.\n> \n\n> Based on , Python 2.4 doesn't\n> seem to have a CalledProcessError type. Maybe these code paths\n> weren't being exercised before.\n\nWell, I don't see anywhere where CalledProccessError is actually caught.\ni.e. I don't see\n\n except subprocess.CalledProcessError:\n\nanywhere. So, on python 2.5+ if this exception is ever raised, the\nscript would just exit and produce a backtrace right? And the last\nline would look something like this:\n\n File \"XXX\", line XX, in check_call\n raise subprocess.CalledProcessError(retcode, cmd)\n subprocess.CalledProcessError: Command '['XXX', 'YYY', 'ZZZ']' returned non-zero exit status 1\n\nOn python 2.4, it would also exit and produce a backtrace that\nlooks like this:\n\n File \"test.py\", line 11, in check_call\n raise subprocess.CalledProcessError(retcode, cmd)\n AttributeError: 'module' object has no attribute 'CalledProcessError'\n\nSo, maybe it would work on python 2.4, but for the wrong reasons.\nSince the CalledProcessError is never caught, couldn't we just\nprint an error message and exit(1) within Sverre's check_call\nimplementation?\n\nBtw. the only reason I submitted those changes to support python 2.4\nwas because RHEL 5.X ships with python 2.4, and the changes were not\ntoo intrusive. So, it should be considered whether supporting 2.4\nis desirable. I wouldn't want to increase the maintenance burden\non the real python developers (i.e. not me).\n\nAlso, sorry about introducing this bug. I'm definitely no python guru.\n\n-Brandon\n"},{"id":"170311","messageId":"BANLkTimW8paPUHoYLpB_diY_s0L4aQCcgQ@mail.gmail.com","threadId":"27656","inReplyTo":"jfzVoEhsVKgfY8Vp692jfTk1lNp2lhg9JCJc8MiY6N6th3PzBJoHzw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 10/20] git-remote-testgit: fix error handling","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-20T18:02:10Z","receivedAt":"2011-06-20T18:02:10Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 19:50, Brandon Casey\n wrote:\n> Well, I don't see anywhere where CalledProccessError is actually caught.\n> i.e. I don't see\n>\n>   except subprocess.CalledProcessError:\n>\n> anywhere.  So, on python 2.5+ if this exception is ever raised, the\n> script would just exit and produce a backtrace right?\n\nWe're not trying to catch it, but we raise it (since that's what the\npython 2.5 implementation of check_call does). But if it's not defined\nin python 2.4, then my patch broke 2.4 again :).\n\n> On python 2.4, it would also exit and produce a backtrace that\n> looks like this:\n>\n>     File \"test.py\", line 11, in check_call\n>       raise subprocess.CalledProcessError(retcode, cmd)\n>   AttributeError: 'module' object has no attribute 'CalledProcessError'\n\nYeah, it would, my bad :(. I'll just define a dummy CalledProcessError\nif it isn't defined to support python 2.4\n\n> Btw. the only reason I submitted those changes to support python 2.4\n> was because RHEL 5.X ships with python 2.4, and the changes were not\n> too intrusive.  So, it should be considered whether supporting 2.4\n> is desirable.  I wouldn't want to increase the maintenance burden\n> on the real python developers (i.e. not me).\n\nWikipedia says RHEL 5.x will ready End of Production 1 on Q4 2011, but\nRHEL 6.x has been out only a little over half a year, so it's probably\nworth doing.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170317","messageId":"7v4o3kcmup.fsf@alter.siamese.dyndns.org","threadId":"27656","inReplyTo":"20110620075541.GA28282@elie","subject":"Re: [PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-20T19:41:02Z","receivedAt":"2011-06-20T19:41:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Side note: a \"done\" capability doesn't sound like a bad idea, though,\n> for another reason. The transport-helper could tell fast-import to\n> expect a \"done\" command at the end when importing from a remote helper\n> declaring it, to catch situations in which the pipe prematurely closes\n> (for example, because the remote helper has segfaulted).\n>\n>> I don't see a convention of terminating on a\n>> blank line in docs,\n>\n> Yes, this would be nice to document.\n>\n>> only on EOF. Also I can imagine a blank\n>> line being read in a case of communication error\n>\n> A spurious NL, NL, EOF sequence does not sound likely to me. If the\n> command stream is passing through a noisy channel, there are worse\n> corruptions to worry about (e.g., fetching to the wrong ref).\n\nI think everything you said in this message makes sense, especially the\npart about \"premature EOF detection\".\n\nThe spurious LF is an issue if you are trying to drive the backend by hand\nfor testing, but otherwise probably not.\n"},{"id":"170318","messageId":"7vzklcb7sc.fsf@alter.siamese.dyndns.org","threadId":"27656","inReplyTo":"20110620114556.GA29818@elie","subject":"Re: [PATCH v2 15/20] transport-helper: use the new done feature where possible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-20T19:51:47Z","receivedAt":"2011-06-20T19:51:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n>> @@ -412,11 +413,8 @@ static int fetch_with_import(struct transport *transport,\n>> \t\tsendline(data, &buf);\n>> \t\tstrbuf_reset(&buf);\n>> \t}\n>> -\tif (disconnect_helper(transport))\n>> -\t\tdie(\"Error while disconnecting helper\");\n>> \tif (finish_command(&fastimport))\n>> \t\tdie(\"Error while running fast-import\");\n>> -\n>\n> What is this change about? Is the plan to allow other commands after\n> a fetch_with_import? Sounds reasonable; I think it should be\n> advertised in the log message, though.\n>\n> When does the disconnect_helper call happen (to avoid leaks)? Ah, in\n> release_helper; phew.\n>\n> The disconnect_helper call writes the blank line that terminates the\n> list of \"import %s\" commands to start the import, so there would need\n> to be a\n>\n> \tstrbuf_reset(&buf);\n> \tstrbuf_addf(&buf, \"\\n\");\n> \tsendline(data, &buf);\n>\n> in its place.\n\nAll sensible suggestions for a potential reroll.\n"},{"id":"170421","messageId":"7vpqm76jd1.fsf@alter.siamese.dyndns.org","threadId":"27656","inReplyTo":"1308496725-22329-17-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v2 16/20] transport-helper: update ref status after push with export","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-21T20:05:14Z","receivedAt":"2011-06-21T20:05:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> + changed = {}\n> +\n> + for name, value in refs_after.iteritems():\n> + if refs_before.get(name) == value:\n> + continue\n> +\n> + changed[name] = value\n\nThis assumes that nobody will ever _remove_ refs. I think it is a sensible\nassumption but somebody might want to give a power to the fast-import stream\nto say \"I do not want that ref anymore\".\n\nI dunno; it is not a big deal to me either way. Just something to think\nabout.\n"},{"id":"170423","messageId":"BANLkTinXKCxFoPOuFHqy0XTpd6Jco+J=YQ@mail.gmail.com","threadId":"27656","inReplyTo":"7vpqm76jd1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 16/20] transport-helper: update ref status after push with export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-21T20:11:12Z","receivedAt":"2011-06-21T20:11:12Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Jun 21, 2011 at 22:05, Junio C Hamano wrote:\n> This assumes that nobody will ever _remove_ refs. I think it is a sensible\n> assumption but somebody might want to give a power to the fast-import stream\n> to say \"I do not want that ref anymore\".\n>\n> I dunno; it is not a big deal to me either way.  Just something to think\n> about.\n\nI'll have a look at addressing your and Jonathan's comments (the case\nof 'already up to date' and 'refs removed').\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170813","messageId":"CAGdFq_iaGRnfkf=2vxDe--E3oP1CFCvDarkhKJxdgNzGq+KqcA@mail.gmail.com","threadId":"27656","inReplyTo":"20110619220246.GE23893@elie","subject":"Re: [PATCH v2 04/20] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T11:19:00Z","receivedAt":"2011-07-04T11:19:00Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 00:02, Jonathan Nieder wrote:\n> Looks good.  What happened to the extra tests (e.g., push tag)\n> mentioned in the last round?\n\nDo you think they should be added as test_expect_fail targets? I\ndidn't add them because they didn't work, but I suppose it might make\nsense to at least document these missing features.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170814","messageId":"CAGdFq_iTojuvVAdB6sC7-Kro9E49q_1Lfh87Yy4nR5BTdZ1eaA@mail.gmail.com","threadId":"27656","inReplyTo":"20110619233822.GJ23893@elie","subject":"Re: [PATCH v2 17/20] transport-helper: change import semantics","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T11:20:20Z","receivedAt":"2011-07-04T11:20:20Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 01:38, Jonathan Nieder wrote:\n> I think that for learnability, it would be better to use\n>\n>        import \n>        import \n>        ...\n>        [blank line]\n>\n> since that is consistent with \"fetch\" and \"push\".  I'll try mocking up\n> a patch for that tonight.\n\nThat would mean the only change compared to the current behavior is\nthe additional blank line, yes?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170816","messageId":"CAGdFq_gknRaWzZjOOZyE9WvgTLBGADDLb_WcxE3vfN+uRBztGw@mail.gmail.com","threadId":"27656","inReplyTo":"20110620114556.GA29818@elie","subject":"Re: [PATCH v2 15/20] transport-helper: use the new done feature where possible","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T13:37:41Z","receivedAt":"2011-07-04T13:37:41Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 13:45, Jonathan Nieder wrote:\n> What is this change about?  Is the plan to allow other commands after\n> a fetch_with_import?  Sounds reasonable; I think it should be\n> advertised in the log message, though.\n\nYeah, the order/granularity of these commits is perhaps suboptimal.\nThe actual changes is by now explained in \"export is no longer always\nthe last command\" and \"change import semantics\". Perhaps they should\njust be squashed together?\n\n> The disconnect_helper call writes the blank line that terminates the\n> list of \"import %s\" commands to start the import, so there would need\n\nTerminating the list of import commands isn't needed until the \"change\nimport semantics\" commit, in which a write_constant(data->helper->in,\n\"\\n\") is added.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170817","messageId":"CAGdFq_ijB6fNr7favL+J7WeHOvY+MyqgZu-vwfPY3NRZg5mPag@mail.gmail.com","threadId":"27656","inReplyTo":"20110620012937.GB25836@elie","subject":"Re: [PATCH v2 20/20] transport-helper: implement marks location as capability","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T13:43:53Z","receivedAt":"2011-07-04T13:43:53Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 03:29, Jonathan Nieder wrote:\n> Why do we use a \"gitdir\" capability for this at all, instead of\n> exporting the GIT_DIR environment variable as Tomas has suggested?\n> (Not about this patch, but a separate patch explaining that in the\n> documentation would be nice.)\n\nMainly because I didn't take the time to implement that, will do.\n\n> How does this interact with fast-import's --relative-marks feature\n> (if at all)?\n\nThey're a supplement to it, the exporter uses all three features (see\ngit_remote_helpers/git/exporter.py).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170818","messageId":"CAGdFq_ioz+MW8SuWUZAyeia979odTHGEsmyMrc00o1ZwbywUbA@mail.gmail.com","threadId":"27656","inReplyTo":"20110619224216.GF23893@elie","subject":"Re: [PATCH v2 08/20] remote-curl: accept empty line as terminator","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T14:11:22Z","receivedAt":"2011-07-04T14:11:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 20, 2011 at 00:42, Jonathan Nieder wrote:\n> Thanks.  I wonder if that first \"if\" should be something like\n>\n>        if (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n>                if (ferror(stdin))\n>                        fprintf(stderr, \"Error reading command stream\\n\");\n>                else\n>                        fprintf(stderr, \"Unexpected end of command stream\\n\");\n>                return 1;\n>        }\n>\n> to catch I/O errors (e.g., the transport-helper exiting early).\n\nGood idea, done. Tests still pass too ;).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170823","messageId":"20110704215859.GA8909@elie","threadId":"27656","inReplyTo":"CAGdFq_iTojuvVAdB6sC7-Kro9E49q_1Lfh87Yy4nR5BTdZ1eaA@mail.gmail.com","subject":"Re: [PATCH v2 17/20] transport-helper: change import semantics","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-07-04T21:58:59Z","receivedAt":"2011-07-04T21:58:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Mon, Jun 20, 2011 at 01:38, Jonathan Nieder wrote:\n\n>> I think that for learnability, it would be better to use\n>>\n>>        import \n>>        import \n>>        ...\n>>        [blank line]\n>>\n>> since that is consistent with \"fetch\" and \"push\".  I'll try mocking up\n>> a patch for that tonight.\n>\n> That would mean the only change compared to the current behavior is\n> the additional blank line, yes?\n\nIt would mean no change compared to the current behavior. :)\ndisconnect_helper() writes the blank line.\n\nThat's why no patch came; sorry, I should have mentioned so.\n"},{"id":"170824","messageId":"CAGdFq_hFQRTjRZBGZKBejpUqCCzo+wHWG69nM8=57peUBN9yjw@mail.gmail.com","threadId":"27656","inReplyTo":"20110704215859.GA8909@elie","subject":"Re: [PATCH v2 17/20] transport-helper: change import semantics","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-04T22:23:14Z","receivedAt":"2011-07-04T22:23:14Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jul 4, 2011 at 23:58, Jonathan Nieder wrote:\n> It would mean no change compared to the current behavior. :)\n> disconnect_helper() writes the blank line.\n>\n> That's why no patch came; sorry, I should have mentioned so.\n\nWith the only difference that currently that \\n signifies the end of\nthe entire stream, whereas now it will mean the end of the imports?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"170825","messageId":"20110704223759.GC8909@elie","threadId":"27656","inReplyTo":"CAGdFq_hFQRTjRZBGZKBejpUqCCzo+wHWG69nM8=57peUBN9yjw@mail.gmail.com","subject":"Re: [PATCH v2 17/20] transport-helper: change import semantics","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-07-04T22:37:59Z","receivedAt":"2011-07-04T22:37:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Mon, Jul 4, 2011 at 23:58, Jonathan Nieder wrote:\n\n>> It would mean no change compared to the current behavior. :)\n[...]\n> With the only difference that currently that \\n signifies the end of\n> the entire stream, whereas now it will mean the end of the imports?\n\nExactly. The git-remote-testgit part of the patch would be pretty\nsimilar to what you have and could even go earlier in the series if\nwanted (since it does not require transport-helper changes), while the\ntransport-helper part should be unnecessary as long as patch 15/20\n\"use the new done feature where possible\" does not remove the \\n in\nits output (as discussed in its subthread).\n\nMeanwhile I should probably implement a \"capability done\" as Dmitry\nhinted, to make blank lines no longer act as a terminator as a way of\nhelping people debugging by hand that hit too many times by\nmistake.\n"}]}