{"thread":{"id":"27583","subject":"[PATCH 00/19] remote-helper improvements","startedAt":"2011-06-08T18:48:31Z","lastAt":"2011-06-13T15:24:03Z","messageCount":54,"participants":["Sverre Rabbelier","Jonathan Nieder","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":19},"messages":[{"id":"169659","messageId":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":null,"subject":"[PATCH 00/19] remote-helper improvements","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:31Z","receivedAt":"2011-06-08T18:48:31Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This is a re-roll of Peff's recent series [0] and the series [1] I sent out\nwell over half a year ago. My series replaces two from Peff's.\n\n[0] http://thread.gmane.org/gmane.comp.version-control.git/174904\n[1] http://thread.gmane.org/gmane.comp.version-control.git/154669\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 (13):\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 | 54 ++++++---\n git_remote_helpers/git/exporter.py | 8 +-\n git_remote_helpers/git/importer.py | 5 +-\n git_remote_helpers/git/non_local.py | 2 +-\n remote-curl.c | 3 +\n t/t5800-remote-helpers.sh | 59 ++++++++-\n t/t9300-fast-import.sh | 42 ++++++\n transport-helper.c | 238 ++++++++++++++++++----------------\n 12 files changed, 320 insertions(+), 137 deletions(-)\n"},{"id":"169661","messageId":"1307558930-16074-2-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 01/19] transport-helper: fix minor leak in push_refs_with_export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:32Z","receivedAt":"2011-06-08T18:48:32Z","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: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\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":"169673","messageId":"1307558930-16074-3-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 02/19] t5800: factor out some ref tests","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:33Z","receivedAt":"2011-06-08T18:48:33Z","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: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\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":"169678","messageId":"1307558930-16074-4-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:34Z","receivedAt":"2011-06-08T18:48:34Z","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 new refs without us.\nThis could impact git users who use remote helpers to access\nforeign 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\nThere are two possible solutions:\n\n 1. Indicate the mapping to fast-export, so that it can\n generate the \"mapped\" names. Unfortunately, this is\n somewhat difficult due to the way fast-import is\n implemented. It feeds its revision parameters to the\n regular rev-walking machinery, asking the revision code\n to mark the \"source\" of each commit. So if fast-export\n sees that we want commits from \"refs/heads/old\", but\n also that we want to call commits from old as\n \"refs/heads/new\", it can't distinguish between the\n case of \"I want _only_ new, using the commits of old\"\n and \"I want _both_ old and new, which happen to point\n to the same commits\".\n\n And it's important to distinguish those cases, because\n in one, the remote will update master, and in the\n other, it will not.\n\n This isn't an insurmountable obstacle, but it's going\n to mean either fast-export has to take on more of the\n revision argument parsing and revwalking\n responsibility, or the revision walker's show_source\n code will have to learn about mapping names.\n\n 2. Indicate the mapping to the remote helper, who can then\n apply the mapping to the results of the export. Right\n now the export remote helper command has no arguments;\n it simply says \"hey, I'm going to dump a fast-export\n stream on you\". And the remote-helper looks through the\n results of the stream to decide which refs are being\n pushed. This could be extended instead to something\n like:\n\n export refs/heads/foo\n export refs/heads/old:refs/heads/new\n \n\n which would allow the remote helper to map names as\n appropriate. This a reasonably elegant solution; the\n downside is that it breaks compatibility with existing\n remote helpers.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\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 3a37ad0..a10a48d 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -81,4 +81,51 @@ test_expect_success PYTHON_24 'pushing remote local repo' '\n \tcompare_refs clone HEAD server HEAD\n '\n \n+test_expect_failure PYTHON_24 '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 PYTHON_24 '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 PYTHON_24 '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 PYTHON_24 '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 PYTHON_24 '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":"169660","messageId":"1307558930-16074-5-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:35Z","receivedAt":"2011-06-08T18:48:35Z","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: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\n\n git-remote-testgit.py | 2 +-\n git_remote_helpers/git/exporter.py | 8 +++++++-\n t/t5800-remote-helpers.sh | 2 +-\n 3 files changed, 9 insertions(+), 3 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..8fd83c7 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@@ -38,6 +43,7 @@ class GitExporter(object):\n sys.stdout.flush()\n \n args = [\"git\", \"--git-dir=\" + self.repo.gitpath, \"fast-export\", \"--export-marks=\" + path]\n+ args.extend(refs)\n \n if os.path.exists(path):\n args.append(\"--import-marks=\" + path)\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex a10a48d..562edf4 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -81,7 +81,7 @@ test_expect_success PYTHON_24 'pushing remote local repo' '\n \tcompare_refs clone HEAD server HEAD\n '\n \n-test_expect_failure PYTHON_24 'fetch new branch' '\n+test_expect_success PYTHON_24 '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":"169671","messageId":"1307558930-16074-6-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 05/19] transport-helper: don't feed bogus refs to export push","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:36Z","receivedAt":"2011-06-08T18:48:36Z","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: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\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 562edf4..1abfa5c 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -102,7 +102,7 @@ test_expect_failure PYTHON_24 'fetch multiple branches' '\n \tcompare_refs server new localclone refs/remotes/origin/new\n '\n \n-test_expect_failure PYTHON_24 'push when remote has extra refs' '\n+test_expect_success PYTHON_24 '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":"169663","messageId":"1307558930-16074-7-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:37Z","receivedAt":"2011-06-08T18:48:37Z","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\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged from Peff's series.\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 1abfa5c..a4afe38 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -111,7 +111,7 @@ test_expect_success PYTHON_24 'push when remote has extra refs' '\n \tcompare_refs clone master server master\n '\n \n-test_expect_failure PYTHON_24 'push new branch by name' '\n+test_expect_success PYTHON_24 '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":"169662","messageId":"1307558930-16074-8-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 07/19] remote-curl: accept empty line as terminator","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:38Z","receivedAt":"2011-06-08T18:48:38Z","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 Moved to the front for bisectability.\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":"169666","messageId":"1307558930-16074-9-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 08/19] git-remote-testgit: only push for non-local repositories","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:39Z","receivedAt":"2011-06-08T18:48:39Z","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 Moved to the front for bisectability.\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":"169668","messageId":"1307558930-16074-10-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 09/19] git-remote-testgit: fix error handling","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:40Z","receivedAt":"2011-06-08T18:48:40Z","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\nSigned-off-by: Sverre Rabbelier \n---\n\n Moved to the front for bisectability.\n\n git_remote_helpers/git/importer.py | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py\nindex 70a7127..d938611 100644\n--- a/git_remote_helpers/git/importer.py\n+++ b/git_remote_helpers/git/importer.py\n@@ -36,5 +36,6 @@ class GitImporter(object):\n args.append(\"--import-marks=\" + path)\n \n child = subprocess.Popen(args)\n- if child.wait() != 0:\n- raise CalledProcessError\n+ ret = child.wait()\n+ if ret != 0:\n+ raise subprocess.CalledProcessError(ret, args)\n-- \n1.7.5.1.292.g728120\n"},{"id":"169669","messageId":"1307558930-16074-11-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 10/19] fast-import: introduce 'done' command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:41Z","receivedAt":"2011-06-08T18:48:41Z","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 This is actually Jonathan's version that makes done required if\n feature done was specified. See also:\n\n\thttp://mid.gmane.org/20110213094212.GA25435@elie\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 from my original series.\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":"169664","messageId":"1307558930-16074-13-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 12/19] transport-helper: factor out push_update_refs_status","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:43Z","receivedAt":"2011-06-08T18:48:43Z","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 Changed from the original addressing comments by Jonathan.\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":"169665","messageId":"1307558930-16074-14-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 13/19] transport-helper: check status code of finish_command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:44Z","receivedAt":"2011-06-08T18:48:44Z","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 Changed from the original only to address style nits.\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":"169676","messageId":"1307558930-16074-15-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 14/19] transport-helper: use the new done feature where possible","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:45Z","receivedAt":"2011-06-08T18:48:45Z","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 from my original series.\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":"169677","messageId":"1307558930-16074-16-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 15/19] transport-helper: update ref status after push with export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:46Z","receivedAt":"2011-06-08T18:48:46Z","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 Obviously the testgit helper shouldn't just print 'ok' for master,\n but it demonstrates the idea.\n\n Jonathan added for those who wondered what it should do:\n\n When the push is complete, outputs one or more ok or\n error ? lines to indicate success or failure of\n each pushed ref. The status report output is terminated by a\n blank line. The option field may be quoted in a C style\n string if it contains an LF.\n\n So testgit should be getting this information from the\n result of non_local.push(). Another example is if a ref is a\n non-fast-forward, it should probably detect that before even\n exporting it, and report that here.\n\n git-remote-testgit.py | 3 +++\n transport-helper.c | 1 +\n 2 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex a8e47d9..47a30da 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -153,6 +153,9 @@ def do_export(repo, args):\n if not repo.local:\n repo.non_local.push(repo.gitdir)\n \n+ print \"ok refs/heads/master\"\n+ print\n+\n \n def do_gitdir(repo, args):\n \"\"\"Stores the location of the gitdir.\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":"169670","messageId":"1307558930-16074-17-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 16/19] transport-helper: change import semantics","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:47Z","receivedAt":"2011-06-08T18:48:47Z","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 from my original series. Replaces Peff's workaround from\n his series.\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 47a30da..88bcb20 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 a4afe38..682f813 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -94,7 +94,7 @@ test_expect_success PYTHON_24 'fetch new branch' '\n \tcompare_refs public HEAD localclone FETCH_HEAD\n '\n \n-test_expect_failure PYTHON_24 'fetch multiple branches' '\n+test_expect_success PYTHON_24 '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":"169674","messageId":"1307558930-16074-18-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:48Z","receivedAt":"2011-06-08T18:48:48Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Now that the remote helper protocol uses the new done command in its\nfast-import streams, export no longer needs to be the last command in\nthe stream.\n\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged from my original series, but with commit message updated\n after review.\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":"169672","messageId":"1307558930-16074-19-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 18/19] transport-helper: Use capname for gitdir capability too","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:49Z","receivedAt":"2011-06-08T18:48:49Z","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\nSigned-off-by: Sverre Rabbelier \n---\n\n Unchanged from my original series.\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":"169675","messageId":"1307558930-16074-20-git-send-email-srabbelier@gmail.com","threadId":"27583","inReplyTo":"1307558930-16074-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 19/19] transport-helper: implement marks location as capability","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T18:48:50Z","receivedAt":"2011-06-08T18:48:50Z","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 Sanity check added as suggested by Jonathan during review.\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 88bcb20..d882fa3 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 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":"169685","messageId":"20110608192850.GF27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-4-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T19:28:51Z","receivedAt":"2011-06-08T19:28:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n[....]\n> The fifth test is the most complex, and does not have a fix\n> in this series. It tests pushing a ref via the export\n> mechanism to a new name on the remote side (i.e.,\n> \"git push $remote old:new\").\n\nThanks. Makes sense up to here (and the previous two patches are\nobviously good, too).\n\n> The problem is that we push all of the work of generating\n> the export stream onto fast-export,\n\nHere \"we\" means remote-testgit, not transport-helper, right?\n\n> but we have no way of\n> communicating to fast-export that this name mapping is\n> happening.\n\nIt's not just name mapping, no? E.g., I could try\n\n\tgit push testgit::/path/to/repo $(git rev-parse HEAD):new\n\nSo I think the current implementation of \"export\" by testgit\nis just wrong. ;-)\n\n[...]\n> There are two possible solutions:\n>\n> 1. Indicate the mapping to fast-export, so that it can\n> generate the \"mapped\" names. Unfortunately, this is\n> somewhat difficult due to the way fast-import is\n> implemented. It feeds its revision parameters to the\n> regular rev-walking machinery\n\nThis seems like a reasonable solution. One way would be to teach\nfast-export about refspecs; another way would be to set up refs in a\nprivate namespace and then munge the stream that fast-export spits out\nbefore passing it back to the transport-helper; another (more ugly and\nhackish) way would be to set up a private repository with the real\nrepository set up as an alternate, put the appropriate refs there, and\npoint fast-export to that.\n\nAnyway, I don't think this should be in the commit message for the new\ntest which doesn't even know about the remote helper protocol. :)\n\n> --- a/t/t5800-remote-helpers.sh\n> +++ b/t/t5800-remote-helpers.sh\n> @@ -81,4 +81,51 @@ test_expect_success PYTHON_24 'pushing remote local repo' '\n> \tcompare_refs clone HEAD server HEAD\n> '\n> \n> +test_expect_failure PYTHON_24 'fetch new branch' '\n\nSide note: this repeated PYTHON_24 implementation detail as a\nprerequisite feels wrong. Would it make sense to do something like\n\n\tif test_have_prereq PYTHON_24\n\tthen\n\t\ttest_set_prereq REMOTE_TESTGIT\n\tfi\n\nat the start and use that?\n\n[...]\n> +test_expect_failure PYTHON_24 '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\nIt would also be interesting to test tag pushes and pushes by object\nname.\n\n\t(cd clone &&\n\t git tag -a -m 'example tag' example-tag &&\n\t git push origin tag example-tag\n\t) &&\n\tcompare_refs clone example-tag server refs/tags/example-tag\n\n\t(cd clone &&\n\t file_blob=$(git rev-parse --verify HEAD:file) &&\n\t git push origin \"$file_blob\":refs/blobs/file\n\t) &&\n\tcompare_refs clone HEAD:file server refs/blobs/file\n\n\t(cd clone &&\n\t echo more >>file &&\n\t git add file &&\n\t file_blob=$(git rev-parse --verify :file) &&\n\t git push origin \"$file_blob\":refs/blobs/newfile\n\t) &&\n\tcompare_refs clone :file server refs/blobs/newfile\n\n\t(cd clone &&\n\t echo more >>file &&\n\t git commit -a -m another &&\n\t git push origin HEAD^0:master\n\t) &&\n\tcompare_refs clone HEAD server HEAD\n"},{"id":"169686","messageId":"20110608193049.GG27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-5-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T19:30:49Z","receivedAt":"2011-06-08T19:30:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Unchanged from Peff's series.\n\nLooks good. What happened to Peff's signoff?\n"},{"id":"169687","messageId":"20110608193630.GH27715@elie","threadId":"27583","inReplyTo":"20110608192850.GF27715@elie","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T19:36:31Z","receivedAt":"2011-06-08T19:36:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Sverre Rabbelier wrote:\n\n>> The problem is that we push all of the work of generating\n>> the export stream onto fast-export,\n>\n> Here \"we\" means remote-testgit, not transport-helper, right?\n\nGah! Clearly I am not thinking straight and it means the\ntransport-helper.\n\n> It's not just name mapping, no? E.g., I could try\n>\n> \tgit push testgit::/path/to/repo $(git rev-parse HEAD):new\n>\n> So I think the current implementation of \"export\" by testgit\n> is just wrong. ;-)\n\nSo this refers to transport-helper, too, and...\n\n[...]\n> This seems like a reasonable solution. One way would be to teach\n> fast-export about refspecs\n\n... the level of permissible hackishness goes down :), making teaching\nfast-export about refspecs the only obviously reasonable fix at first\nglance.\n\n[...]\n> Anyway, I don't think this should be in the commit message for the new\n> test which doesn't even know about the remote helper protocol. :)\n\nSorry for the nonsense. At least this last part still applies.\n"},{"id":"169688","messageId":"20110608194205.GI27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-7-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T19:42:05Z","receivedAt":"2011-06-08T19:42:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\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\nDoes this deal with forced (non-fast-forward) pushes? (Not a\ncomplaint, just curious.)\n"},{"id":"169689","messageId":"BANLkTin6u_oZpaHiRfEzsL2kSbj9coZcrQ@mail.gmail.com","threadId":"27583","inReplyTo":"20110608193049.GG27715@elie","subject":"Re: [PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T19:47:38Z","receivedAt":"2011-06-08T19:47:38Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 21:30, Jonathan Nieder wrote:\n>>   Unchanged from Peff's series.\n>\n> Looks good.  What happened to Peff's signoff?\n\nI pulled the series from his github repo, which didn't have it.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169690","messageId":"BANLkTinP2bnREuok6ptc7oBKdiPfqUqG_g@mail.gmail.com","threadId":"27583","inReplyTo":"20110608193630.GH27715@elie","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T19:51:59Z","receivedAt":"2011-06-08T19:51:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 21:36, Jonathan Nieder wrote:\n> Gah!  Clearly I am not thinking straight and it means the\n> transport-helper.\n\nYup.\n\n> ... the level of permissible hackishness goes down :), making teaching\n> fast-export about refspecs the only obviously reasonable fix at first\n> glance.\n\nOr, as I mentioned in the nearby thread, by having\nfast-import/fast-export not modify any refs at all, and do that\nseparately.\n\n>> Anyway, I don't think this should be in the commit message for the new\n>> test which doesn't even know about the remote helper protocol. :)\n>\n> Sorry for the nonsense.  At least this last part still applies.\n\nSure, this was just a resend of Peff's patch, it does need a new\ncommit message :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169691","messageId":"20110608200345.GJ27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-11-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 10/19] fast-import: introduce 'done' command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T20:03:45Z","receivedAt":"2011-06-08T20:03:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Add a 'done' command that causes fast-import to stop reading from the\n> stream and exit.\n>\n> If 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),\n> make the 'done' command mandatory.\n\nHoorah! Thanks for reviving this.\n\n> Another possible application of the 'done' command would to be allow a\n> fast-import stream that is only a small part of a larger encapsulating\n> stream to be easily parsed, leaving the file offset after the \"done\\n\"\n> so the other application can pick up from there. This patch does not\n> teach fast-import to do that --- fast-import still uses buffered input\n> (stdio).\n\nPossible approaches, two ugly and one sane:\n\n A) use setvbuf() to make input line-buffered. Unfortunately, as\n POSIX tells us:\n\n\tApplications should note that many implementations only provide\n\tline buffering on input from terminal devices.\n\n So making it actually work involves pseudo-tty craziness. Not a\n good idea.\n\n B) leave some room for buffering after the \"done\" command and make\n fast-import use setvbuf() to control the size of the stdio buffer.\n This is disgusting but it probably works.\n\n C) make sure the upstream of the pipe does not write anything after\n \"done\\n\" until the \"done\\n\" command has been read. In other\n words:\n\n\ttransport-helper>\tdone\n\tremote-helper> \tyep, I'm done\n\ttransport-helper>\tah, now that you mention that, here are\n\t \ta few more things you could do...\n\nAs long as there is nothing after the \"done\\n\" in the pipe buffer when\nfast-import reads \"done\\n\", it won't read too far.\n\nThis is all made complicated to debug by stdio's habit of closing\nstreams at exit time, flushing streams at close time, and\nrepositioning the file offset of seekable input (but not pipes) at\nflush time.\n\nHope that helps.\n"},{"id":"169693","messageId":"BANLkTik71j9qw_eaFmAMXjMmZq9wCTpGMg@mail.gmail.com","threadId":"27583","inReplyTo":"20110608200345.GJ27715@elie","subject":"Re: [PATCH 10/19] fast-import: introduce 'done' command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T20:07:07Z","receivedAt":"2011-06-08T20:07:07Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 22:03, Jonathan Nieder wrote:\n>  C) make sure the upstream of the pipe does not write anything after\n>    \"done\\n\" until the \"done\\n\" command has been read.  In other\n>    words:\n>\n>        transport-helper>       done\n>        remote-helper>          yep, I'm done\n>        transport-helper>       ah, now that you mention that, here are\n>                                a few more things you could do...\n>\n> As long as there is nothing after the \"done\\n\" in the pipe buffer when\n> fast-import reads \"done\\n\", it won't read too far.\n\nThis I think makes the most sense. Although I'm not sure it's\ncurrently possible in remote-testgit, as in, I'm not sure the relevant\npipes are connected to the right programs to make feasible. Also, I'm\nnot really interested in implementing that, or at least, not right\nnow, I just shamelessly stole your commit message ;).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169694","messageId":"20110608204754.GK27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-17-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 16/19] transport-helper: change import semantics","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T20:47:54Z","receivedAt":"2011-06-08T20:47:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Currently the helper must somehow guess how many import statements to\n> read before it starts outputting its fast-export stream.\n\nI'm guessing one way is to read until EOF.\n\n> This is\n> because the remote helper infrastructure runs fast-import only once,\n> so the helper is forced to output one stream for all import commands\n> it will receive. The only reason this worked in the past was because\n> only one ref was imported at a time.\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\nSeems like a good idea to me. Documentation/git-remote-helpers.txt\ncould use a corresponding update ---\n\nWait, now that I look there, wouldn't it be more consistent with\nwhat \"push\" does to make the semantics\n\n\t... A batch sequence of\n\tone or more import commands is terminated with a blank line.\n\n? In other words, what was the rationale for making push work like\n\n\tpush +src1:dst1\n\tpush +src2:dst2\n\tpush +src3:dst3\n\n\t[... command stream continues here ...]\n\ninstead of\n\n\tpush\n\t+src1:dst1\n\t+src2:dst2\n\t+src3:dst3\n\n\t[... command stream continues ...]\n\nand does it apply for import, too?\n"},{"id":"169695","messageId":"BANLkTi=tfE2GtpSZxY7ir98MDREefSFT1g@mail.gmail.com","threadId":"27583","inReplyTo":"20110608204754.GK27715@elie","subject":"Re: [PATCH 16/19] transport-helper: change import semantics","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T20:52:22Z","receivedAt":"2011-06-08T20:52:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 22:47, Jonathan Nieder wrote:\n> Wait, now that I look there, wouldn't it be more consistent with\n> what \"push\" does to make the semantics\n>\n>        ... A batch sequence of\n>        one or more import commands is terminated with a blank line.\n>\n> ?  In other words, what was the rationale for making push work like\n>\n>        push +src1:dst1\n>        push +src2:dst2\n>        push +src3:dst3\n>\n>        [... command stream continues here ...]\n>\n> instead of\n>\n>        push\n>        +src1:dst1\n>        +src2:dst2\n>        +src3:dst3\n>\n>        [... command stream continues ...]\n>\n> and does it apply for import, too?\n\nI don't know. Shawn added the push capability in ae4efe19 (Move WebDAV\nHTTP push under remote-curl, Fri Oct 30 2009) but there is no\nrationale for doing it that way there. Shawn, do you remember?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169696","messageId":"20110608205423.GL27715@elie","threadId":"27583","inReplyTo":"1307558930-16074-19-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 18/19] transport-helper: Use capname for gitdir capability too","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-08T20:54:23Z","receivedAt":"2011-06-08T20:54:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Also properly use capname in the refspec capability.\n>\n> Signed-off-by: Sverre Rabbelier \n\nThanks. The old commit message said\n\n\tPreviously the gitdir and refspec capabilities could not be listed as\n\trequired or their parsing would break.\n\nwhich seemed like a somewhat helpful thing to mention. :) I suspect\nthe after-the-cut comment\n\n\tI suspect the reason the second hunk wasn't caught is because the\n\tseries that added 'gitdir' as capability, and the one that added\n\trequired capabilities were done in parallel.\n\nwould be helpful for future readers, too.\n"},{"id":"169697","messageId":"BANLkTi=BkqgRY2qoYL9uM2FoLVMqAEMoAQ@mail.gmail.com","threadId":"27583","inReplyTo":"20110608205423.GL27715@elie","subject":"Re: [PATCH 18/19] transport-helper: Use capname for gitdir capability too","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T20:57:59Z","receivedAt":"2011-06-08T20:57:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 22:54, Jonathan Nieder wrote:\n> Thanks.  The old commit message said [...]\n> which seemed like a somewhat helpful thing to mention. :)  I suspect\n> the after-the-cut comment would be helpful for future readers, too.\n\nYeah, not sure what happened, but all my commits had zero-to-none\ncommit messages. Perhaps I just added them while sending out the patch\nseries last time? Anyway, can't remember, but that's why, fixed\nlocally :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169698","messageId":"BANLkTi=-roQ5DH6RQ75+EwfDr9LutU0jrw@mail.gmail.com","threadId":"27583","inReplyTo":"20110608192850.GF27715@elie","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T21:13:16Z","receivedAt":"2011-06-08T21:13:16Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 21:28, Jonathan Nieder wrote:\n>> +test_expect_failure PYTHON_24 'fetch new branch' '\n>\n> Side note: this repeated PYTHON_24 implementation detail as a\n> prerequisite feels wrong.  Would it make sense to do something like\n>\n>        if test_have_prereq PYTHON_24\n>        then\n>                test_set_prereq REMOTE_TESTGIT\n>        fi\n>\n> at the start and use that?\n\nI don't know, it already says:\n\nif test_have_prereq PYTHON && \"$PYTHON_PATH\" -c '\nimport sys\nif sys.hexversion < 0x02040000:\n sys.exit(1)\n'\nthen\n # Requires Python 2.4 or newer\n\ttest_set_prereq PYTHON_24\nfi\n\nAt the start. Are you suggesting to rename PYTHON_24 to REMOTE_TESTGIT?\n\n> It would also be interesting to test tag pushes and pushes by object\n> name.\n\nI was going to test this, but it seems there's a problem with my\nseries. Peff, if you want to look into it, (if not I'll try to look at\nit tomorrow) it's up at my github fork [0].\n\nok 12 - push when remote has extra refs\n\nexpecting success:\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\nSwitched to a new branch 'new-name'\n[new-name f29f436] seven\n Author: A U Thor \n 1 files changed, 1 insertions(+), 0 deletions(-)\nUnpacking objects: 100% (3/3), done.\nTo testgit::file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server\n * [new branch] master\nerror: Trying to write ref refs/remotes/origin/master with nonexistant\nobject 0000000000000000000000000000000000000000\nerror: Cannot update the ref 'refs/remotes/origin/master'.\n\n[0] https://github.com/SRabbelier/git/commits/remote-helpers\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169702","messageId":"20110608215723.GC13021@sigill.intra.peff.net","threadId":"27583","inReplyTo":"1307558930-16074-2-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 01/19] transport-helper: fix minor leak in push_refs_with_export","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-08T21:57:25Z","receivedAt":"2011-06-08T21:57:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 08, 2011 at 08:48:32PM +0200, Sverre Rabbelier wrote:\n\n> From: Jeff King \n> \n> \n> Signed-off-by: Sverre Rabbelier \n\nHmm. This and the other ones from me lack my S-o-b. I'm pretty sure my\noriginals included it, so I'm not sure what stripped it out.\n\n-Peff\n"},{"id":"169703","messageId":"BANLkTin6mYLJyusmyOycNzmZsNHOKgDWKw@mail.gmail.com","threadId":"27583","inReplyTo":"20110608215723.GC13021@sigill.intra.peff.net","subject":"Re: [PATCH 01/19] transport-helper: fix minor leak in push_refs_with_export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T22:08:35Z","receivedAt":"2011-06-08T22:08:35Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 23:57, Jeff King wrote:\n> Hmm. This and the other ones from me lack my S-o-b. I'm pretty sure my\n> originals included it, so I'm not sure what stripped it out.\n\nI'm using gmail, so since I can't really apply from the list I pulled\nfrom your github, where you hadn't added the s-o-b it seems :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169706","messageId":"20110608221535.GA15530@sigill.intra.peff.net","threadId":"27583","inReplyTo":"BANLkTin6u_oZpaHiRfEzsL2kSbj9coZcrQ@mail.gmail.com","subject":"Re: [PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-08T22:15:38Z","receivedAt":"2011-06-08T22:15:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 08, 2011 at 09:47:38PM +0200, Sverre Rabbelier wrote:\n\n> On Wed, Jun 8, 2011 at 21:30, Jonathan Nieder wrote:\n> >>   Unchanged from Peff's series.\n> >\n> > Looks good.  What happened to Peff's signoff?\n> \n> I pulled the series from his github repo, which didn't have it.\n\nAh, yeah, I stick it in when I send the email. Now that I am publishing\nmy topic branches, I should maybe do it at commit time.\n\n-Peff\n"},{"id":"169707","messageId":"20110608221908.GB15530@sigill.intra.peff.net","threadId":"27583","inReplyTo":"20110608194205.GI27715@elie","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-08T22:19:10Z","receivedAt":"2011-06-08T22:19:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 08, 2011 at 02:42:05PM -0500, Jonathan Nieder wrote:\n\n> Sverre Rabbelier wrote:\n> \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> \n> Does this deal with forced (non-fast-forward) pushes? (Not a\n> complaint, just curious.)\n\nNo, nor can it. The problem is that any information about the ref\nmapping or force-state is lost. All the helper sees is that stuff got\nput into the staging repo, and it has to move it over.\n\nIf we do a fix that allows \"refs/heads/foo:refs/heads/bar\", then it\nshould also properly allow \"+refs/heads/foo:refs/heads/bar\". Which I\nthink means changing the single \"export\" command in the ref-helper to\nsomething like:\n\n export refs/heads/foo\n export refs/heads/foo:refs/heads/bar\n export +refs/heads/force\n\n-Peff\n"},{"id":"169708","messageId":"BANLkTi=TYArAgp_+fXvOSDMqsyteQJrVCg@mail.gmail.com","threadId":"27583","inReplyTo":"20110608221908.GB15530@sigill.intra.peff.net","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-08T22:21:07Z","receivedAt":"2011-06-08T22:21:07Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 00:19, Jeff King wrote:\n> If we do a fix that allows \"refs/heads/foo:refs/heads/bar\", then it\n> should also properly allow \"+refs/heads/foo:refs/heads/bar\". Which I\n> think means changing the single \"export\" command in the ref-helper to\n> something like:\n>\n>  export refs/heads/foo\n>  export refs/heads/foo:refs/heads/bar\n>  export +refs/heads/force\n\nOr, since we changed import, to:\n\nexport\nrefs/heads/foo\nrefs/heads/foo:refs/heads/bar\n+refs/heads/force\n\\n\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169712","messageId":"7vei3328c8.fsf@alter.siamese.dyndns.org","threadId":"27583","inReplyTo":"1307558930-16074-5-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-08T23:48:23Z","receivedAt":"2011-06-08T23:48:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> From: Jeff King \n>\n> Upon receiving an \"import\" command, the testgit remote\n> helper would ignore the ref asked for by git and generate a\n> fast-export stream based on HEAD. Instead, we should\n> actually give git the ref it asked for.\n>\n> This requires adding a new parameter to the export_repo\n> method in the remote-helpers python library, which may be\n> used by code outside of git.git. We use a default parameter\n> so that callers without the new parameter will get the same\n> behavior as before.\n>\n> Signed-off-by: Sverre Rabbelier \n> ---\n>\n> Unchanged from Peff's series.\n\nThis seems to be slightly different from what was sent to the list,\nthough. Using refs=None as default and assigning [\"HEAD\"] is more in line\nwith the standard Python practice, so the implementation is better.\n\nBut \"do we still append HEAD after --import-marks?\" still stands.\n"},{"id":"169720","messageId":"7vsjrjzubl.fsf@alter.siamese.dyndns.org","threadId":"27583","inReplyTo":"1307558930-16074-18-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-09T01:07:10Z","receivedAt":"2011-06-09T01:07:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> Now that the remote helper protocol uses the new done command in its\n> fast-import streams, export no longer needs to be the last command in\n> the stream.\n>\n> Signed-off-by: Sverre Rabbelier \n> ---\n> ...\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 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\nI've looked at fa8c097 (Support remote helpers implementing smart\ntransports, 2009-12-09) which introduced this no_disconnect_req field,\n73b49a7 (remote-helpers: add support for an export command, 2010-03-29)\nwhich added push_refs_with_export() and made it set the field to 1, and\nalso have read Documentation/git-remote-helpers.txt, but fail to see the\nconnection between \"this command no longer needs to be the last one\" and\n\"we do not set no-disconnect-req to 1, so that we do not send an empty\nline when we disconnect the helper\".\n\nCould you clarify the logic, perhaps by commenting a bit more where this\nfield is examined and code changes its behaviour in disconnect_helper()?\n"},{"id":"169730","messageId":"BANLkTi=puAfoPhuKxsHd9ayu0DHA6d1peA@mail.gmail.com","threadId":"27583","inReplyTo":"7vei3328c8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 04/19] teach remote-testgit to import non-HEAD refs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T06:23:23Z","receivedAt":"2011-06-09T06:23:23Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 01:48, Junio C Hamano wrote:\n> This seems to be slightly different from what was sent to the list,\n> though.  Using refs=None as default and assigning [\"HEAD\"] is more in line\n> with the standard Python practice, so the implementation is better.\n\nAh, yes, I fixed the style nits suggested in my own review.\n\n> But \"do we still append HEAD after --import-marks?\" still stands.\n\nFixed locally. Sharp eyes :)\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169732","messageId":"BANLkTim1R=1SrUXgOOjmRioG45KKVity3Q@mail.gmail.com","threadId":"27583","inReplyTo":"7vsjrjzubl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T06:48:27Z","receivedAt":"2011-06-09T06:48:27Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 03:07, Junio C Hamano wrote:\n> Could you clarify the logic, perhaps by commenting a bit more where this\n> field is examined and code changes its behaviour in disconnect_helper()?\n\nPreviously this bit had to be set, so that we didn't try to write the\ntrailing \\n on a closed socket. Now, the socket is no longer closed,\nso we can send the trailing \\n again. Does that make sense?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169740","messageId":"20110609075141.GA4885@elie","threadId":"27583","inReplyTo":"BANLkTim1R=1SrUXgOOjmRioG45KKVity3Q@mail.gmail.com","subject":"Re: [PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-09T07:51:41Z","receivedAt":"2011-06-09T07:51:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Thu, Jun 9, 2011 at 03:07, Junio C Hamano wrote:\n\n>> Could you clarify the logic, perhaps by commenting a bit more where this\n>> field is examined and code changes its behaviour in disconnect_helper()?\n>\n> Previously this bit had to be set, so that we didn't try to write the\n> trailing \\n on a closed socket. Now, the socket is no longer closed,\n> so we can send the trailing \\n again. Does that make sense?\n\nYes, but I'm still missing something. What does the trailing \\n ever\nhave to be written?\n\n\"git log -Sdisconnect_helper -- transport-helper.c\" doesn't give many\nclues. I imagine it's a way to check whether the child is still alive\nand to warn it not to be alarmed when the output end of its input pipe\ncloses.\n"},{"id":"169741","messageId":"20110609080912.GB4885@elie","threadId":"27583","inReplyTo":"20110608221908.GB15530@sigill.intra.peff.net","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-09T08:09:12Z","receivedAt":"2011-06-09T08:09:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> If we do a fix that allows \"refs/heads/foo:refs/heads/bar\", then it\n> should also properly allow \"+refs/heads/foo:refs/heads/bar\". Which I\n> think means changing the single \"export\" command in the ref-helper to\n> something like:\n>\n> export refs/heads/foo\n> export refs/heads/foo:refs/heads/bar\n> export +refs/heads/force\n\nThanks for explaining. The spirit of what you're saying seems right,\nso I'd like to spell it out a little to make sure.\n\nIf we imagine that the remote helper author wants to write as little\ncode as possible (which seems reasonable), then probably their\n\"export\" command will simply feed its input to a vcs-fast-import\nprogram. git fast-export doesn't know how to do ref mapping and\nneither would vcs-fast-import. So one is led to wonder which stage in\nthe pipeline can make the adaptations to make \"git push hgrepo\nHEAD:refs/heads/bar\" work.\n\nTo be friendly to remote helper authors, it would be nice to take\ncare of the ref mapping somewhere on the transport-helper side, unless\nfast-import learns a new mode that does not label its result with\nrefs. In the latter case, the \"export\" command could look like[*]\n\n\texport :1 refs/heads/foo\n\texport :2 refs/heads/bar\n\texport :3 +refs/heads/force\n\nwith :1, :2, and :3 being marks in the fast-import stream.\n\nIn the former case, the transport-helper could arrange for a stream\nthat sets up refs/heads/foo, refs/heads/bar, and refs/heads/force\nto be written to stdout, but there would need to be a way to mention\nin the stream that the update to refs/heads/force is allowed to be\nnon fast-forward. fast-import has a --force option for that, which\nunfortunately can't be controlled ref by ref.\n\nWhich is all a long way to say, I think you're right and that\nsomething like the syntax marked with [*] would be nice.\n"},{"id":"169742","messageId":"BANLkTinuZAJtuGw4hOLLjqLx5pN9iDhh+w@mail.gmail.com","threadId":"27583","inReplyTo":"20110609075141.GA4885@elie","subject":"Re: [PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T08:28:20Z","receivedAt":"2011-06-09T08:28:20Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 09:51, Jonathan Nieder wrote:\n> Yes, but I'm still missing something.  What does the trailing \\n ever\n> have to be written?\n>\n> \"git log -Sdisconnect_helper -- transport-helper.c\" doesn't give many\n> clues.  I imagine it's a way to check whether the child is still alive\n> and to warn it not to be alarmed when the output end of its input pipe\n> closes.\n\nYes, the trailing \\n is to signal to the helper that the connection is\nabout to close, allowing it to do whatever cleanup necessarily. It's\nkind of like the \"done\" command for fast-import.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169743","messageId":"BANLkTincedbjH+ta6g7+uA3=3k_30RQaCg@mail.gmail.com","threadId":"27583","inReplyTo":"20110609080912.GB4885@elie","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T08:29:57Z","receivedAt":"2011-06-09T08:29:57Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 10:09, Jonathan Nieder wrote:\n>        export :1 refs/heads/foo\n>        export :2 refs/heads/bar\n>        export :3 +refs/heads/force\n>\n> with :1, :2, and :3 being marks in the fast-import stream.\n\nThe only problem there is that we don't know the relevant marks beforehand.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169744","messageId":"20110609084342.GC4885@elie","threadId":"27583","inReplyTo":"BANLkTincedbjH+ta6g7+uA3=3k_30RQaCg@mail.gmail.com","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-09T08:43:42Z","receivedAt":"2011-06-09T08:43:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Thu, Jun 9, 2011 at 10:09, Jonathan Nieder wrote:\n\n>>        export :1 refs/heads/foo\n>>        export :2 refs/heads/bar\n>>        export :3 +refs/heads/force\n>>\n>> with :1, :2, and :3 being marks in the fast-import stream.\n>\n> The only problem there is that we don't know the relevant marks beforehand.\n\nSince this requires changing \"git fast-export\" anyway, we could\narrange to know.\n\nA related problem, though: it is not friendly to stomp on mark numbers\nwhich an exporter might want to use for some other purpose. So yeah,\nthe precise syntax above is not so great.\n"},{"id":"169748","messageId":"20110609091045.GE4885@elie","threadId":"27583","inReplyTo":"1307558930-16074-16-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 15/19] transport-helper: update ref status after push with export","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-06-09T09:10:45Z","receivedAt":"2011-06-09T09:10:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Obviously the testgit helper shouldn't just print 'ok' for master,\n> but it demonstrates the idea.\n[... and more interesting commentary ...]\n\nThe above probably belongs in the commit message.\n\n[...]\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\nNow the conversation looks like:\n\n\tgit> \tdone [from fast-export]\n\tremote helper>\tok refs/heads/master\n\tremote helper>\t[blank line]\n\nand after patch 17:\n\n\tgit> \tdone\n\tremote helper>\tok refs/heads/master\n\tremote helper>\n\tgit>\n\nwhich is to say, this implements the \"approach C\" mentioned in reply\nto patch 10 that ensures that the remote helper gets the blank line\ntelling it disconnect is imminent. Good to see.\n"},{"id":"169752","messageId":"BANLkTik2cWBAU9jYSKWR_MOd4DfLi5AJYg@mail.gmail.com","threadId":"27583","inReplyTo":"20110609091045.GE4885@elie","subject":"Re: [PATCH 15/19] transport-helper: update ref status after push with export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T10:23:03Z","receivedAt":"2011-06-09T10:23:03Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 11:10, Jonathan Nieder wrote:\n> which is to say, this implements the \"approach C\" mentioned in reply\n> to patch 10 that ensures that the remote helper gets the blank line\n> telling it disconnect is imminent.  Good to see.\n\nAh, yes it does :). Happy coincidence.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169753","messageId":"BANLkTimBF=hVmPLdh1Wqe4WBNFA2kpw6mg@mail.gmail.com","threadId":"27583","inReplyTo":"20110609084342.GC4885@elie","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T10:26:11Z","receivedAt":"2011-06-09T10:26:11Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Jun 9, 2011 at 10:43, Jonathan Nieder wrote:\n> Sverre Rabbelier wrote:\n>> On Thu, Jun 9, 2011 at 10:09, Jonathan Nieder wrote:\n>\n>>>        export :1 refs/heads/foo\n>>>        export :2 refs/heads/bar\n>>>        export :3 +refs/heads/force\n>>>\n>>> with :1, :2, and :3 being marks in the fast-import stream.\n>>\n>> The only problem there is that we don't know the relevant marks beforehand.\n>\n> Since this requires changing \"git fast-export\" anyway, we could\n> arrange to know.\n\nHow? Or do you mean like, 'reserve' a certain mark?\n\n> A related problem, though: it is not friendly to stomp on mark numbers\n> which an exporter might want to use for some other purpose.  So yeah,\n> the precise syntax above is not so great.\n\nWe might just want to do it _afterward_. We already have some\ncommunication after the import (the \"ok ...\" lines), so why not add a\n\"update :1 /refs/heads/master\"?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169758","messageId":"BANLkTikuAjchuACmdHYG6giy=3s2RkQxhw@mail.gmail.com","threadId":"27583","inReplyTo":"BANLkTi=-roQ5DH6RQ75+EwfDr9LutU0jrw@mail.gmail.com","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-06-09T12:45:40Z","receivedAt":"2011-06-09T12:45:40Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jun 8, 2011 at 23:13, Sverre Rabbelier wrote:\n> I was going to test this, but it seems there's a problem with my\n> series. Peff, if you want to look into it, (if not I'll try to look at\n> it tomorrow) it's up at my github fork [0].\n\nTurns out this was caused by the sloppy implementation of [PATCH\n15/19] transport-helper: update ref status after push with export.\nAfter implementing it properly the breakage is fixed although I'm not\nentirely sure the output is (always) correct.\n\nFollows are the test results (ran with GIT_TRACE=1 GIT_DEBUG_TESTGIT=1):\n\n\nexpecting success:\n (cd clone &&\n git tag -a -m \"example tag\" example-tag &&\n git push origin tag example-tag\n ) &&\n compare_refs clone example-tag server refs/tags/example-tag\n\ntrace: built-in: git 'tag' '-a' '-m' 'example tag' 'example-tag'\ntrace: built-in: git 'push' 'origin' 'tag' 'example-tag'\ntrace: run_command: 'git-remote-testgit' 'origin'\n'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server'\ntrace: built-in: git 'ls-remote' 'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\ntrace: run_command: 'git-upload-pack '\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: exec: 'sh' '-c' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\''' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\nprefix: 'refs/testgit/origin/'\nGot arguments ['origin', 'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server']\nGot command 'capabilities' with args ''\nGot command 'gitdir' with args '.git'\nGot command 'list' with args ''\n? refs/heads/new\n? refs/heads/master\n? refs/heads/new-name\n@refs/heads/master HEAD\nGot command 'export' with args ''\ntrace: run_command: 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'refs/tags/example-tag'\ntrace: exec: 'git' 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'refs/tags/example-tag'\ntrace: built-in: git 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'refs/tags/example-tag'\ntrace: built-in: git 'fetch' '--quiet'\n'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\ntrace: run_command: 'git-upload-pack '\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: exec: 'sh' '-c' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\''' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: run_command: 'rev-list' '--quiet' '--objects' '--stdin' '--not' '--all'\ntrace: built-in: git 'update-ref' 'refs/heads/master' 'FETCH_HEAD'\ntrace: built-in: git 'for-each-ref' 'refs/heads'\ntrace: exec: 'git-fast-import' '--quiet'\n'--export-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\n'--import-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\ntrace: run_command: 'git-fast-import' '--quiet'\n'--export-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\n'--import-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\ntrace: built-in: git 'for-each-ref' 'refs/heads'\ntrace: built-in: git 'push' '--quiet'\n'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git' '--all'\ntrace: run_command: 'git-receive-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: exec: 'sh' '-c' 'git-receive-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\''' 'git-receive-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: built-in: git 'receive-pack' '/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\nEverything up-to-date\ntrace: built-in: git 'rev-parse' '--verify' 'example-tag'\ntrace: built-in: git 'rev-parse' '--verify' 'refs/tags/example-tag'\nfatal: Needed a single revision\nnot ok - 15 test pushing tags\n\nSimilar output for the following two:\n\n\nEverything up-to-date\ntrace: built-in: git 'rev-parse' '--verify' 'HEAD:file'\ntrace: built-in: git 'rev-parse' '--verify' 'refs/blobs/file'\nfatal: Needed a single revision\nnot ok - 16 test pushing a blob\n\n\nEverything up-to-date\ntrace: built-in: git 'rev-parse' '--verify' ':file'\ntrace: built-in: git 'rev-parse' '--verify' 'refs/blobs/newfile'\nfatal: Needed a single revision\nnot ok - 17 test pushing an updated blob\n\nAnd then finally a rather different failure:\n\nexpecting success:\n (cd clone &&\n echo more >>file &&\n git commit -a -m another &&\n git push origin HEAD^0:master\n ) &&\n compare_refs clone HEAD server HEAD\n\n\ntrace: built-in: git 'commit' '-a' '-m' 'another'\n[new-name a02c029] another\n Author: A U Thor \n 1 files changed, 1 insertions(+), 0 deletions(-)\ntrace: built-in: git 'push' 'origin' 'HEAD^0:master'\ntrace: run_command: 'git-remote-testgit' 'origin'\n'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server'\ntrace: built-in: git 'ls-remote' 'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\ntrace: run_command: 'git-upload-pack '\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: exec: 'sh' '-c' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\''' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\nprefix: 'refs/testgit/origin/'\nGot arguments ['origin', 'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server']\nGot command 'capabilities' with args ''\nGot command 'gitdir' with args '.git'\nGot command 'list' with args ''\n? refs/heads/new\n? refs/heads/master\n? refs/heads/new-name\n@refs/heads/master HEAD\nGot command 'export' with args ''\ntrace: run_command: 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'HEAD^0'\ntrace: exec: 'git' 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'HEAD^0'\ntrace: built-in: git 'fast-export' '--use-done-feature'\n'--export-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'--import-marks=.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/testgit.marks'\n'^refs/testgit/origin/master' 'HEAD^0'\ntrace: built-in: git 'fetch' '--quiet'\n'file:///home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\ntrace: run_command: 'git-upload-pack '\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: exec: 'sh' '-c' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\''' 'git-upload-pack\n'\\''/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/server/.git'\\'''\ntrace: run_command: 'rev-list' '--quiet' '--objects' '--stdin' '--not' '--all'\ntrace: built-in: git 'update-ref' 'refs/heads/master' 'FETCH_HEAD'\ntrace: built-in: git 'for-each-ref' 'refs/heads'\ntrace: exec: 'git-fast-import' '--quiet'\n'--export-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\n'--import-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\ntrace: run_command: 'git-fast-import' '--quiet'\n'--export-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\n'--import-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks'\nfatal: Branch name doesn't conform to GIT standards: HEAD^0\nfast-import: dumping crash report to /home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/.git/fast_import_crash_2016\nTraceback (most recent call last):\n File \"/home/sverre/code/git/git-remote-testgit\", line 265, in \n sys.exit(main(sys.argv))\n File \"/home/sverre/code/git/git-remote-testgit\", line 262, in main\n more = read_one_line(repo)\n File \"/home/sverre/code/git/git-remote-testgit\", line 227, in read_one_line\n func(repo, cmdline)\n File \"/home/sverre/code/git/git-remote-testgit\", line 164, in do_export\n changed = repo.importer.do_import(repo.gitdir)\n File \"/home/sverre/code/git/t/../git_remote_helpers/build/lib/git_remote_helpers/git/importer.py\",\nline 54, in do_import\n check_call(args)\n File \"/home/sverre/code/git/t/../git_remote_helpers/build/lib/git_remote_helpers/util.py\",\nline 159, in check_call\n raise subprocess.CalledProcessError(retcode, cmd)\nsubprocess.CalledProcessError: Command '['git',\n'--git-dir=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/.git',\n'fast-import', '--quiet',\n'--export-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks',\n'--import-marks=/home/sverre/code/git/t/trash\ndirectory.t5800-remote-helpers/clone/.git/info/fast-import/27c45ccde749c9cc424db9cf911f01688e75d056/git.marks']'\nreturned non-zero exit status 128\nnot ok - 18 test pushing HEAD^0\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"169829","messageId":"20110610011858.GA12256@sigill.intra.peff.net","threadId":"27583","inReplyTo":"20110608192850.GF27715@elie","subject":"Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-10T01:18:58Z","receivedAt":"2011-06-10T01:18:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 08, 2011 at 02:28:51PM -0500, Jonathan Nieder wrote:\n\n> > but we have no way of\n> > communicating to fast-export that this name mapping is\n> > happening.\n> \n> It's not just name mapping, no? E.g., I could try\n> \n> \tgit push testgit::/path/to/repo $(git rev-parse HEAD):new\n> \n> So I think the current implementation of \"export\" by testgit\n> is just wrong. ;-)\n\nYeah, it's the same issue. We have no way of communicating the name of\nthe right-hand ref at all to a helper, aside from asking fast-export to\nmunge the refname in its output stream. But we have no way of telling\nfast-export that even though we asked for \"foo\", it needs to write the\nresult into \"bar\" (and potentially _also_ into \"foo\", if it's a ref\nfound on the RHS of a different refspec).\n\n> This seems like a reasonable solution. One way would be to teach\n> fast-export about refspecs; another way would be to set up refs in a\n> private namespace and then munge the stream that fast-export spits out\n> before passing it back to the transport-helper; another (more ugly and\n> hackish) way would be to set up a private repository with the real\n> repository set up as an alternate, put the appropriate refs there, and\n> point fast-export to that.\n\nMunging the stream is a little tricky. We have to correctly parse the\nstream and not just do something hack-ish like sed (which is what\ngit-remote-testgit does, but that's OK, because it's a hack-ish\ntest script).\n\nBut we also have to take care to handle duplicates. So exporting\n\"refs/heads/foo:refs/heads/bar\" would mean that we rewrite all of\nrefs/heads/foo into refs/heads/bar on commit lines. But two refspecs\nlike \"refs/heads/foo:refs/heads/bar refs/heads/foo:refs/heads/foo\" mean\nwe must write all of the commit lines as \"foo\" and then do a \"reset\" of\nbar to foo at the end (at least this is what fast-export already does).\nBut we also need to handle criss-crosses, like:\n\n refs/heads/foo:refs/heads/bar refs/heads/baz:refs/heads/foo\n\nIn that case we want change all commit lines for \"foo\" to \"bar\", but\nalso all for \"baz\" to \"foo\".\n\nSo implementation-wise, the simplest way is if you can give each tip a\nname, and then at the end assign all of the tips to refnames (this is\nmore or less what regular push does internally, of course).\n\nI'm not too familiar with the fast-import protocol or what other\nsystems' fast importers are capable of. Is it OK to just make a bunch of\ncommits with marks, and then at the end do our resets? That would make\nthe problem much simpler, I think.\n\n> Anyway, I don't think this should be in the commit message for the new\n> test which doesn't even know about the remote helper protocol. :)\n\nYeah, maybe not. I wanted to stick the discussion somewhere, and since I\nnever actually fixed this bug, there wasn't another relevant commit. :)\n\n> > --- a/t/t5800-remote-helpers.sh\n> > +++ b/t/t5800-remote-helpers.sh\n> > @@ -81,4 +81,51 @@ test_expect_success PYTHON_24 'pushing remote local repo' '\n> > \tcompare_refs clone HEAD server HEAD\n> > '\n> > \n> > +test_expect_failure PYTHON_24 'fetch new branch' '\n> \n> Side note: this repeated PYTHON_24 implementation detail as a\n> prerequisite feels wrong. Would it make sense to do something like\n> \n> \tif test_have_prereq PYTHON_24\n> \tthen\n> \t\ttest_set_prereq REMOTE_TESTGIT\n> \tfi\n> \n> at the start and use that?\n\nReally, I think you could just exit early if PYTHON_24 isn't set, as all\nof the tests require it. If we end up with other non-python helper\ntests, they will probably go in a separate script anyway.\n\n> > +test_expect_failure PYTHON_24 '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> It would also be interesting to test tag pushes and pushes by object\n> name.\n> [examples]\n\nAgreed.\n\n-Peff\n"},{"id":"169830","messageId":"20110610014022.GB12256@sigill.intra.peff.net","threadId":"27583","inReplyTo":"20110609080912.GB4885@elie","subject":"Re: [PATCH 06/19] git_remote_helpers: push all refs during a non-local export","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-10T01:40:22Z","receivedAt":"2011-06-10T01:40:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 09, 2011 at 03:09:12AM -0500, Jonathan Nieder wrote:\n\n> If we imagine that the remote helper author wants to write as little\n> code as possible (which seems reasonable), then probably their\n> \"export\" command will simply feed its input to a vcs-fast-import\n> program. git fast-export doesn't know how to do ref mapping and\n> neither would vcs-fast-import. So one is led to wonder which stage in\n> the pipeline can make the adaptations to make \"git push hgrepo\n> HEAD:refs/heads/bar\" work.\n\nYeah, when writing the original series, I really had the thought that\nthe remote helper would be sitting in the middle and could do the\nmapping. But really, not necessarily; the data is more likely to go\nstraight to an importer. And the mapping code may be ugly, which means\nwe want git itself to do the heavy lifting, not individual helpers.\n\n> To be friendly to remote helper authors, it would be nice to take\n> care of the ref mapping somewhere on the transport-helper side, unless\n> fast-import learns a new mode that does not label its result with\n> refs. In the latter case, the \"export\" command could look like[*]\n> \n> \texport :1 refs/heads/foo\n> \texport :2 refs/heads/bar\n> \texport :3 +refs/heads/force\n> \n> with :1, :2, and :3 being marks in the fast-import stream.\n\nI think we already have something like that with:\n\n reset refs/heads/foo\n from :1\n\nin the import stream. And then that matches the concept that the helper\nis really just pushing all of the work to that VCS's fast-import stream.\nAnd _if_ we can convince fast-export to write a stream of commits that\nare not on any particular ref, then we could just dump the ref-mapping\nat the end of the export stream.\n\nThere's nowhere to talk about \"forced\" pushing there, though. We could\nadd in a \"force\" flag on that reset command. But it's not even\nnecessarily a concept that will map to other version control systems. I\nwonder if it is simply something that we might have to live without\nwhen moving commits between systems.\n\n-Peff\n"},{"id":"169947","messageId":"7vvcw9pxf0.fsf@alter.siamese.dyndns.org","threadId":"27583","inReplyTo":"BANLkTinuZAJtuGw4hOLLjqLx5pN9iDhh+w@mail.gmail.com","subject":"Re: [PATCH 17/19] transport-helper: export is no longer always the last command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-13T15:24:03Z","receivedAt":"2011-06-13T15:24:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Thu, Jun 9, 2011 at 09:51, Jonathan Nieder wrote:\n>> Yes, but I'm still missing something. What does the trailing \\n ever\n>> have to be written?\n>>\n>> \"git log -Sdisconnect_helper -- transport-helper.c\" doesn't give many\n>> clues. I imagine it's a way to check whether the child is still alive\n>> and to warn it not to be alarmed when the output end of its input pipe\n>> closes.\n>\n> Yes, the trailing \\n is to signal to the helper that the connection is\n> about to close, allowing it to do whatever cleanup necessarily. It's\n> kind of like the \"done\" command for fast-import.\n\n... which is the kind of clarification I wanted to see as the comment to\ndocument that no_disconnect_req field, whose semantics was unclear even to\ntwo people who are known to know a bit about git and had to ask you to\nclarify.\n"}]}