{"thread":{"id":"32071","subject":"[PATCH v5 00/15] fast-export and remote-testgit improvements","startedAt":"2012-11-11T13:59:37Z","lastAt":"2012-11-26T22:22:12Z","messageCount":65,"participants":["Felipe Contreras","Torsten Bögershausen","Jeff King","Max Horn","Jonathan Nieder","Junio C Hamano","Sverre Rabbelier","Johannes Schindelin"],"isPatch":true,"patchVersion":5,"patchTotal":15},"messages":[{"id":"202830","messageId":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":null,"subject":"[PATCH v5 00/15] fast-export and remote-testgit improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:37Z","receivedAt":"2012-11-11T13:59:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nBasically resending with a few fixes...\n\nI found more issues in fast-export. remote-testgit, and eventually I decided\nthere's no reason to use this python script, so I wrote a much simpler version\nthat does the same, and more. I'm not going to list all the reasons because\napparently opinions are not welcome in the list any more. For the actual\ndifferences you can check the patch itself.\n\nThe old remote-testgit is now remote-testpy (as it's testing the python\nframework, not really remote helpers). The tests are simplified, and exercise\nmore features of transport-helper, and unsuprisingly, find more bugs.\n\nSome of these bugs are fixed in this patch series as well, for which I already\nsent 3 versions, and they come at the end. I was surprised they did fix them,\nbut hey... good is good.\n\nI know how to fix the rest of the issues, but I'm not going to bother sending a\npatch because obvious... er, simple? fixes are not accepted, so there's no\nchance of something less... evident? getting through.\n\nCheers.\n\nChanges since v4:\n\n * Add check for bash in test\n * Avoid bash associative arrays for older versions\n * Apply trivial comments\n * White-space cleanups\n\nFelipe Contreras (15):\n fast-export: avoid importing blob marks\n remote-testgit: fix direction of marks\n remote-helpers: fix failure message\n Rename git-remote-testgit to git-remote-testpy\n Add new simplified git-remote-testgit\n remote-testgit: get rid of non-local functionality\n remote-testgit: remove irrelevant test\n remote-testgit: cleanup tests\n remote-testgit: exercise more features\n remote-testgit: report success after an import\n remote-testgit: make clear the 'done' feature\n fast-export: trivial cleanup\n fast-export: fix comparison in tests\n fast-export: make sure updated refs get updated\n fast-export: don't handle uninteresting refs\n\n .gitignore | 2 +-\n Documentation/git-remote-testgit.txt | 2 +-\n Makefile | 2 +-\n builtin/fast-export.c | 20 ++-\n git-remote-testgit | 82 +++++++++++\n git-remote-testgit.py | 272 -----------------------------------\n git-remote-testpy.py | 272 +++++++++++++++++++++++++++++++++++\n git_remote_helpers/git/importer.py | 2 +-\n t/t5800-remote-helpers.sh | 148 -------------------\n t/t5800-remote-testpy.sh | 148 +++++++++++++++++++\n t/t5801-remote-helpers.sh | 158 ++++++++++++++++++++\n t/t9350-fast-export.sh | 41 +++++-\n 12 files changed, 717 insertions(+), 432 deletions(-)\n create mode 100755 git-remote-testgit\n delete mode 100644 git-remote-testgit.py\n create mode 100644 git-remote-testpy.py\n delete mode 100755 t/t5800-remote-helpers.sh\n create mode 100755 t/t5800-remote-testpy.sh\n create mode 100644 t/t5801-remote-helpers.sh\n\n-- \n1.8.0\n"},{"id":"202831","messageId":"1352642392-28387-2-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 01/15] fast-export: avoid importing blob marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:38Z","receivedAt":"2012-11-11T13:59:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We want to be able to import, and then export, using the same marks, so\nthat we don't push things that the other side already received.\n\nUnfortunately, fast-export doesn't store blobs in the marks, but\nfast-import does. This creates a mismatch when fast export is reusing a\nmark that was previously stored by fast-import.\n\nThere is no point in one tool saving blobs, and the other not, but for\nnow let's just check in fast-export that the objects are indeed commits.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 4 ++++\n t/t9350-fast-export.sh | 14 ++++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 12220ad..a06fe10 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -614,6 +614,10 @@ static void import_marks(char *input_file)\n \t\tif (object->flags & SHOWN)\n \t\t\terror(\"Object %s already has a mark\", sha1_to_hex(sha1));\n \n+\t\tif (object->type != 1)\n+\t\t\t/* only commits */\n+\t\t\tcontinue;\n+\n \t\tmark_object(object, mark);\n \t\tif (last_idnum < mark)\n \t\t\tlast_idnum = mark;\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 3e821f9..0c8d828 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -440,4 +440,18 @@ test_expect_success 'fast-export quotes pathnames' '\n \t)\n '\n \n+test_expect_success 'test biridectionality' '\n+\techo -n > marks-cur &&\n+\techo -n > marks-new &&\n+\tgit init marks-test &&\n+\tgit fast-export --export-marks=marks-cur --import-marks=marks-cur --branches | \\\n+\tgit --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks=marks-new &&\n+\t(cd marks-test &&\n+\tgit reset --hard &&\n+\techo Wohlauf > file &&\n+\tgit commit -a -m \"back in time\") &&\n+\tgit --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks=marks-new --branches | \\\n+\tgit fast-import --export-marks=marks-cur --import-marks=marks-cur\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"202833","messageId":"1352642392-28387-3-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 02/15] remote-testgit: fix direction of marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:39Z","receivedAt":"2012-11-11T13:59:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Basically this is what we want:\n\n == pull ==\n\n\ttestgit\t\t\ttransport-helper\n\n\t* export ->\t\timport\n\n\t# testgit.marks\t\tgit.marks\n\n == push ==\n\n\ttestgit\t\t\ttransport-helper\n\n\t* import\t\t<- export\n\n\t# testgit.marks\t\tgit.marks\n\nEach side should be agnostic of the other side. Because testgit.marks\n(our helper marks) could be anything, not necesarily a format parsable\nby fast-export or fast-import. In this test hey happen to be compatible,\nbecause we use those tools, but in the real world it would be something\ncompelely different. For example, they might be mapping marks to\nmercurial revisions (certainly not parsable by fast-import/export).\n\nThis is what we have:\n\n == pull ==\n\n\ttestgit\t\t\ttransport-helper\n\n\t* export ->\t\timport\n\n\t# testgit.marks\t\tgit.marks\n\n == push ==\n\n\ttestgit\t\t\ttransport-helper\n\n\t* import\t\t<- export\n\n\t# git.marks\t\ttestgit.marks\n\nThe only reason this is working is that git.marks and testgit.marks are\nroughly the same.\n\nThis new behavior used to not be possible before due to a bug in\nfast-export, but with the bug fixed, it works fine.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit.py | 2 +-\n git_remote_helpers/git/importer.py | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex 5f3ebd2..ade797b 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -91,7 +91,7 @@ def do_capabilities(repo, args):\n if not os.path.exists(dirname):\n os.makedirs(dirname)\n \n- path = os.path.join(dirname, 'testgit.marks')\n+ path = os.path.join(dirname, 'git.marks')\n \n print \"*export-marks %s\" % path\n if os.path.exists(path):\ndiff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py\nindex 5c6b595..e28cc8f 100644\n--- a/git_remote_helpers/git/importer.py\n+++ b/git_remote_helpers/git/importer.py\n@@ -39,7 +39,7 @@ class GitImporter(object):\n gitdir = self.repo.gitpath\n else:\n gitdir = os.path.abspath(os.path.join(dirname, '.git'))\n- path = os.path.abspath(os.path.join(dirname, 'git.marks'))\n+ path = os.path.abspath(os.path.join(dirname, 'testgit.marks'))\n \n if not os.path.exists(dirname):\n os.makedirs(dirname)\n-- \n1.8.0\n"},{"id":"202832","messageId":"1352642392-28387-4-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 03/15] remote-helpers: fix failure message","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:40Z","receivedAt":"2012-11-11T13:59:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This is remote-testgit, not remote-hg.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5800-remote-helpers.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex e7dc668..d46fa40 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -8,7 +8,7 @@ test_description='Test remote-helper import and export commands'\n . ./test-lib.sh\n \n if ! test_have_prereq PYTHON ; then\n-\tskip_all='skipping git-remote-hg tests, python not available'\n+\tskip_all='skipping remote-testgit tests, python not available'\n \ttest_done\n fi\n \n@@ -17,7 +17,7 @@ import sys\n if sys.hexversion < 0x02040000:\n sys.exit(1)\n ' || {\n-\tskip_all='skipping git-remote-hg tests, python version < 2.4'\n+\tskip_all='skipping remote-testgit tests, python version < 2.4'\n \ttest_done\n }\n \n-- \n1.8.0\n"},{"id":"202834","messageId":"1352642392-28387-5-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 04/15] Rename git-remote-testgit to git-remote-testpy","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:41Z","receivedAt":"2012-11-11T13:59:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This script is not really exercising the remote-helper functionality,\nbut more the python framework for remote helpers that live in\ngit_remote_helpers.\n\nIt's also not a good example of how to write remote-helpers, unless you\nare planning to use python, and even then you might not want to use this\nframework.\n\nSo let's use a more appropriate name: git-remote-testpy.\n\nA patch that replaces git-remote-testgit with a simpler version is on\nthe way.\n\nSigned-off-by: Felipe Contreras \n---\n .gitignore | 2 +-\n Makefile | 2 +-\n git-remote-testgit.py | 272 ----------------------------------------------\n git-remote-testpy.py | 272 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t5800-remote-helpers.sh | 148 -------------------------\n t/t5800-remote-testpy.sh | 148 +++++++++++++++++++++++++\n 6 files changed, 422 insertions(+), 422 deletions(-)\n delete mode 100644 git-remote-testgit.py\n create mode 100644 git-remote-testpy.py\n delete mode 100755 t/t5800-remote-helpers.sh\n create mode 100755 t/t5800-remote-testpy.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex a188a82..48d1bbb 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -124,7 +124,7 @@\n /git-remote-ftps\n /git-remote-fd\n /git-remote-ext\n-/git-remote-testgit\n+/git-remote-testpy\n /git-repack\n /git-replace\n /git-repo-config\ndiff --git a/Makefile b/Makefile\nindex f69979e..e18ee48 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -470,7 +470,7 @@ SCRIPT_PERL += git-relink.perl\n SCRIPT_PERL += git-send-email.perl\n SCRIPT_PERL += git-svn.perl\n \n-SCRIPT_PYTHON += git-remote-testgit.py\n+SCRIPT_PYTHON += git-remote-testpy.py\n SCRIPT_PYTHON += git-p4.py\n \n SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \\\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\ndeleted file mode 100644\nindex ade797b..0000000\n--- a/git-remote-testgit.py\n+++ /dev/null\n@@ -1,272 +0,0 @@\n-#!/usr/bin/env python\n-\n-# This command is a simple remote-helper, that is used both as a\n-# testcase for the remote-helper functionality, and as an example to\n-# show remote-helper authors one possible implementation.\n-#\n-# This is a Git <-> Git importer/exporter, that simply uses git\n-# fast-import and git fast-export to consume and produce fast-import\n-# streams.\n-#\n-# To understand better the way things work, one can activate debug\n-# traces by setting (to any value) the environment variables\n-# GIT_TRANSPORT_HELPER_DEBUG and GIT_DEBUG_TESTGIT, to see messages\n-# from the transport-helper side, or from this example remote-helper.\n-\n-# hashlib is only available in python >= 2.5\n-try:\n- import hashlib\n- _digest = hashlib.sha1\n-except ImportError:\n- import sha\n- _digest = sha.new\n-import sys\n-import os\n-import time\n-sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"))\n-\n-from git_remote_helpers.util import die, debug, warn\n-from git_remote_helpers.git.repo import GitRepo\n-from git_remote_helpers.git.exporter import GitExporter\n-from git_remote_helpers.git.importer import GitImporter\n-from git_remote_helpers.git.non_local import NonLocalGit\n-\n-def get_repo(alias, url):\n- \"\"\"Returns a git repository object initialized for usage.\n- \"\"\"\n-\n- repo = GitRepo(url)\n- repo.get_revs()\n- repo.get_head()\n-\n- hasher = _digest()\n- hasher.update(repo.path)\n- repo.hash = hasher.hexdigest()\n-\n- repo.get_base_path = lambda base: os.path.join(\n- base, 'info', 'fast-import', repo.hash)\n-\n- prefix = 'refs/testgit/%s/' % alias\n- debug(\"prefix: '%s'\", prefix)\n-\n- repo.gitdir = os.environ[\"GIT_DIR\"]\n- repo.alias = alias\n- repo.prefix = prefix\n-\n- repo.exporter = GitExporter(repo)\n- repo.importer = GitImporter(repo)\n- repo.non_local = NonLocalGit(repo)\n-\n- return repo\n-\n-\n-def local_repo(repo, path):\n- \"\"\"Returns a git repository object initalized for usage.\n- \"\"\"\n-\n- local = GitRepo(path)\n-\n- local.non_local = None\n- local.gitdir = repo.gitdir\n- local.alias = repo.alias\n- local.prefix = repo.prefix\n- local.hash = repo.hash\n- local.get_base_path = repo.get_base_path\n- local.exporter = GitExporter(local)\n- local.importer = GitImporter(local)\n-\n- return local\n-\n-\n-def do_capabilities(repo, args):\n- \"\"\"Prints the supported capabilities.\n- \"\"\"\n-\n- print \"import\"\n- print \"export\"\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, 'git.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-def do_list(repo, args):\n- \"\"\"Lists all known references.\n-\n- Bug: This will always set the remote head to master for non-local\n- repositories, since we have no way of determining what the remote\n- head is at clone time.\n- \"\"\"\n-\n- for ref in repo.revs:\n- debug(\"? refs/heads/%s\", ref)\n- print \"? refs/heads/%s\" % ref\n-\n- if repo.head:\n- debug(\"@refs/heads/%s HEAD\" % repo.head)\n- print \"@refs/heads/%s HEAD\" % repo.head\n- else:\n- debug(\"@refs/heads/master HEAD\")\n- print \"@refs/heads/master HEAD\"\n-\n- print # end list\n-\n-\n-def update_local_repo(repo):\n- \"\"\"Updates (or clones) a local repo.\n- \"\"\"\n-\n- if repo.local:\n- return repo\n-\n- path = repo.non_local.clone(repo.gitdir)\n- repo.non_local.update(repo.gitdir)\n- repo = local_repo(repo, path)\n- return repo\n-\n-\n-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-\n- if not repo.gitdir:\n- die(\"Need gitdir to import\")\n-\n- ref = args[0]\n- refs = [ref]\n-\n- while True:\n- line = sys.stdin.readline()\n- if line == '\\n':\n- break\n- if not line.startswith('import '):\n- die(\"Expected import line.\")\n-\n- # strip of leading 'import '\n- ref = line[7:].strip()\n- refs.append(ref)\n-\n- repo = update_local_repo(repo)\n- repo.exporter.export_repo(repo.gitdir, refs)\n-\n- print \"done\"\n-\n-\n-def do_export(repo, args):\n- \"\"\"Imports a fast-import stream from git to testgit.\n- \"\"\"\n-\n- if not repo.gitdir:\n- die(\"Need gitdir to export\")\n-\n- update_local_repo(repo)\n- changed = repo.importer.do_import(repo.gitdir)\n-\n- if not repo.local:\n- repo.non_local.push(repo.gitdir)\n-\n- for ref in changed:\n- print \"ok %s\" % ref\n- print\n-\n-\n-COMMANDS = {\n- 'capabilities': do_capabilities,\n- 'list': do_list,\n- 'import': do_import,\n- 'export': do_export,\n-}\n-\n-\n-def sanitize(value):\n- \"\"\"Cleans up the url.\n- \"\"\"\n-\n- if value.startswith('testgit::'):\n- value = value[9:]\n-\n- return value\n-\n-\n-def read_one_line(repo):\n- \"\"\"Reads and processes one command.\n- \"\"\"\n-\n- sleepy = os.environ.get(\"GIT_REMOTE_TESTGIT_SLEEPY\")\n- if sleepy:\n- debug(\"Sleeping %d sec before readline\" % int(sleepy))\n- time.sleep(int(sleepy))\n-\n- line = sys.stdin.readline()\n-\n- cmdline = line\n-\n- if not cmdline:\n- warn(\"Unexpected EOF\")\n- return False\n-\n- cmdline = cmdline.strip().split()\n- if not cmdline:\n- # Blank line means we're about to quit\n- return False\n-\n- cmd = cmdline.pop(0)\n- debug(\"Got command '%s' with args '%s'\", cmd, ' '.join(cmdline))\n-\n- if cmd not in COMMANDS:\n- die(\"Unknown command, %s\", cmd)\n-\n- func = COMMANDS[cmd]\n- func(repo, cmdline)\n- sys.stdout.flush()\n-\n- return True\n-\n-\n-def main(args):\n- \"\"\"Starts a new remote helper for the specified repository.\n- \"\"\"\n-\n- if len(args) != 3:\n- die(\"Expecting exactly three arguments.\")\n- sys.exit(1)\n-\n- if os.getenv(\"GIT_DEBUG_TESTGIT\"):\n- import git_remote_helpers.util\n- git_remote_helpers.util.DEBUG = True\n-\n- alias = sanitize(args[1])\n- url = sanitize(args[2])\n-\n- if not alias.isalnum():\n- warn(\"non-alnum alias '%s'\", alias)\n- alias = \"tmp\"\n-\n- args[1] = alias\n- args[2] = url\n-\n- repo = get_repo(alias, url)\n-\n- debug(\"Got arguments %s\", args[1:])\n-\n- more = True\n-\n- sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)\n- while (more):\n- more = read_one_line(repo)\n-\n-if __name__ == '__main__':\n- sys.exit(main(sys.argv))\ndiff --git a/git-remote-testpy.py b/git-remote-testpy.py\nnew file mode 100644\nindex 0000000..ade797b\n--- /dev/null\n+++ b/git-remote-testpy.py\n@@ -0,0 +1,272 @@\n+#!/usr/bin/env python\n+\n+# This command is a simple remote-helper, that is used both as a\n+# testcase for the remote-helper functionality, and as an example to\n+# show remote-helper authors one possible implementation.\n+#\n+# This is a Git <-> Git importer/exporter, that simply uses git\n+# fast-import and git fast-export to consume and produce fast-import\n+# streams.\n+#\n+# To understand better the way things work, one can activate debug\n+# traces by setting (to any value) the environment variables\n+# GIT_TRANSPORT_HELPER_DEBUG and GIT_DEBUG_TESTGIT, to see messages\n+# from the transport-helper side, or from this example remote-helper.\n+\n+# hashlib is only available in python >= 2.5\n+try:\n+ import hashlib\n+ _digest = hashlib.sha1\n+except ImportError:\n+ import sha\n+ _digest = sha.new\n+import sys\n+import os\n+import time\n+sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"))\n+\n+from git_remote_helpers.util import die, debug, warn\n+from git_remote_helpers.git.repo import GitRepo\n+from git_remote_helpers.git.exporter import GitExporter\n+from git_remote_helpers.git.importer import GitImporter\n+from git_remote_helpers.git.non_local import NonLocalGit\n+\n+def get_repo(alias, url):\n+ \"\"\"Returns a git repository object initialized for usage.\n+ \"\"\"\n+\n+ repo = GitRepo(url)\n+ repo.get_revs()\n+ repo.get_head()\n+\n+ hasher = _digest()\n+ hasher.update(repo.path)\n+ repo.hash = hasher.hexdigest()\n+\n+ repo.get_base_path = lambda base: os.path.join(\n+ base, 'info', 'fast-import', repo.hash)\n+\n+ prefix = 'refs/testgit/%s/' % alias\n+ debug(\"prefix: '%s'\", prefix)\n+\n+ repo.gitdir = os.environ[\"GIT_DIR\"]\n+ repo.alias = alias\n+ repo.prefix = prefix\n+\n+ repo.exporter = GitExporter(repo)\n+ repo.importer = GitImporter(repo)\n+ repo.non_local = NonLocalGit(repo)\n+\n+ return repo\n+\n+\n+def local_repo(repo, path):\n+ \"\"\"Returns a git repository object initalized for usage.\n+ \"\"\"\n+\n+ local = GitRepo(path)\n+\n+ local.non_local = None\n+ local.gitdir = repo.gitdir\n+ local.alias = repo.alias\n+ local.prefix = repo.prefix\n+ local.hash = repo.hash\n+ local.get_base_path = repo.get_base_path\n+ local.exporter = GitExporter(local)\n+ local.importer = GitImporter(local)\n+\n+ return local\n+\n+\n+def do_capabilities(repo, args):\n+ \"\"\"Prints the supported capabilities.\n+ \"\"\"\n+\n+ print \"import\"\n+ print \"export\"\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, 'git.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+def do_list(repo, args):\n+ \"\"\"Lists all known references.\n+\n+ Bug: This will always set the remote head to master for non-local\n+ repositories, since we have no way of determining what the remote\n+ head is at clone time.\n+ \"\"\"\n+\n+ for ref in repo.revs:\n+ debug(\"? refs/heads/%s\", ref)\n+ print \"? refs/heads/%s\" % ref\n+\n+ if repo.head:\n+ debug(\"@refs/heads/%s HEAD\" % repo.head)\n+ print \"@refs/heads/%s HEAD\" % repo.head\n+ else:\n+ debug(\"@refs/heads/master HEAD\")\n+ print \"@refs/heads/master HEAD\"\n+\n+ print # end list\n+\n+\n+def update_local_repo(repo):\n+ \"\"\"Updates (or clones) a local repo.\n+ \"\"\"\n+\n+ if repo.local:\n+ return repo\n+\n+ path = repo.non_local.clone(repo.gitdir)\n+ repo.non_local.update(repo.gitdir)\n+ repo = local_repo(repo, path)\n+ return repo\n+\n+\n+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+\n+ if not repo.gitdir:\n+ die(\"Need gitdir to import\")\n+\n+ ref = args[0]\n+ refs = [ref]\n+\n+ while True:\n+ line = sys.stdin.readline()\n+ if line == '\\n':\n+ break\n+ if not line.startswith('import '):\n+ die(\"Expected import line.\")\n+\n+ # strip of leading 'import '\n+ ref = line[7:].strip()\n+ refs.append(ref)\n+\n+ repo = update_local_repo(repo)\n+ repo.exporter.export_repo(repo.gitdir, refs)\n+\n+ print \"done\"\n+\n+\n+def do_export(repo, args):\n+ \"\"\"Imports a fast-import stream from git to testgit.\n+ \"\"\"\n+\n+ if not repo.gitdir:\n+ die(\"Need gitdir to export\")\n+\n+ update_local_repo(repo)\n+ changed = repo.importer.do_import(repo.gitdir)\n+\n+ if not repo.local:\n+ repo.non_local.push(repo.gitdir)\n+\n+ for ref in changed:\n+ print \"ok %s\" % ref\n+ print\n+\n+\n+COMMANDS = {\n+ 'capabilities': do_capabilities,\n+ 'list': do_list,\n+ 'import': do_import,\n+ 'export': do_export,\n+}\n+\n+\n+def sanitize(value):\n+ \"\"\"Cleans up the url.\n+ \"\"\"\n+\n+ if value.startswith('testgit::'):\n+ value = value[9:]\n+\n+ return value\n+\n+\n+def read_one_line(repo):\n+ \"\"\"Reads and processes one command.\n+ \"\"\"\n+\n+ sleepy = os.environ.get(\"GIT_REMOTE_TESTGIT_SLEEPY\")\n+ if sleepy:\n+ debug(\"Sleeping %d sec before readline\" % int(sleepy))\n+ time.sleep(int(sleepy))\n+\n+ line = sys.stdin.readline()\n+\n+ cmdline = line\n+\n+ if not cmdline:\n+ warn(\"Unexpected EOF\")\n+ return False\n+\n+ cmdline = cmdline.strip().split()\n+ if not cmdline:\n+ # Blank line means we're about to quit\n+ return False\n+\n+ cmd = cmdline.pop(0)\n+ debug(\"Got command '%s' with args '%s'\", cmd, ' '.join(cmdline))\n+\n+ if cmd not in COMMANDS:\n+ die(\"Unknown command, %s\", cmd)\n+\n+ func = COMMANDS[cmd]\n+ func(repo, cmdline)\n+ sys.stdout.flush()\n+\n+ return True\n+\n+\n+def main(args):\n+ \"\"\"Starts a new remote helper for the specified repository.\n+ \"\"\"\n+\n+ if len(args) != 3:\n+ die(\"Expecting exactly three arguments.\")\n+ sys.exit(1)\n+\n+ if os.getenv(\"GIT_DEBUG_TESTGIT\"):\n+ import git_remote_helpers.util\n+ git_remote_helpers.util.DEBUG = True\n+\n+ alias = sanitize(args[1])\n+ url = sanitize(args[2])\n+\n+ if not alias.isalnum():\n+ warn(\"non-alnum alias '%s'\", alias)\n+ alias = \"tmp\"\n+\n+ args[1] = alias\n+ args[2] = url\n+\n+ repo = get_repo(alias, url)\n+\n+ debug(\"Got arguments %s\", args[1:])\n+\n+ more = True\n+\n+ sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)\n+ while (more):\n+ more = read_one_line(repo)\n+\n+if __name__ == '__main__':\n+ sys.exit(main(sys.argv))\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\ndeleted file mode 100755\nindex d46fa40..0000000\n--- a/t/t5800-remote-helpers.sh\n+++ /dev/null\n@@ -1,148 +0,0 @@\n-#!/bin/sh\n-#\n-# Copyright (c) 2010 Sverre Rabbelier\n-#\n-\n-test_description='Test remote-helper import and export commands'\n-\n-. ./test-lib.sh\n-\n-if ! test_have_prereq PYTHON ; then\n-\tskip_all='skipping remote-testgit tests, python not available'\n-\ttest_done\n-fi\n-\n-\"$PYTHON_PATH\" -c '\n-import sys\n-if sys.hexversion < 0x02040000:\n- sys.exit(1)\n-' || {\n-\tskip_all='skipping remote-testgit tests, python version < 2.4'\n-\ttest_done\n-}\n-\n-compare_refs() {\n-\tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n-\tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n-\ttest_cmp expect actual\n-}\n-\n-test_expect_success 'setup repository' '\n-\tgit init --bare server/.git &&\n-\tgit clone server public &&\n-\t(cd public &&\n-\t echo content >file &&\n-\t git add file &&\n-\t git commit -m one &&\n-\t git push origin master)\n-'\n-\n-test_expect_success 'cloning from local repo' '\n-\tgit clone \"testgit::${PWD}/server\" localclone &&\n-\ttest_cmp public/file localclone/file\n-'\n-\n-test_expect_success 'cloning from remote repo' '\n-\tgit clone \"testgit::file://${PWD}/server\" clone &&\n-\ttest_cmp public/file clone/file\n-'\n-\n-test_expect_success 'create new commit on remote' '\n-\t(cd public &&\n-\t echo content >>file &&\n-\t git commit -a -m two &&\n-\t git push)\n-'\n-\n-test_expect_success 'pulling from local repo' '\n-\t(cd localclone && git pull) &&\n-\ttest_cmp public/file localclone/file\n-'\n-\n-test_expect_success 'pulling from remote remote' '\n-\t(cd clone && git pull) &&\n-\ttest_cmp public/file clone/file\n-'\n-\n-test_expect_success 'pushing to local repo' '\n-\t(cd localclone &&\n-\techo content >>file &&\n-\tgit commit -a -m three &&\n-\tgit push) &&\n-\tcompare_refs localclone HEAD server HEAD\n-'\n-\n-# Generally, skip this test. It demonstrates a now-fixed race in\n-# git-remote-testgit, but is too slow to leave in for general use.\n-: test_expect_success 'racily pushing to local repo' '\n-\ttest_when_finished \"rm -rf server2 localclone2\" &&\n-\tcp -R server server2 &&\n-\tgit clone \"testgit::${PWD}/server2\" localclone2 &&\n-\t(cd localclone2 &&\n-\techo content >>file &&\n-\tgit commit -a -m three &&\n-\tGIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&\n-\tcompare_refs localclone2 HEAD server2 HEAD\n-'\n-\n-test_expect_success 'synch with changes from localclone' '\n-\t(cd clone &&\n-\t git pull)\n-'\n-\n-test_expect_success 'pushing remote local repo' '\n-\t(cd clone &&\n-\techo content >>file &&\n-\tgit commit -a -m four &&\n-\tgit push) &&\n-\tcompare_refs clone HEAD server HEAD\n-'\n-\n-test_expect_success '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_success '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_success '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_success 'push new branch by name' '\n-\t(cd clone &&\n-\t git checkout -b new-name &&\n-\t echo content >>file &&\n-\t git commit -a -m seven &&\n-\t git push origin new-name\n-\t) &&\n-\tcompare_refs clone HEAD server refs/heads/new-name\n-'\n-\n-test_expect_failure 'push new branch with old:new refspec' '\n-\t(cd clone &&\n-\t git push origin new-name:new-refspec\n-\t) &&\n-\tcompare_refs clone HEAD server refs/heads/new-refspec\n-'\n-\n-test_done\ndiff --git a/t/t5800-remote-testpy.sh b/t/t5800-remote-testpy.sh\nnew file mode 100755\nindex 0000000..6750961\n--- /dev/null\n+++ b/t/t5800-remote-testpy.sh\n@@ -0,0 +1,148 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Sverre Rabbelier\n+#\n+\n+test_description='Test python remote-helper framework'\n+\n+. ./test-lib.sh\n+\n+if ! test_have_prereq PYTHON ; then\n+\tskip_all='skipping python remote-helper tests, python not available'\n+\ttest_done\n+fi\n+\n+\"$PYTHON_PATH\" -c '\n+import sys\n+if sys.hexversion < 0x02040000:\n+ sys.exit(1)\n+' || {\n+\tskip_all='skipping python remote-helper tests, python version < 2.4'\n+\ttest_done\n+}\n+\n+compare_refs() {\n+\tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n+\tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'setup repository' '\n+\tgit init --bare server/.git &&\n+\tgit clone server public &&\n+\t(cd public &&\n+\t echo content >file &&\n+\t git add file &&\n+\t git commit -m one &&\n+\t git push origin master)\n+'\n+\n+test_expect_success 'cloning from local repo' '\n+\tgit clone \"testpy::${PWD}/server\" localclone &&\n+\ttest_cmp public/file localclone/file\n+'\n+\n+test_expect_success 'cloning from remote repo' '\n+\tgit clone \"testpy::file://${PWD}/server\" clone &&\n+\ttest_cmp public/file clone/file\n+'\n+\n+test_expect_success 'create new commit on remote' '\n+\t(cd public &&\n+\t echo content >>file &&\n+\t git commit -a -m two &&\n+\t git push)\n+'\n+\n+test_expect_success 'pulling from local repo' '\n+\t(cd localclone && git pull) &&\n+\ttest_cmp public/file localclone/file\n+'\n+\n+test_expect_success 'pulling from remote remote' '\n+\t(cd clone && git pull) &&\n+\ttest_cmp public/file clone/file\n+'\n+\n+test_expect_success 'pushing to local repo' '\n+\t(cd localclone &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tgit push) &&\n+\tcompare_refs localclone HEAD server HEAD\n+'\n+\n+# Generally, skip this test. It demonstrates a now-fixed race in\n+# git-remote-testpy, but is too slow to leave in for general use.\n+: test_expect_success 'racily pushing to local repo' '\n+\ttest_when_finished \"rm -rf server2 localclone2\" &&\n+\tcp -R server server2 &&\n+\tgit clone \"testpy::${PWD}/server2\" localclone2 &&\n+\t(cd localclone2 &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tGIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&\n+\tcompare_refs localclone2 HEAD server2 HEAD\n+'\n+\n+test_expect_success 'synch with changes from localclone' '\n+\t(cd clone &&\n+\t git pull)\n+'\n+\n+test_expect_success 'pushing remote local repo' '\n+\t(cd clone &&\n+\techo content >>file &&\n+\tgit commit -a -m four &&\n+\tgit push) &&\n+\tcompare_refs clone HEAD server HEAD\n+'\n+\n+test_expect_success '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_success '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_success '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_success 'push new branch by name' '\n+\t(cd clone &&\n+\t git checkout -b new-name &&\n+\t echo content >>file &&\n+\t git commit -a -m seven &&\n+\t git push origin new-name\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-name\n+'\n+\n+test_expect_failure 'push new branch with old:new refspec' '\n+\t(cd clone &&\n+\t git push origin new-name:new-refspec\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-refspec\n+'\n+\n+test_done\n-- \n1.8.0\n"},{"id":"202835","messageId":"1352642392-28387-6-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 05/15] Add new simplified git-remote-testgit","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:42Z","receivedAt":"2012-11-11T13:59:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's way simpler. It exerceises the same features of remote helpers.\nIt's easy to read and understand. It doesn't depend on python.\n\nIt does _not_ exercise the python remote helper framework; there's\nanother tool and another test for that.\n\nFor now let's just copy the old remote-helpers test script, although\nsome of those tests don't make sense for this testgit (they still pass).\n\nIn addition, this script would be able to test other features not\ncurrently being tested.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-remote-testgit.txt | 2 +-\n git-remote-testgit | 62 ++++++++++++++++\n t/t5801-remote-helpers.sh | 139 +++++++++++++++++++++++++++++++++++\n 3 files changed, 202 insertions(+), 1 deletion(-)\n create mode 100755 git-remote-testgit\n create mode 100755 t/t5801-remote-helpers.sh\n\ndiff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt\nindex 2a67d45..612a625 100644\n--- a/Documentation/git-remote-testgit.txt\n+++ b/Documentation/git-remote-testgit.txt\n@@ -19,7 +19,7 @@ testcase for the remote-helper functionality, and as an example to\n show remote-helper authors one possible implementation.\n \n The best way to learn more is to read the comments and source code in\n-'git-remote-testgit.py'.\n+'git-remote-testgit'.\n \n SEE ALSO\n --------\ndiff --git a/git-remote-testgit b/git-remote-testgit\nnew file mode 100755\nindex 0000000..83ccb86\n--- /dev/null\n+++ b/git-remote-testgit\n@@ -0,0 +1,62 @@\n+#!/bin/bash\n+# Copyright (c) 2012 Felipe Contreras\n+\n+alias=$1\n+url=$2\n+\n+# huh?\n+url=\"${url#file://}\"\n+\n+dir=\"$GIT_DIR/testgit/$alias\"\n+prefix=\"refs/testgit/$alias\"\n+refspec=\"refs/heads/*:${prefix}/heads/*\"\n+\n+gitmarks=\"$dir/git.marks\"\n+testgitmarks=\"$dir/testgit.marks\"\n+\n+export GIT_DIR=\"$url/.git\"\n+\n+mkdir -p \"$dir\"\n+\n+test -e \"$gitmarks\" || > \"$gitmarks\"\n+test -e \"$testgitmarks\" || > \"$testgitmarks\"\n+\n+while read line; do\n+\tcase $line in\n+\tcapabilities)\n+\t\techo 'import'\n+\t\techo 'export'\n+\t\techo \"refspec $refspec\"\n+\t\techo \"*import-marks $gitmarks\"\n+\t\techo \"*export-marks $gitmarks\"\n+\t\techo\n+\t\t;;\n+\tlist)\n+\t\tgit for-each-ref --format='? %(refname)' 'refs/heads/'\n+\t\thead=$(git symbolic-ref HEAD)\n+\t\techo \"@$head HEAD\"\n+\t\techo\n+\t\t;;\n+\timport*)\n+\t\t# read all import lines\n+\t\twhile true; do\n+\t\t\tref=\"${line#* }\"\n+\t\t\trefs=\"$refs $ref\"\n+\t\t\tread line\n+\t\t\ttest \"${line%% *}\" != \"import\" && break\n+\t\tdone\n+\n+\t\techo \"feature import-marks=$gitmarks\"\n+\t\techo \"feature export-marks=$gitmarks\"\n+\t\tgit fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n+\t\t\tsed -e \"s#refs/heads/#${prefix}/heads/#g\"\n+\t\t;;\n+\texport)\n+\t\tgit fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n+\t\techo\n+\t\t;;\n+\t'')\n+\t\texit\n+\t\t;;\n+\tesac\n+done\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nnew file mode 100755\nindex 0000000..f52ab14\n--- /dev/null\n+++ b/t/t5801-remote-helpers.sh\n@@ -0,0 +1,139 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Sverre Rabbelier\n+#\n+\n+test_description='Test remote-helper import and export commands'\n+\n+. ./test-lib.sh\n+\n+if ! type \"${BASH-bash}\" >/dev/null 2>&1; then\n+\tskip_all='skipping remote-testgit tests, bash not available'\n+\ttest_done\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 'setup repository' '\n+\tgit init --bare server/.git &&\n+\tgit clone server public &&\n+\t(cd public &&\n+\t echo content >file &&\n+\t git add file &&\n+\t git commit -m one &&\n+\t git push origin master)\n+'\n+\n+test_expect_success 'cloning from local repo' '\n+\tgit clone \"testgit::${PWD}/server\" localclone &&\n+\ttest_cmp public/file localclone/file\n+'\n+\n+test_expect_success 'cloning from remote repo' '\n+\tgit clone \"testgit::file://${PWD}/server\" clone &&\n+\ttest_cmp public/file clone/file\n+'\n+\n+test_expect_success 'create new commit on remote' '\n+\t(cd public &&\n+\t echo content >>file &&\n+\t git commit -a -m two &&\n+\t git push)\n+'\n+\n+test_expect_success 'pulling from local repo' '\n+\t(cd localclone && git pull) &&\n+\ttest_cmp public/file localclone/file\n+'\n+\n+test_expect_success 'pulling from remote remote' '\n+\t(cd clone && git pull) &&\n+\ttest_cmp public/file clone/file\n+'\n+\n+test_expect_success 'pushing to local repo' '\n+\t(cd localclone &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tgit push) &&\n+\tcompare_refs localclone HEAD server HEAD\n+'\n+\n+# Generally, skip this test. It demonstrates a now-fixed race in\n+# git-remote-testgit, but is too slow to leave in for general use.\n+: test_expect_success 'racily pushing to local repo' '\n+\ttest_when_finished \"rm -rf server2 localclone2\" &&\n+\tcp -R server server2 &&\n+\tgit clone \"testgit::${PWD}/server2\" localclone2 &&\n+\t(cd localclone2 &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tGIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&\n+\tcompare_refs localclone2 HEAD server2 HEAD\n+'\n+\n+test_expect_success 'synch with changes from localclone' '\n+\t(cd clone &&\n+\t git pull)\n+'\n+\n+test_expect_success 'pushing remote local repo' '\n+\t(cd clone &&\n+\techo content >>file &&\n+\tgit commit -a -m four &&\n+\tgit push) &&\n+\tcompare_refs clone HEAD server HEAD\n+'\n+\n+test_expect_success '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_success '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_success '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_success 'push new branch by name' '\n+\t(cd clone &&\n+\t git checkout -b new-name &&\n+\t echo content >>file &&\n+\t git commit -a -m seven &&\n+\t git push origin new-name\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-name\n+'\n+\n+test_expect_failure 'push new branch with old:new refspec' '\n+\t(cd clone &&\n+\t git push origin new-name:new-refspec\n+\t) &&\n+\tcompare_refs clone HEAD server refs/heads/new-refspec\n+'\n+\n+test_done\n-- \n1.8.0\n"},{"id":"202836","messageId":"1352642392-28387-7-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 06/15] remote-testgit: get rid of non-local functionality","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:43Z","receivedAt":"2012-11-11T13:59:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This only makes sense for the python remote helpers framework.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit | 3 ---\n t/t5801-remote-helpers.sh | 50 ++++++++++++++++++++---------------------------\n 2 files changed, 21 insertions(+), 32 deletions(-)\n\ndiff --git a/git-remote-testgit b/git-remote-testgit\nindex 83ccb86..fe73c36 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -4,9 +4,6 @@\n alias=$1\n url=$2\n \n-# huh?\n-url=\"${url#file://}\"\n-\n dir=\"$GIT_DIR/testgit/$alias\"\n prefix=\"refs/testgit/$alias\"\n refspec=\"refs/heads/*:${prefix}/heads/*\"\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex f52ab14..2f7fc10 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -33,11 +33,6 @@ test_expect_success 'cloning from local repo' '\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success 'cloning from remote repo' '\n-\tgit clone \"testgit::file://${PWD}/server\" clone &&\n-\ttest_cmp public/file clone/file\n-'\n-\n test_expect_success 'create new commit on remote' '\n \t(cd public &&\n \t echo content >>file &&\n@@ -50,11 +45,6 @@ test_expect_success 'pulling from local repo' '\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success 'pulling from remote remote' '\n-\t(cd clone && git pull) &&\n-\ttest_cmp public/file clone/file\n-'\n-\n test_expect_success 'pushing to local repo' '\n \t(cd localclone &&\n \techo content >>file &&\n@@ -76,19 +66,6 @@ test_expect_success 'pushing to local repo' '\n \tcompare_refs localclone2 HEAD server2 HEAD\n '\n \n-test_expect_success 'synch with changes from localclone' '\n-\t(cd clone &&\n-\t git pull)\n-'\n-\n-test_expect_success 'pushing remote local repo' '\n-\t(cd clone &&\n-\techo content >>file &&\n-\tgit commit -a -m four &&\n-\tgit push) &&\n-\tcompare_refs clone HEAD server HEAD\n-'\n-\n test_expect_success 'fetch new branch' '\n \t(cd public &&\n \t git checkout -b new &&\n@@ -102,6 +79,20 @@ test_expect_success 'fetch new branch' '\n \tcompare_refs public HEAD localclone FETCH_HEAD\n '\n \n+#\n+# This is only needed because of a bug not detected by this script. It will be\n+# fixed shortly, but for now lets not cause regressions.\n+#\n+test_expect_success 'bump commit in public' '\n+\t(cd public &&\n+\tgit checkout master &&\n+\tgit pull &&\n+\techo content >>file &&\n+\tgit commit -a -m four &&\n+\tgit push) &&\n+\tcompare_refs public HEAD server HEAD\n+'\n+\n test_expect_success 'fetch multiple branches' '\n \t(cd localclone &&\n \t git fetch\n@@ -111,29 +102,30 @@ test_expect_success 'fetch multiple branches' '\n '\n \n test_expect_success 'push when remote has extra refs' '\n-\t(cd clone &&\n+\t(cd localclone &&\n+\t git reset --hard origin/master &&\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+\tcompare_refs localclone master server master\n '\n \n test_expect_success 'push new branch by name' '\n-\t(cd clone &&\n+\t(cd localclone &&\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+\tcompare_refs localclone HEAD server refs/heads/new-name\n '\n \n test_expect_failure 'push new branch with old:new refspec' '\n-\t(cd clone &&\n+\t(cd localclone &&\n \t git push origin new-name:new-refspec\n \t) &&\n-\tcompare_refs clone HEAD server refs/heads/new-refspec\n+\tcompare_refs localclone HEAD server refs/heads/new-refspec\n '\n \n test_done\n-- \n1.8.0\n"},{"id":"202837","messageId":"1352642392-28387-8-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 07/15] remote-testgit: remove irrelevant test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:44Z","receivedAt":"2012-11-11T13:59:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Only makes sense for remote-testpy.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 13 -------------\n 1 file changed, 13 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 2f7fc10..6801529 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -53,19 +53,6 @@ test_expect_success 'pushing to local repo' '\n \tcompare_refs localclone HEAD server HEAD\n '\n \n-# Generally, skip this test. It demonstrates a now-fixed race in\n-# git-remote-testgit, but is too slow to leave in for general use.\n-: test_expect_success 'racily pushing to local repo' '\n-\ttest_when_finished \"rm -rf server2 localclone2\" &&\n-\tcp -R server server2 &&\n-\tgit clone \"testgit::${PWD}/server2\" localclone2 &&\n-\t(cd localclone2 &&\n-\techo content >>file &&\n-\tgit commit -a -m three &&\n-\tGIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&\n-\tcompare_refs localclone2 HEAD server2 HEAD\n-'\n-\n test_expect_success 'fetch new branch' '\n \t(cd public &&\n \t git checkout -b new &&\n-- \n1.8.0\n"},{"id":"202838","messageId":"1352642392-28387-9-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 08/15] remote-testgit: cleanup tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:45Z","receivedAt":"2012-11-11T13:59:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We don't need a bare 'server' and an intermediary 'public'. The repos\ncan talk to each other directly; that's what we want to exercise.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 63 ++++++++++++++++++++++-------------------------\n 1 file changed, 29 insertions(+), 34 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 6801529..bc0b5f7 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -19,100 +19,95 @@ compare_refs() {\n }\n \n test_expect_success 'setup repository' '\n-\tgit init --bare server/.git &&\n-\tgit clone server public &&\n-\t(cd public &&\n+\tgit init server &&\n+\t(cd server &&\n \t echo content >file &&\n \t git add file &&\n-\t git commit -m one &&\n-\t git push origin master)\n+\t git commit -m one)\n '\n \n test_expect_success 'cloning from local repo' '\n-\tgit clone \"testgit::${PWD}/server\" localclone &&\n-\ttest_cmp public/file localclone/file\n+\tgit clone \"testgit::${PWD}/server\" local &&\n+\ttest_cmp server/file local/file\n '\n \n test_expect_success 'create new commit on remote' '\n-\t(cd public &&\n+\t(cd server &&\n \t echo content >>file &&\n-\t git commit -a -m two &&\n-\t git push)\n+\t git commit -a -m two)\n '\n \n test_expect_success 'pulling from local repo' '\n-\t(cd localclone && git pull) &&\n-\ttest_cmp public/file localclone/file\n+\t(cd local && git pull) &&\n+\ttest_cmp server/file local/file\n '\n \n test_expect_success 'pushing to local repo' '\n-\t(cd localclone &&\n+\t(cd local &&\n \techo content >>file &&\n \tgit commit -a -m three &&\n \tgit push) &&\n-\tcompare_refs localclone HEAD server HEAD\n+\tcompare_refs local HEAD server HEAD\n '\n \n test_expect_success 'fetch new branch' '\n-\t(cd public &&\n+\t(cd server &&\n+\t git reset --hard &&\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 git commit -a -m five\n \t) &&\n-\t(cd localclone &&\n+\t(cd local &&\n \t git fetch origin new\n \t) &&\n-\tcompare_refs public HEAD localclone FETCH_HEAD\n+\tcompare_refs server HEAD local FETCH_HEAD\n '\n \n #\n # This is only needed because of a bug not detected by this script. It will be\n # fixed shortly, but for now lets not cause regressions.\n #\n-test_expect_success 'bump commit in public' '\n-\t(cd public &&\n+test_expect_success 'bump commit in server' '\n+\t(cd server &&\n \tgit checkout master &&\n-\tgit pull &&\n \techo content >>file &&\n-\tgit commit -a -m four &&\n-\tgit push) &&\n-\tcompare_refs public HEAD server HEAD\n+\tgit commit -a -m four) &&\n+\tcompare_refs server HEAD server HEAD\n '\n \n test_expect_success 'fetch multiple branches' '\n-\t(cd localclone &&\n+\t(cd local &&\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+\tcompare_refs server master local refs/remotes/origin/master &&\n+\tcompare_refs server new local refs/remotes/origin/new\n '\n \n test_expect_success 'push when remote has extra refs' '\n-\t(cd localclone &&\n+\t(cd local &&\n \t git reset --hard origin/master &&\n \t echo content >>file &&\n \t git commit -a -m six &&\n \t git push\n \t) &&\n-\tcompare_refs localclone master server master\n+\tcompare_refs local master server master\n '\n \n test_expect_success 'push new branch by name' '\n-\t(cd localclone &&\n+\t(cd local &&\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 localclone HEAD server refs/heads/new-name\n+\tcompare_refs local HEAD server refs/heads/new-name\n '\n \n test_expect_failure 'push new branch with old:new refspec' '\n-\t(cd localclone &&\n+\t(cd local &&\n \t git push origin new-name:new-refspec\n \t) &&\n-\tcompare_refs localclone HEAD server refs/heads/new-refspec\n+\tcompare_refs local HEAD server refs/heads/new-refspec\n '\n \n test_done\n-- \n1.8.0\n"},{"id":"202840","messageId":"1352642392-28387-10-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 09/15] remote-testgit: exercise more features","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:46Z","receivedAt":"2012-11-11T13:59:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Unfortunately they do not work.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit | 18 +++++++++++++----\n t/t5801-remote-helpers.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 63 insertions(+), 4 deletions(-)\n mode change 100755 => 100644 t/t5801-remote-helpers.sh\n\ndiff --git a/git-remote-testgit b/git-remote-testgit\nindex fe73c36..31c7533 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -6,24 +6,34 @@ url=$2\n \n dir=\"$GIT_DIR/testgit/$alias\"\n prefix=\"refs/testgit/$alias\"\n-refspec=\"refs/heads/*:${prefix}/heads/*\"\n+\n+default_refspec=\"refs/heads/*:${prefix}/heads/*\"\n+\n+refspec=\"${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}\"\n \n gitmarks=\"$dir/git.marks\"\n testgitmarks=\"$dir/testgit.marks\"\n \n+test -z \"$refspec\" && prefix=\"refs\"\n+\n export GIT_DIR=\"$url/.git\"\n \n mkdir -p \"$dir\"\n \n-test -e \"$gitmarks\" || > \"$gitmarks\"\n-test -e \"$testgitmarks\" || > \"$testgitmarks\"\n+if [ -z \"$GIT_REMOTE_TESTGIT_NO_MARKS\" ]; then\n+\ttest -e \"$gitmarks\" || > \"$gitmarks\"\n+\ttest -e \"$testgitmarks\" || > \"$testgitmarks\"\n+else\n+\t> \"$gitmarks\"\n+\t> \"$testgitmarks\"\n+fi\n \n while read line; do\n \tcase $line in\n \tcapabilities)\n \t\techo 'import'\n \t\techo 'export'\n-\t\techo \"refspec $refspec\"\n+\t\ttest -n \"$refspec\" && echo \"refspec $refspec\"\n \t\techo \"*import-marks $gitmarks\"\n \t\techo \"*export-marks $gitmarks\"\n \t\techo\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nold mode 100755\nnew mode 100644\nindex bc0b5f7..31940c9\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -110,4 +110,53 @@ test_expect_failure 'push new branch with old:new refspec' '\n \tcompare_refs local HEAD server refs/heads/new-refspec\n '\n \n+test_expect_failure 'cloning without refspec' '\n+\tGIT_REMOTE_TESTGIT_REFSPEC=\"\" \\\n+\tgit clone \"testgit::${PWD}/server\" local2 &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pulling without refspecs' '\n+\t(cd local2 &&\n+\tgit reset --hard &&\n+\tGIT_REMOTE_TESTGIT_REFSPEC=\"\" git pull) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pushing without refspecs' '\n+\t(cd local2 &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tGIT_REMOTE_TESTGIT_REFSPEC=\"\" git push) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pulling with straight refspec' '\n+\t(cd local2 &&\n+\tGIT_REMOTE_TESTGIT_REFSPEC=\"*:*\" git pull) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pushing with straight refspec' '\n+\t(cd local2 &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tGIT_REMOTE_TESTGIT_REFSPEC=\"*:*\" git push) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pulling without marks' '\n+\t(cd local2 &&\n+\tGIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n+test_expect_failure 'pushing without marks' '\n+\t(cd local2 &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tGIT_REMOTE_TESTGIT_NO_MARKS=1 git push) &&\n+\tcompare_refs local2 HEAD server HEAD\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"202839","messageId":"1352642392-28387-11-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 10/15] remote-testgit: report success after an import","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:47Z","receivedAt":"2012-11-11T13:59:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Doesn't make a difference for the tests, but it does for the ones\nseeking reference.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/git-remote-testgit b/git-remote-testgit\nindex 31c7533..698effc 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -59,7 +59,18 @@ while read line; do\n \t\t\tsed -e \"s#refs/heads/#${prefix}/heads/#g\"\n \t\t;;\n \texport)\n+\t\tbefore=$(git for-each-ref --format='%(refname) %(objectname)')\n+\n \t\tgit fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n+\n+\t\tafter=$(git for-each-ref --format='%(refname) %(objectname)')\n+\n+\t\t# figure out which refs were updated\n+\t\tjoin -e 0 -o '0 1.2 2.2' -a 2 <(echo \"$before\") <(echo \"$after\") | while read ref a b; do\n+\t\t\ttest $a == $b && continue\n+\t\t\techo \"ok $ref\"\n+\t\tdone\n+\n \t\techo\n \t\t;;\n \t'')\n-- \n1.8.0\n"},{"id":"202841","messageId":"1352642392-28387-12-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:48Z","receivedAt":"2012-11-11T13:59:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"People seeking for reference would find it useful.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-remote-testgit b/git-remote-testgit\nindex 698effc..812321e 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -55,8 +55,10 @@ while read line; do\n \n \t\techo \"feature import-marks=$gitmarks\"\n \t\techo \"feature export-marks=$gitmarks\"\n-\t\tgit fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n+\t\techo \"feature done\"\n+\t\tgit fast-export --{import,export}-marks=\"$testgitmarks\" $refs | \\\n \t\t\tsed -e \"s#refs/heads/#${prefix}/heads/#g\"\n+\t\techo \"done\"\n \t\t;;\n \texport)\n \t\tbefore=$(git for-each-ref --format='%(refname) %(objectname)')\n-- \n1.8.0\n"},{"id":"202842","messageId":"1352642392-28387-13-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 12/15] fast-export: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:49Z","receivedAt":"2012-11-11T13:59:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Setting 'commit' to 'commit' is a no-op. It might have been there to\navoid a compiler warning, but if so, it was the compiler to blame, and\nit's certainly not there any more.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex a06fe10..4f3c35f 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \tfor (i = 0; i < pending->nr; i++) {\n \t\tstruct object_array_entry *e = pending->objects + i;\n \t\tunsigned char sha1[20];\n-\t\tstruct commit *commit = commit;\n+\t\tstruct commit *commit;\n \t\tchar *full_name;\n \n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)\n-- \n1.8.0\n"},{"id":"202843","messageId":"1352642392-28387-14-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 13/15] fast-export: fix comparison in tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:50Z","receivedAt":"2012-11-11T13:59:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"First the expected, then the actual, otherwise the diff would be the\nopposite of what we want.\n\nSigned-off-by: Felipe Contreras \n---\n t/t9350-fast-export.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 0c8d828..b7d3009 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '\n (\n \tcd limit-by-paths &&\n \tgit fast-export --tag-of-filtered-object=drop mytag -- there > output &&\n-\ttest_cmp output expected\n+\ttest_cmp expected output\n )\n '\n \n@@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '\n (\n \tcd limit-by-paths &&\n \tgit fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&\n-\ttest_cmp output expected\n+\ttest_cmp expected output\n )\n '\n \n@@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '\n \t(\n \t\tcd limit-by-paths &&\n \t\tgit fast-export master~2..master~1 > output &&\n-\t\ttest_cmp output expected\n+\t\ttest_cmp expected output\n \t)\n '\n \n-- \n1.8.0\n"},{"id":"202844","messageId":"1352642392-28387-15-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 14/15] fast-export: make sure updated refs get updated","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:51Z","receivedAt":"2012-11-11T13:59:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"When an object has already been exported (and thus is in the marks) it's\nflagged as SHOWN, so it will not be exported again, even if in a later\ntime it's exported through a different ref.\n\nWe don't need the object to be exported again, but we want the ref\nupdated, which doesn't happen.\n\nSince we can't know if a ref was exported or not, let's just assume that\nif the commit was marked (flags & SHOWN), the user still wants the ref\nupdated.\n\nIOW: If it's specified in the command line, it will get updated,\nregardless of wihether or not the object was marked.\n\nSo:\n\n % git branch test master\n % git fast-export $mark_flags master\n % git fast-export $mark_flags test\n\nWould export 'test' properly.\n\nAdditionally, this fixes issues with remote helpers; now they can push\nrefs wich objects have already been exported, and a few other issues as\nwell. So update the tests accordingly.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 10 +++++++---\n t/t5801-remote-helpers.sh | 24 ++++++++++--------------\n t/t9350-fast-export.sh | 15 +++++++++++++++\n 3 files changed, 32 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 4f3c35f..26f6d1c 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -523,10 +523,14 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t\t\t\ttypename(e->item->type));\n \t\t\tcontinue;\n \t\t}\n-\t\tif (commit->util)\n-\t\t\t/* more than one name for the same object */\n+\n+\t\t/*\n+\t\t * This ref will not be updated through a commit, lets make\n+\t\t * sure it gets properly upddated eventually.\n+\t\t */\n+\t\tif (commit->util || commit->object.flags & SHOWN)\n \t\t\tstring_list_append(extra_refs, full_name)->util = commit;\n-\t\telse\n+\t\tif (!commit->util)\n \t\t\tcommit->util = full_name;\n \t}\n }\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 31940c9..b6cc5c0 100644\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -63,18 +63,6 @@ test_expect_success 'fetch new branch' '\n \tcompare_refs server HEAD local FETCH_HEAD\n '\n \n-#\n-# This is only needed because of a bug not detected by this script. It will be\n-# fixed shortly, but for now lets not cause regressions.\n-#\n-test_expect_success 'bump commit in server' '\n-\t(cd server &&\n-\tgit checkout master &&\n-\techo content >>file &&\n-\tgit commit -a -m four) &&\n-\tcompare_refs server HEAD server HEAD\n-'\n-\n test_expect_success 'fetch multiple branches' '\n \t(cd local &&\n \t git fetch\n@@ -110,13 +98,13 @@ test_expect_failure 'push new branch with old:new refspec' '\n \tcompare_refs local HEAD server refs/heads/new-refspec\n '\n \n-test_expect_failure 'cloning without refspec' '\n+test_expect_success 'cloning without refspec' '\n \tGIT_REMOTE_TESTGIT_REFSPEC=\"\" \\\n \tgit clone \"testgit::${PWD}/server\" local2 &&\n \tcompare_refs local2 HEAD server HEAD\n '\n \n-test_expect_failure 'pulling without refspecs' '\n+test_expect_success 'pulling without refspecs' '\n \t(cd local2 &&\n \tgit reset --hard &&\n \tGIT_REMOTE_TESTGIT_REFSPEC=\"\" git pull) &&\n@@ -159,4 +147,12 @@ test_expect_failure 'pushing without marks' '\n \tcompare_refs local2 HEAD server HEAD\n '\n \n+test_expect_success 'push ref with existing object' '\n+\t(cd local &&\n+\tgit branch dup master &&\n+\tgit push origin dup\n+\t) &&\n+\tcompare_refs local dup server dup\n+'\n+\n test_done\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex b7d3009..67a7372 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -454,4 +454,19 @@ test_expect_success 'test biridectionality' '\n \tgit fast-import --export-marks=marks-cur --import-marks=marks-cur\n '\n \n+cat > expected << EOF\n+reset refs/heads/master\n+from :12\n+\n+EOF\n+\n+test_expect_success 'refs are updated even if no commits need to be exported' '\n+\techo -n > tmp-marks &&\n+\tgit fast-export --import-marks=tmp-marks \\\n+\t\t--export-marks=tmp-marks master > /dev/null &&\n+\tgit fast-export --import-marks=tmp-marks \\\n+\t\t--export-marks=tmp-marks master > actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"202845","messageId":"1352642392-28387-16-git-send-email-felipe.contreras@gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T13:59:52Z","receivedAt":"2012-11-11T13:59:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"They have been marked as UNINTERESTING for a reason, lets respect that.\n\nCurrently the first ref is handled properly, but not the rest, so:\n\n % git fast-export master ^master\n\nWould currently throw a reset for master (2nd ref), which is not what we\nwant.\n\n % git fast-export master ^foo ^bar ^roo\n % git fast-export master salsa..tacos\n\nEven if all these refs point to the same object; foo, bar, roo, salsa,\nand tacos would all get a reset, and to a non-existing object (invalid\nmark :0).\n\nAnd even more, it would only happen if the ref is pointing to exactly\nthe same commit, but not otherwise:\n\n % git fast-export ^next next\n reset refs/heads/next\n from :0\n\n % git fast-export ^next next^{commit}\n # nothing\n % git fast-export ^next next~0\n # nothing\n % git fast-export ^next next~1\n # nothing\n % git fast-export ^next next~2\n # nothing\n\nThe reason this happens is that before traversing the commits,\nfast-export checks if any of the refs point to the same object, and any\nduplicated ref gets added to a list in order to issue 'reset' commands\nafter the traversing. Unfortunately, it's not even checking if the\ncommit is flagged as UNINTERESTING. The fix of course, is to do\nprecisely that.\n\nThe current behavior is most certainly not what we want. After this\npatch, nothing gets exported, because nothing was selected (everything\nis UNINTERESTING).\n\nSigned-off-by: Felipe Contreras \n---\n builtin/fast-export.c | 4 +++-\n t/t9350-fast-export.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 26f6d1c..7a310e4 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -529,7 +529,9 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t\t * sure it gets properly upddated eventually.\n \t\t */\n \t\tif (commit->util || commit->object.flags & SHOWN)\n-\t\t\tstring_list_append(extra_refs, full_name)->util = commit;\n+\t\t\tif (!(commit->object.flags & UNINTERESTING))\n+\t\t\t\tstring_list_append(extra_refs, full_name)->util = commit;\n+\n \t\tif (!commit->util)\n \t\t\tcommit->util = full_name;\n \t}\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 67a7372..9b53ba7 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -469,4 +469,10 @@ test_expect_success 'refs are updated even if no commits need to be exported' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'proper extra refs handling' '\n+\tgit fast-export master ^master master..master > actual &&\n+\techo -n > expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"202871","messageId":"509FD425.5030702@web.de","threadId":"32071","inReplyTo":"1352642392-28387-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 01/15] fast-export: avoid importing blob marks","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-11-11T16:36:53Z","receivedAt":"2012-11-11T16:36:53Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11.11.12 14:59, Felipe Contreras wrote:\n> test_expect_success 'test biridectionality' '\n> +\techo -n > marks-cur &&\n> +\techo -n > marks-new &&\nUnless I messed up the patch:\n\nMinor issue: still a typo \"biridectionality\"\nMajor issue: \"echo -n\" is still not portable.\n\nCould we simply use\n\ntouch marks-cur &&\ntouch marks-new\n\n\n/Torsten\n"},{"id":"202872","messageId":"20121111163827.GA11408@sigill.intra.peff.net","threadId":"32071","inReplyTo":"509FD425.5030702@web.de","subject":"Re: [PATCH v5 01/15] fast-export: avoid importing blob marks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:38:28Z","receivedAt":"2012-11-11T16:38:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 11, 2012 at 05:36:53PM +0100, Torsten Bögershausen wrote:\n\n> On 11.11.12 14:59, Felipe Contreras wrote:\n> > test_expect_success 'test biridectionality' '\n> > +\techo -n > marks-cur &&\n> > +\techo -n > marks-new &&\n> Unless I messed up the patch:\n> \n> Minor issue: still a typo \"biridectionality\"\n> Major issue: \"echo -n\" is still not portable.\n> \n> Could we simply use\n> \n> touch marks-cur &&\n> touch marks-new\n\nYes, \"echo -n\" is definitely not portable. Our preferred way of\ncreating an empty file is just \">file\".\n\n-Peff\n"},{"id":"202904","messageId":"CAMP44s1BKD8NO5QSg2G_H6OM2vV7=h9T7r14zPVfZ7+3ddXFBA@mail.gmail.com","threadId":"32071","inReplyTo":"509FD425.5030702@web.de","subject":"Re: [PATCH v5 01/15] fast-export: avoid importing blob marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T17:53:05Z","receivedAt":"2012-11-11T17:53:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 5:36 PM, Torsten Bögershausen wrote:\n> On 11.11.12 14:59, Felipe Contreras wrote:\n>> test_expect_success 'test biridectionality' '\n>> + echo -n > marks-cur &&\n>> + echo -n > marks-new &&\n> Unless I messed up the patch:\n>\n> Minor issue: still a typo \"biridectionality\"\n> Major issue: \"echo -n\" is still not portable.\n\nYeah.\n\n> Could we simply use\n>\n> touch marks-cur &&\n> touch marks-new\n\nUnless somebody else already modified those marks. Better be safe than sorry.\n\n-- \nFelipe Contreras\n"},{"id":"202920","messageId":"82C1BB44-EA99-47E4-91E3-36F4D17BAFBA@quendi.de","threadId":"32071","inReplyTo":"1352642392-28387-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 02/15] remote-testgit: fix direction of marks","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-11T20:39:28Z","receivedAt":"2012-11-11T20:39:28Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 11.11.2012, at 14:59, Felipe Contreras wrote:\n\n> Basically this is what we want:\n> \n> == pull ==\n> \n> \ttestgit\t\t\ttransport-helper\n> \n> \t* export ->\t\timport\n> \n> \t# testgit.marks\t\tgit.marks\n> \n> == push ==\n> \n> \ttestgit\t\t\ttransport-helper\n> \n> \t* import\t\t<- export\n> \n> \t# testgit.marks\t\tgit.marks\n> \n> Each side should be agnostic of the other side. Because testgit.marks\n> (our helper marks) could be anything, not necesarily a format parsable\n\nTypo: necesarily => necessarily\n\n\nCheers,\nMax\n"},{"id":"202921","messageId":"6EF0EFE1-70F2-4CE5-9602-B13362294653@quendi.de","threadId":"32071","inReplyTo":"1352642392-28387-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 05/15] Add new simplified git-remote-testgit","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-11T20:40:23Z","receivedAt":"2012-11-11T20:40:23Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 11.11.2012, at 14:59, Felipe Contreras wrote:\n\n> It's way simpler. It exerceises the same features of remote helpers.\n\nTypo: exerceises => exercises\n\n\nCheers,\nMax\n"},{"id":"202922","messageId":"D39D26A9-16B4-4A9F-9102-BD2C92FA10AF@quendi.de","threadId":"32071","inReplyTo":"1352642392-28387-15-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 14/15] fast-export: make sure updated refs get updated","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-11T20:43:26Z","receivedAt":"2012-11-11T20:43:26Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 11.11.2012, at 14:59, Felipe Contreras wrote:\n\n> When an object has already been exported (and thus is in the marks) it's\n> flagged as SHOWN, so it will not be exported again, even if in a later\n> time it's exported through a different ref.\n> \n> We don't need the object to be exported again, but we want the ref\n> updated, which doesn't happen.\n> \n> Since we can't know if a ref was exported or not, let's just assume that\n> if the commit was marked (flags & SHOWN), the user still wants the ref\n> updated.\n> \n> IOW: If it's specified in the command line, it will get updated,\n> regardless of wihether or not the object was marked.\n\nTypo: wihether => whether\n\n> \n> So:\n> \n> % git branch test master\n> % git fast-export $mark_flags master\n> % git fast-export $mark_flags test\n> \n> Would export 'test' properly.\n> \n> Additionally, this fixes issues with remote helpers; now they can push\n> refs wich objects have already been exported, and a few other issues as\n\nTypo: wich => which\n\n\nCheers,\nMax\n"},{"id":"202919","messageId":"29291552-880A-4FEB-88E0-A73A1C7742F7@quendi.de","threadId":"32071","inReplyTo":"1352642392-28387-12-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-11T20:49:06Z","receivedAt":"2012-11-11T20:49:06Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 11.11.2012, at 14:59, Felipe Contreras wrote:\n\n> People seeking for reference would find it useful.\n\nHm, I don't understand this commit message. Probably means I am just too dumb, but since I am one of those people who would likely be seeking for reference, I would really appreciate if it could clarified. Like, for example, I don't see how the patch below makes anything \"clear\", it just seems to change the \"import\" command of git-remote-testgit to make use of the 'done' feature?\n\nPerhaps the idea of the patch is to make use of the \"done\" feature so that remote-testgit acts as \"reference implementation\"? If that is the intention, then perhaps this could be used as commit message:\n\n remote-testgit: make use of the 'done' feature\n\n This might be helpful for people who would like to see how to properly\n implement the \"done\" feature.\n\nBut again, I am not sure if I understood the purpose of this patch correctly. So please forgive me if this was totally off-base :-(.\n\nCheers,\nMax\n\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> git-remote-testgit | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-remote-testgit b/git-remote-testgit\n> index 698effc..812321e 100755\n> --- a/git-remote-testgit\n> +++ b/git-remote-testgit\n> @@ -55,8 +55,10 @@ while read line; do\n> \n> \t\techo \"feature import-marks=$gitmarks\"\n> \t\techo \"feature export-marks=$gitmarks\"\n> -\t\tgit fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n> +\t\techo \"feature done\"\n> +\t\tgit fast-export --{import,export}-marks=\"$testgitmarks\" $refs | \\\n> \t\t\tsed -e \"s#refs/heads/#${prefix}/heads/#g\"\n> +\t\techo \"done\"\n> \t\t;;\n> \texport)\n> \t\tbefore=$(git for-each-ref --format='%(refname) %(objectname)')\n> -- \n> 1.8.0\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"202924","messageId":"CAMP44s0o1eP+aeT0AHu4uP1NPLqJq56qUDb-+F_x5NjoJCnf+A@mail.gmail.com","threadId":"32071","inReplyTo":"29291552-880A-4FEB-88E0-A73A1C7742F7@quendi.de","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T21:22:09Z","receivedAt":"2012-11-11T21:22:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 9:49 PM, Max Horn wrote:\n>\n> On 11.11.2012, at 14:59, Felipe Contreras wrote:\n>\n>> People seeking for reference would find it useful.\n>\n> Hm, I don't understand this commit message. Probably means I am j git fast-export --use-done-featureust too dumb, but since I am one of those people who would likely be seeking for reference, I would really appreciate if it could clarified. Like, for example, I don't see how the patch below makes anything \"clear\", it just seems to change the \"import\" command of git-remote-testgit to make use of the 'done' feature?\n\nNo, the done feature was there already, but not so visible: git\nfast-export --use-done-feature <-there. Which is the problem, it's too\neasy to miss, therefore the need to make it clear.\n\n> Perhaps the idea of the patch is to make use of the \"done\" feature so that remote-testgit acts as \"reference implementation\"? If that is the intention, then perhaps this could be used as commit message:\n\nIt's already there.\n\n> remote-testgit: make use of the 'done' feature\n>\n> This might be helpful for people who would like to see how to properly\n> implement the \"done\" feature.\n\nEverybody should implement the 'done' feature. Otherwise random error\nmessages quite easily appear.\n\n-- \nFelipe Contreras\n"},{"id":"202949","messageId":"EA56F0CC-7C93-491F-A076-4A1AA9593ED0@quendi.de","threadId":"32071","inReplyTo":"CAMP44s0o1eP+aeT0AHu4uP1NPLqJq56qUDb-+F_x5NjoJCnf+A@mail.gmail.com","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-12T11:20:56Z","receivedAt":"2012-11-12T11:20:56Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 11.11.2012, at 22:22, Felipe Contreras wrote:\n\n> On Sun, Nov 11, 2012 at 9:49 PM, Max Horn wrote:\n>> \n>> On 11.11.2012, at 14:59, Felipe Contreras wrote:\n>> \n>>> People seeking for reference would find it useful.\n>> \n>> Hm, I don't understand this commit message. Probably means I am j git fast-export --use-done-featureust too dumb, but since I am one of those people who would likely be seeking for reference, I would really appreciate if it could clarified. Like, for example, I don't see how the patch below makes anything \"clear\", it just seems to change the \"import\" command of git-remote-testgit to make use of the 'done' feature?\n> \n> No, the done feature was there already, but not so visible: git\n> fast-export --use-done-feature <-there. Which is the problem, it's too\n> easy to miss, therefore the need to make it clear.\n\n\nAha, now I understand what this patch is about. So I would suggest this alternate commit message:\n\n remote-testgit: make it explicit clear that we use the 'done' feature\n\n Previously we relied on passing '--use-done-feature ' to git fast-export, which is\n easy to miss when looking at this script. Since remote-testgit is also a reference\n implementation, we now explicitly output 'feature done' / 'done' to make it\n crystal clear that we implement this feature.\n\n\nOr perhaps a little bit less verbose. With a commit message like the above, I think I would have grokked the patch right away. With the original message, that was not the case (else I wouldn't have wrote my initial email). And even though I now understand (or at least believe to understand) the patch, I don't think the original message is that helpful... indeed, \"make clear the 'done' feature\" is ambiguous. You meant it as \"make clear the 'done' feature is implemented / used\", while I understood it as \"make clear what the 'done' feature is about\". Looking at the patch can help to resolve that, but (a) my wrong interpretation threw me off-track and (b) I thought that the point of commit messages was to give an overview of a patch without having to look at it...\nSo at the very least, the message should explain what exactly is \"made clear\".\n\nAnyway, a small change to the commit message hopefully will not be a problem. :-)\n\n\nCheers,\nMax"},{"id":"202956","messageId":"20121112154515.GB3546@elie.Belkin","threadId":"32071","inReplyTo":"EA56F0CC-7C93-491F-A076-4A1AA9593ED0@quendi.de","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-11-12T15:45:15Z","receivedAt":"2012-11-12T15:45:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Max Horn wrote:\n\n> Aha, now I understand what this patch is about. So I would suggest\n> this alternate commit message:\n>\n> remote-testgit: make it explicit clear that we use the 'done' feature\n>\n> Previously we relied on passing '--use-done-feature ' to git\n> fast-export, which is easy to miss when looking at this script.\n\nI'm not immediately sure I agree this is even a problem. Is the point\nthat other fast-import frontends do not have a --use-done-feature\nswitch, so a typical remote helper has to do that work itself, and the\nsample \"testgit\" remote helper would be a more helpful example by\ndoing that work itself?\n\nThe idea behind --use-done-feature is that if fast-export exits early\nfor some reason and its output is going to a pipe then at least the\nstream will be malformed, making it easier to catch errors. So there\nis something to be weighed here: is it more important to illustrate\nhow to make your fast-export tool's output prefix-free, or is it more\nimportant to illustrate how to work around a fast-export tool that\ndoesn't support that feature? The answer is not immediately obvious\nto me. A good description could provide context to make it obvious.\n\nHoping that clarifies,\nJonathan\n"},{"id":"202957","messageId":"CAMP44s0WH-P7WY4UqhMX3WdrrSCYXUR9yCgsUV+mzLOCK5LkHQ@mail.gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-16-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-12T16:28:28Z","receivedAt":"2012-11-12T16:28:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 2:59 PM, Felipe Contreras\n wrote:\n> They have been marked as UNINTERESTING for a reason, lets respect that.\n>\n> Currently the first ref is handled properly, but not the rest, so:\n>\n> % git fast-export master ^master\n>\n> Would currently throw a reset for master (2nd ref), which is not what we\n> want.\n>\n> % git fast-export master ^foo ^bar ^roo\n> % git fast-export master salsa..tacos\n>\n> Even if all these refs point to the same object; foo, bar, roo, salsa,\n> and tacos would all get a reset, and to a non-existing object (invalid\n> mark :0).\n>\n> And even more, it would only happen if the ref is pointing to exactly\n> the same commit, but not otherwise:\n>\n> % git fast-export ^next next\n> reset refs/heads/next\n> from :0\n>\n> % git fast-export ^next next^{commit}\n> # nothing\n> % git fast-export ^next next~0\n> # nothing\n> % git fast-export ^next next~1\n> # nothing\n> % git fast-export ^next next~2\n> # nothing\n>\n> The reason this happens is that before traversing the commits,\n> fast-export checks if any of the refs point to the same object, and any\n> duplicated ref gets added to a list in order to issue 'reset' commands\n> after the traversing. Unfortunately, it's not even checking if the\n> commit is flagged as UNINTERESTING. The fix of course, is to do\n> precisely that.\n>\n> The current behavior is most certainly not what we want. After this\n> patch, nothing gets exported, because nothing was selected (everything\n> is UNINTERESTING).\n>\n> Signed-off-by: Felipe Contreras \n\nAnd here's yet another reason why this is obviously correct that I just found:\n\n% git fast-export --use-done-feature\n--{import,export}-marks=.git/hg/origin/marks-git\n^refs/hg/origin/branches/default ^refs/hg/origin/bookmarks/test6\nrefs/heads/test6 ^refs/hg/origin/bookmarks/master\n^refs/hg/origin/bookmarks/test\nfeature done\nreset refs/hg/origin/bookmarks/test\nfrom :4\n\nreset refs/heads/test6\nfrom :14\n\ndone\n\nWhat is that refs/hg/origin/bookmarks/test doing there?\n\ntransport-helper does use a fast-export command like that to specify\nprecisely what refs should be *IGNORED*, and yet fast-export will\nthrow a reset for a ref that has been marked as UNINTERESTING. So, the\nreceiving end in the helper will see a reset for a ref that it\nexplicitly said was marked as outside it's refspec realm:\n\nrefspec refs/heads/*:refs/hg/origin/bookmarks/*\n\nWhat is remote-hg supposed to do with 'refs/hg/origin/bookmarks/test'?\nThere's nothing that can be done, it's a bug in fast-export that such\na thing was exported in the first place. And the reason it happens is\nthat another ref happens to be pointing to the same object, (in this\ncase refs/hg/origin/branches/default)\n\nSo yeah, the patch is good.\n\nOf course, transport-helper shouldn't even be specifying the negative\n(^) refs, but that's another story.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202960","messageId":"CAMP44s1zWzaExNC9fOvfSQhxe6UmoDqOs39n_C3EN1JonBT0Dw@mail.gmail.com","threadId":"32071","inReplyTo":"20121112154515.GB3546@elie.Belkin","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-12T16:40:57Z","receivedAt":"2012-11-12T16:40:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 12, 2012 at 4:45 PM, Jonathan Nieder wrote:\n> Max Horn wrote:\n>\n>> Aha, now I understand what this patch is about. So I would suggest\n>> this alternate commit message:\n>>\n>> remote-testgit: make it explicit clear that we use the 'done' feature\n>>\n>> Previously we relied on passing '--use-done-feature ' to git\n>> fast-export, which is easy to miss when looking at this script.\n>\n> I'm not immediately sure I agree this is even a problem. Is the point\n> that other fast-import frontends do not have a --use-done-feature\n> switch,\n\nYou mean other fast-exports.\n\nAnd what other fast-exports? Most remote helpers don't use an external\nfast-export tool, and the only I know that used one is the one I wrote\n(and is now deprecated) that used bzr fast-export, and no, that one\ndidn't support the done feature.\n\nMost remote helpers would probably be doing the equivalent of\nfast-export themselves.\n\n> so a typical remote helper has to do that work itself, and the\n> sample \"testgit\" remote helper would be a more helpful example by\n> doing that work itself?\n\nYes.\n\n> The idea behind --use-done-feature is that if fast-export exits early\n> for some reason and its output is going to a pipe then at least the\n> stream will be malformed, making it easier to catch errors. So there\n> is something to be weighed here: is it more important to illustrate\n> how to make your fast-export tool's output prefix-free,\n\nWhat fast-export tool?\n\nThis is a remote helper.\n\n> or is it more\n> important to illustrate how to work around a fast-export tool that\n> doesn't support that feature?\n\nDitto.\n\nIf you want to launch a campaign of adding the 'done' feature to\nwhatever fast-export tools are out there (that I'm not aware of), go\nahead, but this is about remote helpers, most (all?) of which would\nnot use a fast-export tool to achieve the export, but do it\nthemselves.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202973","messageId":"7vbof2k96j.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"20121111163827.GA11408@sigill.intra.peff.net","subject":"Re: [PATCH v5 01/15] fast-export: avoid importing blob marks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T17:44:36Z","receivedAt":"2012-11-12T17:44:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Major issue: \"echo -n\" is still not portable.\n>> \n>> Could we simply use\n>> \n>> touch marks-cur &&\n>> touch marks-new\n>\n> Yes, \"echo -n\" is definitely not portable. Our preferred way of\n> creating an empty file is just \">file\".\n\nYes.\n\nAnd it is misleading to use \"touch\" in this case; unless you are\nupdating the timestamp of an existing file while preserving the\ncontents of it, please don't use the command.\n\nThanks.\n"},{"id":"203579","messageId":"7vd2z7rj3y.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"CAMP44s0WH-P7WY4UqhMX3WdrrSCYXUR9yCgsUV+mzLOCK5LkHQ@mail.gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-20T22:43:13Z","receivedAt":"2012-11-20T22:43:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Of course, transport-helper shouldn't even be specifying the negative\n> (^) refs, but that's another story.\n\nHrm, I am not sure I understand what you mean by this.\n\nHow should it be telling the fast-export up to what commit the\nreceiving end should already have the history for (hence they do not\nneed to be sent)? Or are you advocating to re-send the entire\nhistory down to the root commit every time?\n"},{"id":"203590","messageId":"CAMP44s0UhTm7rRAQOHbwnv682xWCmD2JKQJBRB7+pXmzBUPqOw@mail.gmail.com","threadId":"32071","inReplyTo":"7vd2z7rj3y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T03:03:03Z","receivedAt":"2012-11-21T03:03:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 20, 2012 at 11:43 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Of course, transport-helper shouldn't even be specifying the negative\n>> (^) refs, but that's another story.\n>\n> Hrm, I am not sure I understand what you mean by this.\n>\n> How should it be telling the fast-export up to what commit the\n> receiving end should already have the history for (hence they do not\n> need to be sent)? Or are you advocating to re-send the entire\n> history down to the root commit every time?\n\nNo, it would not re-send the whole history, that's what marks are for.\n\nAnd right now it doesn't exactly which was the last commit. Let's\nsuppose the remote helper has a refspec like this:\n\nrefs/heads/*:refs/hg/origin/heads/*\n\n1) What happens the first time you push?\n\n5203a268546295ebd895fd87522217ef53bd3313 refs/heads/master\n5203a268546295ebd895fd87522217ef53bd3313 refs/remotes/tmp/master\n\nNotice how the remote ref is updated correctly, but it's not the\nremote helper refspec, so the next time you push, you will from root.\n\nIt's only when you fetch that you get the refspec'ed refs:\n\n5203a268546295ebd895fd87522217ef53bd3313 refs/heads/master\n5203a268546295ebd895fd87522217ef53bd3313 refs/hg/tmp/heads/master\n5203a268546295ebd895fd87522217ef53bd3313 refs/remotes/tmp/master\n\nSo, there's already a mismatch.\n\n2) What happens when you have no marks?\n\nYou get something like:\nreset refs/heads/heads\nfrom :0\n\nWhich is totally useless. Somebody proposed a patch that would replace\nthe :0 with a git sha-1, but that is equally useless for a remote\nhelper: we need a hg ref id, or a bzr id, or whatever, and no, there's\nmapping between git sha-1's and hg ref ids, there's only git->mark\nmark->hg, without marks, there's no way to map the git id to the hg\nid.\n\n3) What happens when you have a refspec like this?\n\n*:*\n\nNow nothing works, because we would be requesting ^refs/heads/master\nrefs/heads/master.\n\nAnd according to the documentation, this is the default when no\nrefspec is used, which is not true.\n\n4) What happens when there's no refspec at all.\n\nNow it's even worst; nothing gets done at all:\n\nif (!data->refspecs)\n\tcontinue;\n\nI documented all this breakages in this patch:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/209365\n\nnot ok 10 - push new branch with old:new refspec # TODO known breakage\nok 11 - cloning without refspec\nok 12 - pulling without refspecs\nnot ok 13 - pushing without refspecs # TODO known breakage\nnot ok 14 - pulling with straight refspec # TODO known breakage\nnot ok 15 - pushing with straight refspec # TODO known breakage\nnot ok 16 - pulling without marks # TODO known breakage\nnot ok 17 - pushing without marks # TODO known breakage\n\nAnd if you apply this patch:\n\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -750,6 +750,7 @@ static int push_refs_with_export(struct transport\n*transport,\n struct helper_data *data = transport->data;\n struct string_list revlist_args = STRING_LIST_INIT_NODUP;\n struct strbuf buf = STRBUF_INIT;\n+ struct remote *remote = transport->remote;\n\n helper = get_helper(transport);\n\n@@ -761,22 +762,23 @@ static int push_refs_with_export(struct\ntransport *transport,\n char *private;\n unsigned char sha1[20];\n\n- if (!data->refspecs)\n+ if (ref->deletion)\n+ die(\"remote-helpers do not support ref deletion\");\n+\n+ if (!ref->peer_ref)\n+ continue;\n+\n+ string_list_append(&revlist_args, ref->peer_ref->name);\n+\n+ if (!data->import_marks)\n continue;\n- private = apply_refspecs(data->refspecs,\ndata->refspec_nr, ref->name);\n+\n+ private = apply_refspecs(remote->fetch,\nremote->fetch_refspec_nr, ref->name);\n if (private && !get_sha1(private, sha1)) {\n strbuf_addf(&buf, \"^%s\", private);\n string_list_append(&revlist_args,\nstrbuf_detach(&buf, NULL));\n }\n free(private);\n-\n- if (ref->deletion) {\n- die(\"remote-helpers do not support ref deletion\");\n- }\n-\n- if (ref->peer_ref)\n- string_list_append(&revlist_args, ref->peer_ref->name);\n-\n }\n\n if (get_exporter(transport, &exporter, &revlist_args))\n\nok 13 - pushing without refspecs # TODO known breakage\nok 14 - pulling with straight refspec # TODO known breakage\nok 15 - pushing with straight refspec # TODO known breakage\nok 16 - pulling without marks # TODO known breakage\nok 17 - pushing without marks # TODO known breakage\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203592","messageId":"20121121041735.GE4634@elie.Belkin","threadId":"32071","inReplyTo":"7vd2z7rj3y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-11-21T04:17:35Z","receivedAt":"2012-11-21T04:17:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n\n>> Of course, transport-helper shouldn't even be specifying the negative\n>> (^) refs, but that's another story.\n>\n> Hrm, I am not sure I understand what you mean by this.\n>\n> How should it be telling the fast-export up to what commit the\n> receiving end should already have the history for (hence they do not\n> need to be sent)? Or are you advocating to re-send the entire\n> history down to the root commit every time?\n\nI think Felipe has mentioned before that he considers it the remote\nhelper's responsibility to keep track of what commits have already\nbeen imported, for example using a marks file.\n\nNever mind that others have said that that's not the current interface\n(I don't yet see why it would be a good interface after a transition,\nbut maybe it would be). Still, hopefully that clarifies the intended\nmeaning.\n\nHope that helps,\nJonathan\n"},{"id":"203593","messageId":"CAMP44s2mpxSX4oMMofffgeYD43CROsM_ruj1NHijPANgPU4d-A@mail.gmail.com","threadId":"32071","inReplyTo":"20121121041735.GE4634@elie.Belkin","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T04:22:33Z","receivedAt":"2012-11-21T04:22:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 5:17 AM, Jonathan Nieder wrote:\n> Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>\n>>> Of course, transport-helper shouldn't even be specifying the negative\n>>> (^) refs, but that's another story.\n>>\n>> Hrm, I am not sure I understand what you mean by this.\n>>\n>> How should it be telling the fast-export up to what commit the\n>> receiving end should already have the history for (hence they do not\n>> need to be sent)? Or are you advocating to re-send the entire\n>> history down to the root commit every time?\n>\n> I think Felipe has mentioned before that he considers it the remote\n> helper's responsibility to keep track of what commits have already\n> been imported, for example using a marks file.\n\nIt's not the remote helper, fast-export does that.\n\n> Never mind that others have said that that's not the current interface\n> (I don't yet see why it would be a good interface after a transition,\n> but maybe it would be). Still, hopefully that clarifies the intended\n> meaning.\n\nThe current interface is broken.\n\nnot ok 16 - pulling without marks # TODO known breakage\nnot ok 17 - pushing without marks # TODO known breakage\n\nSee? A remote helper without marks doesn't work.\n\n-- \nFelipe Contreras\n"},{"id":"203595","messageId":"7vfw43pmp7.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"20121121041735.GE4634@elie.Belkin","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T05:08:36Z","receivedAt":"2012-11-21T05:08:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Never mind that others have said that that's not the current interface\n> (I don't yet see why it would be a good interface after a transition,\n> but maybe it would be). Still, hopefully that clarifies the intended\n> meaning.\n\nCare to explain how the current interface is supposed to work, how\nfast-export and transport-helper should interact with remote helpers\nthat adhere to the current interface, and how well/correctly the\ncurrent implementation of these pieces work?\n\nWhat I am trying to get at is to see where the problem lies. Felipe\nsees bugs in the aggregated whole. Is the root cause of the problems\nhe sees some breakages in the current interface? Is the interface\ndesigned right but the problem is that the implementation of the\ntransport-helper is buggy and driving fast-export incorrectly? Or is\nthe implementation of the fast-export buggy and emitting wrong results,\neven though the transport-helper is driving fast-export correctly?\nSomething else?\n\nI see Felipe keeps repeating that there are bugs, and keeps posting\npatches to change fast-export, but I haven't seen a concrete \"No,\nthe reason why you see these problems is because you are not using\nthe interface correctly; the currrent interface is fine. Here is\nhow you can fix your program\" from \"others\".\n\nWith such a one-sided discussion, I've been having a hard time\nconvincing myself if Felipe's effort is making the interface better,\nor just breaking it even more for existing remote helpers, only to\nfit his world model better.\n\nHelp?\n"},{"id":"203598","messageId":"CAMP44s3vO8q+EW2sUUo6tLCQvD0PB4v_hZ9ySZjMD7wG2M8iwQ@mail.gmail.com","threadId":"32071","inReplyTo":"7vfw43pmp7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T07:11:52Z","receivedAt":"2012-11-21T07:11:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 6:08 AM, Junio C Hamano wrote:\n\n> I see Felipe keeps repeating that there are bugs, and keeps posting\n> patches to change fast-export, but I haven't seen a concrete \"No,\n> the reason why you see these problems is because you are not using\n> the interface correctly; the currrent interface is fine. Here is\n> how you can fix your program\" from \"others\".\n>\n> With such a one-sided discussion, I've been having a hard time\n> convincing myself if Felipe's effort is making the interface better,\n> or just breaking it even more for existing remote helpers, only to\n> fit his world model better.\n\nIIRC you mentioned something about this mailing list being focused on\n*technical* merit. I've explained as much as I could, but at the end\nof the talk, talk is cheap, the code speaks for itself. I added a new\nvery very very simple testgit remote helper, so anybody can see what's\ngoing on, and figure out how the interface could be used wrong.\n\nAnybody can modify the bash version of git-remote-testgit and say 'no,\nthe interface is not broken, here is how you push and pull without\nmarks'. How hard is it to hack 82 lines of bash code?\n\nBut lets assume my testgit is fatally broken, would you, Junio, accept\nthese patches if I show the same broken behavior with the python\ngit-remote-testgit?\n\nI'm afraid I have to point out the hard truth; the reason why nobody\nis doing that is because a) the interface is truly broken b) if they\ntry, they most likely would fail, and that would prove they were wrong\nin previous discussion, or c) not enough familiarity with the code. I\ndon't want to point fingers, nor do I intend to offend anybody, but I\ncannot find any other explanation of why this patch series, which is\nobviously correct (to me), doesn't receive any feedback, even though\nin theory, it should be very very very easy to show what's wrong with\nthe series.\n\nThe tests are there, and the remote helper is as simple as it gets.\nThere's nothing else but fast-export and transport-helper to blame for\nthe issues. It's that simple.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203599","messageId":"CAMP44s3_nMqJ_ieOxwtDXb4ou6pb7AFsX4tr45p97StT35=SKg@mail.gmail.com","threadId":"32071","inReplyTo":"7vfw43pmp7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T08:37:26Z","receivedAt":"2012-11-21T08:37:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 6:08 AM, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> Never mind that others have said that that's not the current interface\n>> (I don't yet see why it would be a good interface after a transition,\n>> but maybe it would be). Still, hopefully that clarifies the intended\n>> meaning.\n>\n> Care to explain how the current interface is supposed to work, how\n> fast-export and transport-helper should interact with remote helpers\n> that adhere to the current interface, and how well/correctly the\n> current implementation of these pieces work?\n>\n> What I am trying to get at is to see where the problem lies. Felipe\n> sees bugs in the aggregated whole. Is the root cause of the problems\n> he sees some breakages in the current interface? Is the interface\n> designed right but the problem is that the implementation of the\n> transport-helper is buggy and driving fast-export incorrectly? Or is\n> the implementation of the fast-export buggy and emitting wrong results,\n> even though the transport-helper is driving fast-export correctly?\n> Something else?\n\nLet me give it a shot at explaining the case for remote helpers that\nuse export/import.\n\n== listing ==\n\nAll operations begin with the transport helper requesting a list of\nrefs. Basically 'git show-ref'.\n\n== fetching ==\n\nIn fetch mode the transport helper will initiate the process by\nrequesting refs to the remote helper, like 'master', or 'devel', and\nso on. These refs were previously provided by the remote helper itself\nin the \"listing\" step.\n\nIt is the total responsibility of the remote helper to decide what to\ndo: nothing, only update the ref pointers, retrieve the whole\nrepository, retrieve only the listed refs, etc. It's also the\nresponsibility of the remote helper to keep track of marks, last known\ncommits the refs pointed to, update local transitory repositories,\netc.\n\nIt's also the responsibility of the remote helper to throw the right\n'feature' commands to fast-import for everything, including where to\nstore the marks.\n\nNote that there are two sets of marks; the marks of the remote helper,\nwhich could be anything: JSON, text files, binary, etc. and don't\ncontain git SHA-1's, and the git marks, which do contain git SHA-1's\nand are exported/imported by fast-import, but *both* are totally under\ncontrol of the remote helper.\n\nAt this point, git (transport helper), has absolutely no idea what's\ngoing on, the communication is completely between the remote helper\nand fast-import. After this process has finished, control goes back to\nthe transport helper, which proceeds to check what fast-import did.\n\nThen, the result is shown to the user as the typical fetch that\nupdated certain refs.\n\n== pushing ==\n\nIn this mode the roles are reversed, now git (transport helper) is in\ncontrol, and everything that happens depends on what commands are\npassed to fast-export. Now the remote helper is a passive receiver of\ndata, and has two options, receive it or die.\n\nWhich refs get updated and how, is the total responsibility of transport helper.\n\nThe only control the remote helper has, is before the export begins,\nin the configuration (capabilities command) that happens at the very\nbeginning (before listing), and where it specifies features to\nsupport, which are then used to pass the relevant arguments to\nfast-export.\n\nAnd these capabilities are very limited:\n* import-marks\n* export-marks\n* refspec\n\nAfter the push has finished, the remote helper then proceeds to report\nwhich refs were actually updated, and the user gets notified.\n\n== details ==\n\nAs it should be obvious by now, there's not many ways in which a\nremote helper can screw things up (other than the parsing and\ngeneration of data for fast-import/export). The only tricky part is\nthe refspec.\n\nTo function properly, a remote helper should specify a refspec such as\n'refs/heads/*:refs/test/heads/*', this way, all the changes a remote\nhelper does are isolated in a specific refspec namespace, and the\nupdate to normal git happens in a controlled way.\n\nHowever, the refspec only makes sense in the *fetching* mode; the\nremote-helper is supposed to throw updates in the form of 'commit\nrefs/test/heads/master', not 'commit refs/heads/master' (although in\nsome case that might work, but I'm not sure which).\n\nBut when pushing the remote helper will receive the refs in the normal\nform 'refs/heads/master'. Also, the namespaced refs are only updated\nwhen fetching, not when pushing.\n\nMarks are very straightforward; the same import and export marks\nshould be specified for both importing and exporting.\n\nEverything works mostly fine as long as the remote helper follows\nthis. Things break in all sorts of ways when it doesn't.\n\nBut I want to emphasize again that there's not many ways in which a\nremote helper can screw things: marks, or refspec, that's it.\n*Specially* when pushing.\n\n== no marks ==\n\nLet's imagine a very simple repository with 3 commits, which gets\npushed to a remote one:\n\n4e891f6 :3\nd9d17c3 :2\ne1aef7b :1\n\nI'm obviously simplifying the marks, but essentially that's what\nfast-export would do when pushing commits to a remote helper; it the\nparent of :2 is :1, and the parent of :3 is :2, but the remote side\n*never* sees any git SHA-1, because they are not interesting in any\nway, there's nothing useful that can be done with them.\n\nThe remote side would generate commits such as:\n\n:3 103\n:2 102\n:1 100\n\nAgain, for simplification purposes (you can picture them as mercurial revs).\n\nNow the push has finished. The marks are gone (no marks).\n\nWhat happens when you fetch? You might think that we will get only the\ncommits after :3, but that's not the case, the transport helper would\nuse 'refs/test/heads/master' to find out the last commit, but that\ndoesn't get updated when pushing, only when fetching, so we would\nstart from the top.\n\n4e891f6 :3\nd9d17c3 :2\ne1aef7b :1\n\n:3 103\n:2 102\n:1 100\n\nThe same will happen if you push, because push also uses\n'refs/test/heads/master'.\n\nBut *now* that we are doing a fetch, the 'refs/test/heads/master'\npointer is updated to 4e891f6. But don't think that those marks are\nthe same as the previous ones: they happen to be the same because they\nwere generated the same way, but they are completely independent.\n\nWhat happens when you push now? Now the 'refs/test/heads/master' is\npointing to 4e891f6, and suppose we have two new commits:\n\n88764ee\n4607106\n4e891f6 :3 <- I'm putting these for reference, but in reality they are gone\nd9d17c3 :2\ne1aef7b :1\n\nThe transport helper would do an export of '^refs/test/heads/master\nrefs/heads/master', or '^4e891f6 88764ee'. And here comes the\ninteresting part:\n\nWhat is the parent of 4607106? It's not :3, because that mark is gone,\nand in fact, even if we sent :3; things would break down because the\nother side has no idea what :3 means; it's gone, caput. What really\nhappens is:\n\n88764ee :2\n4607106 :1\n\nThis is a new tree. That's exactly what you would expect if you do\n'git fast-export ^v1.8.0^ master'; export all the commits as if v1.8.0\nwas the root.\n\nBut in the context of remote helpers, that's not what we want.\n\nWhat can we do to fix this? Let's suppose that through some magic we\nget the parent of 4607106 to be 4e891f6; is that helpful? No. To the\nremote helper 4e891f6 is useless. What we need is 103, but without\nmarks, we can't find that out.\n\nMaybe if we stored it in the last run? We need to parse the git marks,\nand then match our marks with those, and we could get a mapping like\n'4e891f6 -> 103', but what if the parent is 102? So, we need a mapping\nfor all the marks, and then we have to store such mapping anyway. And\nguess what? We are back to using marks again! Except that instead of\nusing the standard git way, we are using a custom hacky way.\n\nAre there other solutions? Maybe we can store the information in refs:\n\n4e891f6 refs/test/ids/103\nd9d17c3 refs/test/ids/102\ne1aef7b refs/test/ids/100\n\nBut that also would require parsing the git marks, and going outside\nof the intended fast-export tool would kind of defeat the purpose of\nbeing fast an efficient, and still very hacky.\n\nAnd lets not even go as to what would be needed for 'git fast-export'\nto actually generate 4e891f6 in the first place, as that would\nprobably require changes that would break other things.\n\nSo no, you can't do it without marks.\n\nAnd why are we even discussing about this? Why would anybody want to\navoid marks? Not only there's no other ways to achieve the same, marks\nare cheap and efficient, as efficient as any other solution could be,\nand then some. And do we have any real remote helpers that try to do\nexport/import without marks? No, heck, we don't even have fake ones.\n\nIt just doesn't work. Seriously.\n\nAnd my patches actually make it work: if there are no marks, then\n_everything_ is pushed. I don't see the point of supporting the\nfunctionality of no marks, clearly nobody is using that because it\njust doesn't work. Nobody has shown a shred of evidence to the\ncontrary. With my patches, at least we try to do something without\nfailing too miserably.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203601","messageId":"CAMP44s3h5+KS3ixoLkJeiS+n_neBV-Dyj=Cww0ZrU6UKsNxphQ@mail.gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 00/15] fast-export and remote-testgit improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T09:46:54Z","receivedAt":"2012-11-21T09:46:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 2:59 PM, Felipe Contreras\n wrote:\n\nSince these are having some problems getting in, let me point out\nwhich I think are important, and which not.\n\n> Felipe Contreras (15):\n> fast-export: avoid importing blob marks\n\nThis fixes a bug, but it's probably not hitting many people.\n\n> remote-testgit: fix direction of marks\n> remote-helpers: fix failure message\n\nI don't care.\n\n> Rename git-remote-testgit to git-remote-testpy\n> Add new simplified git-remote-testgit\n\nThese I think are good.\n\n> remote-testgit: get rid of non-local functionality\n> remote-testgit: remove irrelevant test\n> remote-testgit: cleanup tests\n\nJust cleanups.\n\n> remote-testgit: exercise more features\n\nI think it's good to catch more issues, but I don't care much.\n\n> remote-testgit: report success after an import\n> remote-testgit: make clear the 'done' feature\n\nThese are good, but I could drop them.\n\n> fast-export: trivial cleanup\n> fast-export: fix comparison in tests\n\nObvious and correct, but I don't care.\n\n> fast-export: make sure updated refs get updated\n\nThis is the important one. It fixes real issues quite visible on remote helpers.\n\n> fast-export: don't handle uninteresting refs\n\nThis is nice, but can be dropped.\n\n\nI don't see what are the chances of any of them getting merged, but at\nleast 'fast-export: make sure updated refs get updated' should\ndefinitely go in. Please advice at which level I should drop the\npatches, because at this point it doesn't look like any of them are\ngoing in.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203607","messageId":"7vy5hu3h11.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"EA56F0CC-7C93-491F-A076-4A1AA9593ED0@quendi.de","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:11:21Z","receivedAt":"2012-11-21T18:11:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Horn writes:\n\n> Aha, now I understand what this patch is about. So I would suggest this alternate commit message:\n>\n> remote-testgit: make it explicit clear that we use the 'done' feature\n>\n> Previously we relied on passing '--use-done-feature ' to git fast-export, which is\n> easy to miss when looking at this script. Since remote-testgit is also a reference\n> implementation, we now explicitly output 'feature done' / 'done' to make it\n> crystal clear that we implement this feature.\n\nI'd state it like this, but I may have guessed what Felipe intended\nincorrectly.\n\n remote-testgit: advertise \"done\" feature and write \"done\" ourselves\n \n Instead of letting \"fast-export\" advertise the feature and ending\n its stream with \"done\", do it ourselves. This way, it would make it\n more clear to people who want to write their own remote-helper to\n produce fast-export streams without using \"fast-export\n --use-done-feature\" that they are supposed to end their stream with\n \"done\".\n"},{"id":"203606","messageId":"7vsj823h0z.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"D39D26A9-16B4-4A9F-9102-BD2C92FA10AF@quendi.de","subject":"Re: [PATCH v5 14/15] fast-export: make sure updated refs get updated","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:12:04Z","receivedAt":"2012-11-21T18:12:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Horn writes:\n\n> On 11.11.2012, at 14:59, Felipe Contreras wrote:\n>\n>> When an object has already been exported (and thus is in the marks) it's\n>> flagged as SHOWN, so it will not be exported again, even if in a later\n>> time it's exported through a different ref.\n>> \n>> We don't need the object to be exported again, but we want the ref\n>> updated, which doesn't happen.\n>> \n>> Since we can't know if a ref was exported or not, let's just assume that\n>> if the commit was marked (flags & SHOWN), the user still wants the ref\n>> updated.\n>> \n>> IOW: If it's specified in the command line, it will get updated,\n>> regardless of wihether or not the object was marked.\n>\n> Typo: wihether => whether\n>\n>> \n>> So:\n>> \n>> % git branch test master\n>> % git fast-export $mark_flags master\n>> % git fast-export $mark_flags test\n>> \n>> Would export 'test' properly.\n>> \n>> Additionally, this fixes issues with remote helpers; now they can push\n>> refs wich objects have already been exported, and a few other issues as\n>\n> Typo: wich => which\n\nI'd rather use \"whose\" there.\n"},{"id":"203605","messageId":"7vmwya3h0x.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"1352642392-28387-16-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:14:24Z","receivedAt":"2012-11-21T18:14:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> They have been marked as UNINTERESTING for a reason, lets respect that.\n> ...\n> The current behavior is most certainly not what we want. After this\n> patch, nothing gets exported, because nothing was selected (everything\n> is UNINTERESTING).\n\nThe old behaviour was an incorrect \"workaround\" that has been\nsuperseded by your 14/15 \"make sure updated refs get updated\", no?\nMentioning that would help people realize that this patch would not\ncause regression on them, I would think.\n"},{"id":"203612","messageId":"7vhaoi3h0v.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"1352642392-28387-10-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 09/15] remote-testgit: exercise more features","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:26:29Z","receivedAt":"2012-11-21T18:26:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Unfortunately they do not work.\n\nAs far as I can tell, \"more features\" simply mean one, no? Perhaps\n\n remote-testgit: exercise non-default refspec feature\n\nor something.\n\n> Signed-off-by: Felipe Contreras \n> ---\n> git-remote-testgit | 18 +++++++++++++----\n> t/t5801-remote-helpers.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 63 insertions(+), 4 deletions(-)\n> mode change 100755 => 100644 t/t5801-remote-helpers.sh\n\nOops.\n\nAgain, please check the fixup! interspersed in the result I'll queue\non 'pu'.\n\nThanks.\n"},{"id":"203608","messageId":"7vboeq3h0t.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"1352642392-28387-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 05/15] Add new simplified git-remote-testgit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:26:37Z","receivedAt":"2012-11-21T18:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> It's way simpler. It exerceises the same features of remote helpers.\n> It's easy to read and understand. It doesn't depend on python.\n>\n> It does _not_ exercise the python remote helper framework; there's\n> another tool and another test for that.\n\nYou mention why you _think_ it is better, and what it is _not_, but\nwith your excitement, end up failing to mention what it is. I'll\ntry to reword the commit with this sentence:\n\n\tThis script is to test the remote-helper interface.\n\nsomewhere. Please check what I'll push out on 'pu' after I'm done\nfor the day (probably in 8 hours).\n\n> git-remote-testgit | 62 ++++++++++++++++\n> t/t5801-remote-helpers.sh | 139 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 202 insertions(+), 1 deletion(-)\n> create mode 100755 git-remote-testgit\n\nI hinted at this in an earlier message, but creating this file as a\ntracked file at this point in the history is a bit irritating for\nbisectability. After you build and test an earlier commit, this\npath is a generated and ignored, and then checking out this commit\nor a later one will fail without \"make clean\". It is only a minor\nirritation, but still noticeable.\n\nThanks.\n"},{"id":"203610","messageId":"7v624y3h0q.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"1352642392-28387-7-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 06/15] remote-testgit: get rid of non-local functionality","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:26:42Z","receivedAt":"2012-11-21T18:26:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> This only makes sense for the python remote helpers framework.\n\nA better explanation is sorely needed for this. If the test were\nfeeding python snippet to be sourced by python remote helper to be\ntested, the new remote-testgit.bash would not have any hope (nor\nneed) to grok it, and \"this only makes sense for python\" makes\nperfect sense and clear enough, but that is not the case.\n\nIf the justification were like this:\n\n remote-testgit: remove non-local tests\n \n The simplified remote-testgit does not talk with any remote\n repository and incapable of running non-local tests. Remove\n them.\n\nI would understand it, and I wouldn't say it is a regression in the\ntest not to test \"non-local\", as that is not essential aspect of\nthese tests (we are only interested in testing the object/ref\ntransfer over remote-helper interface and do not care what the\n\"other side\" really is).\n\nBut I am not quite sure what you really mean by \"non-local\"\nfunctionality in the first place. The original test weren't opening\nnetwork port to emulate multi-host remote transfer, were it?\n\nThanks.\n"},{"id":"203611","messageId":"7vzk2a22g8.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"1352642392-28387-9-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 08/15] remote-testgit: cleanup tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T18:28:23Z","receivedAt":"2012-11-21T18:28:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> We don't need a bare 'server' and an intermediary 'public'. The repos\n> can talk to each other directly; that's what we want to exercise.\n\nThe previous patch to remove the test (the one that covered a case\nwhere a bug was fixed in an older git-remote-testpy and tried to\ncatch the bug when it resurfaced) made sense even with its\nultra-short justification \"irrelevant\".\n\nBut I am not sure if this one is so cut-and-dried. The repos can\ntalk to each other directly, but at the same time the tests were\nexercising interactions between bare and non-bare repositories,\nweren't they? Talking to each other may be one of the things we\nwant to exercise, but that does not necessarily be the only thing.\n\nIf it were explained like this (note that I am *guessing* what you\nmeant to achieve by this patch, which may be wrong, in which case\nthe log message needs further clarification):\n\n\tGoing through an intermediary 'public' may have exercised\n\tinteractions among combinations of bare and non-bare\n\trepositories a bit more, but that is not an issue specific\n\tto the remote-helper transfer that we want to be testing in\n\tthis script. Simplify the tests to let two repositories\n\ttalk directly with each other.\n\nI think the changes themselves make sense.\n"},{"id":"203609","messageId":"7vtxsi22g6.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"CAMP44s3h5+KS3ixoLkJeiS+n_neBV-Dyj=Cww0ZrU6UKsNxphQ@mail.gmail.com","subject":"Re: [PATCH v5 00/15] fast-export and remote-testgit improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-21T19:05:58Z","receivedAt":"2012-11-21T19:05:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, Nov 11, 2012 at 2:59 PM, Felipe Contreras\n> wrote:\n>\n> Since these are having some problems getting in, let me point out\n> which I think are important, and which not.\n\nI finished reading the series, and found them mostly sensible.\n\nI'll send out comments on individual patches, and will push them\nout, interspersed with \"fixup!\" commits, later on 'pu' when I am\ndone for the day, perhaps in 7 hours or so.\n\nThere is one thing I am not sure about with this series, though.\n\nI can agree that the updates to fast-export will make remote-testgit\nscript work better, but I cannot tell how big an impact the changes\nwill have to people's existing use of fast-export. Some of them may\nbe relying on the current behaviour (in other words, they may be\nrelying on \"existing bugs\"), which may mean that this series will\nbring regression to them. I am still open to reasonable objections\nalong the lines of \"This script X uses fast-export and is broken\nwhen used with the updated behaviour.\" if there is any.\n"},{"id":"203616","messageId":"CAGdFq_jy2WYE1i99V1gGWY8jc19CVGngf-VFXxDw3HJC3YD-pA@mail.gmail.com","threadId":"32071","inReplyTo":"7vy5hu3h11.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2012-11-21T19:20:49Z","receivedAt":"2012-11-21T19:20:49Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Wed, Nov 21, 2012 at 10:11 AM, Junio C Hamano wrote:\n> I'd state it like this, but I may have guessed what Felipe intended\n> incorrectly.\n>\n> remote-testgit: advertise \"done\" feature and write \"done\" ourselves\n>\n> Instead of letting \"fast-export\" advertise the feature and ending\n> its stream with \"done\", do it ourselves. This way, it would make it\n> more clear to people who want to write their own remote-helper to\n> produce fast-export streams without using \"fast-export\n> --use-done-feature\" that they are supposed to end their stream with\n> \"done\".\n\nWith that commit message:\n\nAcked-by: Sverre Rabbelier \n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"203622","messageId":"20121121194810.GE16280@sigill.intra.peff.net","threadId":"32071","inReplyTo":"7vfw43pmp7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-21T19:48:10Z","receivedAt":"2012-11-21T19:48:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 20, 2012 at 09:08:36PM -0800, Junio C Hamano wrote:\n\n> With such a one-sided discussion, I've been having a hard time\n> convincing myself if Felipe's effort is making the interface better,\n> or just breaking it even more for existing remote helpers, only to\n> fit his world model better.\n\nFelipe responded in more detail, but I will just add the consensus we\ncame to earlier in the discussion: the series does make things better\nfor users of fast-export that use marks, but does not make things any\nbetter for users of negative refs on the command line. However, I do not\nthink that it makes things worse for them, either (neither by changing\nthe behavior negatively, nor by making the code harder for a more\ncomplete fix later).\n\nSo while fixing everybody might be nice, there is no need to hold up\nprogress for the marks case. Which, as he has noted, is probably the\nsanest way to implement a remote-helper[1].\n\n-Peff\n\n[1] There are other possible use cases for fast-export which might\n benefit from negative refs working more sanely, but since they are\n in the minority and are not being made worse, I think the partial\n fix is OK.\n"},{"id":"203665","messageId":"0F47AA24-F5B6-4197-8D74-6DD32E253856@quendi.de","threadId":"32071","inReplyTo":"7vfw43pmp7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2012-11-21T22:30:01Z","receivedAt":"2012-11-21T22:30:01Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"On 21.11.2012, at 06:08, Junio C Hamano wrote:\n\n> Jonathan Nieder writes:\n> \n>> Never mind that others have said that that's not the current interface\n>> (I don't yet see why it would be a good interface after a transition,\n>> but maybe it would be). Still, hopefully that clarifies the intended\n>> meaning.\n> \n> Care to explain how the current interface is supposed to work, how\n> fast-export and transport-helper should interact with remote helpers\n> that adhere to the current interface, and how well/correctly the\n> current implementation of these pieces work?\n\nYes, please!\n\n\n> \n> What I am trying to get at is to see where the problem lies. Felipe\n> sees bugs in the aggregated whole. Is the root cause of the problems\n> he sees some breakages in the current interface? Is the interface\n> designed right but the problem is that the implementation of the\n> transport-helper is buggy and driving fast-export incorrectly? Or is\n> the implementation of the fast-export buggy and emitting wrong results,\n> even though the transport-helper is driving fast-export correctly?\n> Something else?\n> \n> I see Felipe keeps repeating that there are bugs, and keeps posting\n> patches to change fast-export, but I haven't seen a concrete \"No,\n> the reason why you see these problems is because you are not using\n> the interface correctly; the currrent interface is fine. Here is\n> how you can fix your program\" from \"others\".\n\nI was wondering about the same, actually... Moreover, I started to try to understand more about this, but found this a bit difficult. Apparently I am primarily supposed to learn about remote helpers by reverse engineering the (sparsely commented, if at all) existing ones. The fact that remote helpers can implement different subsets of the feature spectrum complicates this further. \n\nOverall, my impression is that there are two kinds of remote helpers:\n\n1) Some are git-to-git helpers, which allow access to another git repos via some intermediate media / protocol (via http, ssh, ...). Those use either connect, or fetch+push. They do not need marks, because they can use the git sha1s. Examples (together with the capabilities they claim to implement):\n\n- remote-curl: fetch, option, push\n- remote-ext: connect\n- remote-fd: connect\n\n\n2) Some are interfaces to foreign systems (bzr, hg, mediawiki, ...). They cannot use sha1s and must use marks (at least that is how I understand felipe's explanation). These tools use import combined with either export, or push. Examples:\n\n- git-remote-mediawiki: import, push, refspec\n (its capabilities command also prints \"list\", but that seems to be a bug?)\n- git-remote-hg: import, export, refspec, import-marks, export-marks\n (both the msysgit one and felipe's\n- git-remote-bzr: import, push\n (the one from https://github.com/lelutin/git-remote-bzr)\n- git-remote-bzr (felipe's): import, export, refspec, *import-marks, *export-marks\n (but why the * ?)\n\n\nDoes that sound about right? If so, can somebody give me a hint when a type 2 helper would use \"export\" and when \"push\"?\n\nAnd while I am at it: git-remote-helpers.txt does not mention the \"export\", \"import-marks\" and \"export-marks\" capabilities. Could somebody who knows what they do look into fixing that? Overall, that doc helped me a bit, but it is more a reference to somebody who already understands in detail how remote helpers work, and who just wants to look up some specific detail :-(. Some hints on when to implement which capabilities might be useful (similar to the \"Tips and Tricks\" section in git-fast-import.txt).\n\nAs it is, felipe's recent explanation on why he thinks marks are essential for remote-helpers (I assume he was only referring to type 2 helpers, though) was one of the most enlightening texts I read on the whole subject so far (then again, I am fairly new to this list, so I may have missed lots of past goodness). Anyway, it would be nice if this could be augmented by \"somebody from the other camp\" ;).\n\n\nCheers,\nMax\n"},{"id":"203663","messageId":"CAMP44s0YPEw-8s4hcVGWWwVj62O1mMWQ4Rh2ZmeRMBXQwPJQxQ@mail.gmail.com","threadId":"32071","inReplyTo":"7vhaoi3h0v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 09/15] remote-testgit: exercise more features","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T23:35:12Z","receivedAt":"2012-11-21T23:35:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 7:26 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Unfortunately they do not work.\n>\n> As far as I can tell, \"more features\" simply mean one, no? Perhaps\n>\n> remote-testgit: exercise non-default refspec feature\n\nIt's the other way around, a good refspec works, anything else\ndoesn't. s/non-default/default/ but there's other stuff:\n\n1) *:* refspec\n2) no refspec\n3) no marks\n\n-- \nFelipe Contreras\n"},{"id":"203666","messageId":"CAMP44s28O+O582OZNB8D064UYyypKHMjg9hbAa6w_zqXrKFs8Q@mail.gmail.com","threadId":"32071","inReplyTo":"7vboeq3h0t.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 05/15] Add new simplified git-remote-testgit","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T23:39:44Z","receivedAt":"2012-11-21T23:39:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 7:26 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> It's way simpler. It exerceises the same features of remote helpers.\n>> It's easy to read and understand. It doesn't depend on python.\n>>\n>> It does _not_ exercise the python remote helper framework; there's\n>> another tool and another test for that.\n>\n> You mention why you _think_ it is better, and what it is _not_, but\n> with your excitement, end up failing to mention what it is. I'll\n> try to reword the commit with this sentence:\n>\n> This script is to test the remote-helper interface.\n\nThat's right.\n\n-- \nFelipe Contreras\n"},{"id":"203637","messageId":"CAMP44s1CH4Ay=Z5oq18UXwPk3iDH1V2uzA_C0wNDw6d87JV-vA@mail.gmail.com","threadId":"32071","inReplyTo":"7v624y3h0q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 06/15] remote-testgit: get rid of non-local functionality","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-21T23:44:44Z","receivedAt":"2012-11-21T23:44:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 7:26 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> This only makes sense for the python remote helpers framework.\n>\n> A better explanation is sorely needed for this. If the test were\n> feeding python snippet to be sourced by python remote helper to be\n> tested, the new remote-testgit.bash would not have any hope (nor\n> need) to grok it, and \"this only makes sense for python\" makes\n> perfect sense and clear enough, but that is not the case.\n>\n> If the justification were like this:\n>\n> remote-testgit: remove non-local tests\n>\n> The simplified remote-testgit does not talk with any remote\n> repository and incapable of running non-local tests. Remove\n> them.\n>\n> I would understand it, and I wouldn't say it is a regression in the\n> test not to test \"non-local\", as that is not essential aspect of\n> these tests (we are only interested in testing the object/ref\n> transfer over remote-helper interface and do not care what the\n> \"other side\" really is).\n>\n> But I am not quite sure what you really mean by \"non-local\"\n> functionality in the first place. The original test weren't opening\n> network port to emulate multi-host remote transfer, were it?\n\nNo, that's not it at all.\n\nBy local, I mean 'file:///home/user/foo', by remote, I mean\n'http://user.org/foo'. How each of these URLs is handled is entirely\nup to the remote helper.\n\nbzr for example doesn't need any change at all, the same API works on\nboth cases. hg OTOH has different APIs, so the code needs a local\nclone to do most operations. The python remote helper framework has\nAPIs to make it easier to implement the local clone functionality (for\nthe remote helpers that need it).\n\nThis has absolutely nothing to do with with remote helpers, this is\n100% a python remote helper feature. So we don't need those tests\nhere, they belong in git-remote-testpy.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203661","messageId":"CAMP44s0Sd9V+GCqxid_rCwNH49-+dzmreg9zwPgxoZb1hQkb1A@mail.gmail.com","threadId":"32071","inReplyTo":"7vmwya3h0x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-22T00:15:45Z","receivedAt":"2012-11-22T00:15:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 7:14 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> They have been marked as UNINTERESTING for a reason, lets respect that.\n>> ...\n>> The current behavior is most certainly not what we want. After this\n>> patch, nothing gets exported, because nothing was selected (everything\n>> is UNINTERESTING).\n>\n> The old behaviour was an incorrect \"workaround\" that has been\n> superseded by your 14/15 \"make sure updated refs get updated\", no?\n> Mentioning that would help people realize that this patch would not\n> cause regression on them, I would think.\n\nThis particular patch is not getting rid of that \"workaround\", if you\ncan call it that, it's just making it work correctly.\n\nThere's absolutely no possibility of regression (that is known or\nanybody has mentioned).\n\nThe only argument that was put forward was that 'git fast-export\n^master master' should throw:\nfrom :0\n\nAs it does now, because in the future, with another patch (that nobody\nis pursuing), it might do:\nfrom 8c7a786\n\nWhich as I have tried to explain; is equally useless.\n\nThere's no regression, nobody would be affected negatively by this\nbecause when there are no marks, nobody expects a 'from :0'; it's\ntotally useless, and when there are marks, nobody expects an update\nwhen the user does '^uninteresting master' for the 'uninteresting'\nref. And not even potential future users would be affected, because\n'from 8c7a786' is not helpful either, even if the user wanted\n'^uninsteresting' to be updated (which they won't), the git SHA-1 is\nuseless to a remote helper without marks.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203654","messageId":"CAMP44s2B2_htR8LFbHk99WaNUcaYJCxVJPdRdj5VQ0k+fB9NOg@mail.gmail.com","threadId":"32071","inReplyTo":"20121121194810.GE16280@sigill.intra.peff.net","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-22T00:28:30Z","receivedAt":"2012-11-22T00:28:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 8:48 PM, Jeff King wrote:\n> On Tue, Nov 20, 2012 at 09:08:36PM -0800, Junio C Hamano wrote:\n>\n>> With such a one-sided discussion, I've been having a hard time\n>> convincing myself if Felipe's effort is making the interface better,\n>> or just breaking it even more for existing remote helpers, only to\n>> fit his world model better.\n>\n> Felipe responded in more detail, but I will just add the consensus we\n> came to earlier in the discussion: the series does make things better\n> for users of fast-export that use marks, but does not make things any\n> better for users of negative refs on the command line. However, I do not\n> think that it makes things worse for them, either (neither by changing\n> the behavior negatively, nor by making the code harder for a more\n> complete fix later).\n\nPatch 14 changes the behavior depending on the marks, patch 15 doesn't.\n\nThis patch is mostly orthogonal to marks.\n\nBefore without marks:\n% git branch unintresting master\n% git fast-export master ^uninteresting\nreset refs/heads/uninteresting\nfrom :0\n\nBefore with marks:\n% git fast-export --import-marks=marks master ^uninteresting\nreset refs/heads/uninteresting\nfrom :100\n\nSee? In both cases git is doing something the user doesn't want, nor\nspecified. After my patch nothing gets updated, because nothing was\nspecified to be updated.\n\nI'm not going to bother explaining why other people objected to this\npatch (again), which is indeed related to marks, they should do it for\nthemselves. Let me reaffirm that no valid reason has put forward to\nobject to this patch.\n\n> So while fixing everybody might be nice\n\nI would like to understand that that even means. What behavior is\ncurrently broken? And for who? And how is this patch related to that?\n\n> [1] There are other possible use cases for fast-export which might\n> benefit from negative refs working more sanely, but since they are\n> in the minority and are not being made worse, I think the partial\n> fix is OK.\n\nWhich ones? I don't think this is a partial fix.\n\nNobody has put forward such a use-case.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203650","messageId":"CAMP44s1tkAZPza7skVza-qm5aQMur7CEbXZdE=RYYN2ZV2gwGw@mail.gmail.com","threadId":"32071","inReplyTo":"0F47AA24-F5B6-4197-8D74-6DD32E253856@quendi.de","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-22T00:38:57Z","receivedAt":"2012-11-22T00:38:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 11:30 PM, Max Horn wrote:\n\n> 2) Some are interfaces to foreign systems (bzr, hg, mediawiki, ...). They cannot use sha1s and must use marks (at least that is how I understand felipe's explanation). These tools use import combined with either export, or push. Examples:\n>\n> - git-remote-mediawiki: import, push, refspec\n> (its capabilities command also prints \"list\", but that seems to be a bug?)\n\nI don't think remote mediawiki uses marks. They are strictly not\nneeded by 'import' because the remote helper has full control over\nthis operation. For example, in my remote helpers, I store the\nprevious tip of the branches, so when git ask for 'import\nrefs/heads/master', I only import the new commits. I named this file\n'marks' anyway, but they are not marks for fast-import.\n\n> - git-remote-hg: import, export, refspec, import-marks, export-marks\n> (both the msysgit one and felipe's\n> - git-remote-bzr: import, push\n> (the one from https://github.com/lelutin/git-remote-bzr)\n> - git-remote-bzr (felipe's): import, export, refspec, *import-marks, *export-marks\n> (but why the * ?)\n\nAFAIK both of my remote helpers have * in them, they are to denote\nthey are required.\n\n> Does that sound about right? If so, can somebody give me a hint when a type 2 helper would use \"export\" and when \"push\"?\n\nI don't know when would be appropriate to use push, there's another\ngit-remote-bzr that uses the bzr-git infrastructure, and uses push.\n\nI picked export/import because I think they are the easiest to\nimplement to get all the features, and should be efficient.\n\n> And while I am at it: git-remote-helpers.txt does not mention the \"export\", \"import-marks\" and \"export-marks\" capabilities. Could somebody who knows what they do look into fixing that? Overall, that doc helped me a bit, but it is more a reference to somebody who already understands in detail how remote helpers work, and who just wants to look up some specific detail :-(. Some hints on when to implement which capabilities might be useful (similar to the \"Tips and Tricks\" section in git-fast-import.txt).\n\nI've had problems finding out the right information, but I hope the\ngit-remote-testgit I wrote in bash in less than 90 lines of code\nshould give a pretty good idea of how the whole thing works.\n\n> As it is, felipe's recent explanation on why he thinks marks are essential for remote-helpers (I assume he was only referring to type 2 helpers, though) was one of the most enlightening texts I read on the whole subject so far (then again, I am fairly new to this list, so I may have missed lots of past goodness). Anyway, it would be nice if this could be augmented by \"somebody from the other camp\" ;).\n\nWouldn't that be nice?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203659","messageId":"CAMP44s1XgmP=-cD+A013LGYWQfkfGJQDDa5w4ubsEkDSBAT2Ng@mail.gmail.com","threadId":"32071","inReplyTo":"7vtxsi22g6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 00/15] fast-export and remote-testgit improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-22T00:51:11Z","receivedAt":"2012-11-22T00:51:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 8:05 PM, Junio C Hamano wrote:\n\n> I can agree that the updates to fast-export will make remote-testgit\n> script work better, but I cannot tell how big an impact the changes\n> will have to people's existing use of fast-export. Some of them may\n> be relying on the current behaviour (in other words, they may be\n> relying on \"existing bugs\"), which may mean that this series will\n> bring regression to them. I am still open to reasonable objections\n> along the lines of \"This script X uses fast-export and is broken\n> when used with the updated behaviour.\" if there is any.\n\nWe've discussed about this extensively, and I've asked the same;\nnobody put forward any. I've also thought long and hard; can't think\nof any.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203640","messageId":"CAMP44s2T_qYeEMD=yTzTD07kL6km+W0XFOiHRAh5KtKi4CqTMw@mail.gmail.com","threadId":"32071","inReplyTo":"7vzk2a22g8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 08/15] remote-testgit: cleanup tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-22T00:55:14Z","receivedAt":"2012-11-22T00:55:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 21, 2012 at 7:28 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> We don't need a bare 'server' and an intermediary 'public'. The repos\n>> can talk to each other directly; that's what we want to exercise.\n>\n> The previous patch to remove the test (the one that covered a case\n> where a bug was fixed in an older git-remote-testpy and tried to\n> catch the bug when it resurfaced) made sense even with its\n> ultra-short justification \"irrelevant\".\n>\n> But I am not sure if this one is so cut-and-dried. The repos can\n> talk to each other directly, but at the same time the tests were\n> exercising interactions between bare and non-bare repositories,\n> weren't they? Talking to each other may be one of the things we\n> want to exercise, but that does not necessarily be the only thing.\n>\n> If it were explained like this (note that I am *guessing* what you\n> meant to achieve by this patch, which may be wrong, in which case\n> the log message needs further clarification):\n>\n> Going through an intermediary 'public' may have exercised\n> interactions among combinations of bare and non-bare\n> repositories a bit more, but that is not an issue specific\n> to the remote-helper transfer that we want to be testing in\n> this script. Simplify the tests to let two repositories\n> talk directly with each other.\n\nRight. I don't think bare vs. non-bare has anything to do with it; the\nintermediary repository was there to have 3 types of repos interacting\nwith each other local testpy, remote testpy, local git. But this\ndoesn't exercise anything from transport helper.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203727","messageId":"CAMP44s38qTcSk4PLMkjgyyNq3OJn9QhTV7MuzseB8jPM3GnmeA@mail.gmail.com","threadId":"32071","inReplyTo":"1352642392-28387-16-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-24T03:12:15Z","receivedAt":"2012-11-24T03:12:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 2:59 PM, Felipe Contreras\n wrote:\n\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -529,7 +529,9 @@ static void get_tags_and_duplicates(struct object_array *pending,\n> * sure it gets properly upddated eventually.\n> */\n> if (commit->util || commit->object.flags & SHOWN)\n> - string_list_append(extra_refs, full_name)->util = commit;\n> + if (!(commit->object.flags & UNINTERESTING))\n> + string_list_append(extra_refs, full_name)->util = commit;\n> +\n> if (!commit->util)\n> commit->util = full_name;\n> }\n\nThere is one case where this can cause problems. I'm about to send\nanother patch series where I fix transport-helper to behave more\nproperly, and in doing so it sends things such as '^old-master master\nnew', and if new points to master, there's a problem. This means the\nuser would have to do 'git push new', instead of 'git push --all'. The\ntransport-helper could do the reset itself, but that would require\nparsing the marks. A simpler solution for this is proposed in the next\npatch series.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203856","messageId":"7v7gp9udsl.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"CAMP44s2B2_htR8LFbHk99WaNUcaYJCxVJPdRdj5VQ0k+fB9NOg@mail.gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T05:35:38Z","receivedAt":"2012-11-26T05:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Wed, Nov 21, 2012 at 8:48 PM, Jeff King wrote:\n> ...\n> I would like to understand that that even means. What behavior is\n> currently broken?\n\nI do not know if this is the same as what Peff was referring to, but\nI found this message in the discussion thread during my absense.\n\nFrom: Johannes Schindelin \nSubject: Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly\nDate: Fri, 2 Nov 2012 16:17:14 +0100 (CET)\nMessage-ID: \n\n(which is $gmane/208946) that says:\n\n\tNote that\n\n\t\t$ git branch foo master~1\n\t\t$ git fast-export foo master~1..master\n\n\tstill does not update the \"foo\" ref, but a partial fix is better\n\tthan no fix.\n"},{"id":"203870","messageId":"CAMP44s1ZUBopJb_RNreV9TQNzG8_yscvGRtuvTFEJWfP=DhsZQ@mail.gmail.com","threadId":"32071","inReplyTo":"7v7gp9udsl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-26T12:16:21Z","receivedAt":"2012-11-26T12:16:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 26, 2012 at 6:35 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Wed, Nov 21, 2012 at 8:48 PM, Jeff King wrote:\n>> ...\n>> I would like to understand that that even means. What behavior is\n>> currently broken?\n>\n> I do not know if this is the same as what Peff was referring to, but\n> I found this message in the discussion thread during my absense.\n>\n> From: Johannes Schindelin \n> Subject: Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly\n> Date: Fri, 2 Nov 2012 16:17:14 +0100 (CET)\n> Message-ID: \n>\n> (which is $gmane/208946) that says:\n>\n> Note that\n>\n> $ git branch foo master~1\n> $ git fast-export foo master~1..master\n>\n> still does not update the \"foo\" ref, but a partial fix is better\n> than no fix.\n\nFirst of all, do we agree that this patch does not change the\nsituation for this command? If so, I don't see why that would be\nrelevant while discussing this patch series.\n\nSecond, this is what I get:\n\n% git log --decorate --oneline foo master~1..master\n8c7a786 (tag: v1.8.0, master) Git 1.8.0\n\nNotice that 'foo' is not there? It's not there because we explicitly\nstated that we didn't want it there.\n\nAnd what do you expect that command to do with 'foo'? To throw a\n'reset refs/heads/foo'? To what commit? There is no mark for that\ncommit. 'reset :0'? That doesn't help anybody. No, that command is not\nbroken, it works as expected.\n\nNotice the situation would be different with 'git fast-export\n--import-marks=marks foo master~1..master', because if there's a mark\nfor foo, *now* we can do something about it. This particular patch\nseries doesn't, but the next one does.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203892","messageId":"alpine.DEB.1.00.1211261726260.7256@s15462909.onlinehome-server.info","threadId":"32071","inReplyTo":"7v7gp9udsl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-26T16:28:20Z","receivedAt":"2012-11-26T16:28:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 25 Nov 2012, Junio C Hamano wrote:\n\n> From: Johannes Schindelin \n> Subject: Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly\n> Date: Fri, 2 Nov 2012 16:17:14 +0100 (CET)\n> Message-ID: \n> \n> (which is $gmane/208946) that says:\n> \n> \tNote that\n> \n> \t\t$ git branch foo master~1\n> \t\t$ git fast-export foo master~1..master\n> \n> \tstill does not update the \"foo\" ref, but a partial fix is better\n> \tthan no fix.\n\nIf you changed your stance on the patch Sverre and I sent to fix this, we\ncould get a non-partial fix for this. You wanted a fix for a bigger\nproblem, though, which I am unwilling to fix because it is not my itch to\nscratch and I have to balance my time.\n\nCiao,\nJohannes\n"},{"id":"203896","messageId":"7vd2z0tfhz.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"alpine.DEB.1.00.1211261726260.7256@s15462909.onlinehome-server.info","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T17:56:24Z","receivedAt":"2012-11-26T17:56:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> If you changed your stance on the patch Sverre and I sent to fix this, we\n> could get a non-partial fix for this.\n\nThis is long time ago so I may be misremembering the details, but I\nthought the original patch was (ab)using object flags to mark \"this\nwas explicitly asked for, even though some other range operation may\nhave marked it uninteresting\". Because it predated the introduction\nof the rev_cmdline_info mechanism to record what was mentioned on\nthe command line separately from what objects are uninteresting\n(i.e. object flags), it may have been one convenient way to record\nthis information, but it still looked unnecessarily ugly hack to me,\nin that it allocated scarce object flag bits to represent a narrow\nspecial case (iirc, only a freestanding \"A\" on the command line but\nnot \"A\" spelled in \"B..A\", or something), making it more expensive\nto record other kinds of command line information in a way\nconsistent with the approach chosen (we do not want to waste object\nflag bits in order to record \"this was right hand side tip of the\nsymmetric difference range\" and such).\n\nIf you are calling \"do not waste object flags to represent one\nspecial case among endless number of possibilities, as it will make\nit impossible to extend it\" my stance, that hasn't changed.\n\nWe added rev_cmdline_info since then so that we can tell what refs\nwere given from the command line in what way, and I thought that we\napplied a patch from Sverre that uses it instead of the object\nflags. Am I misremembering things?\n"},{"id":"203906","messageId":"CAMP44s3tnK+uc0YEEq=Q=2nOoFrkc1ooNPcHMShtcXzpxXhRfQ@mail.gmail.com","threadId":"32071","inReplyTo":"7vd2z0tfhz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-26T19:23:45Z","receivedAt":"2012-11-26T19:23:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 26, 2012 at 6:56 PM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n>\n>> If you changed your stance on the patch Sverre and I sent to fix this, we\n>> could get a non-partial fix for this.\n>\n> This is long time ago so I may be misremembering the details, but I\n> thought the original patch was (ab)using object flags to mark \"this\n> was explicitly asked for, even though some other range operation may\n> have marked it uninteresting\". Because it predated the introduction\n> of the rev_cmdline_info mechanism to record what was mentioned on\n> the command line separately from what objects are uninteresting\n> (i.e. object flags), it may have been one convenient way to record\n> this information, but it still looked unnecessarily ugly hack to me,\n> in that it allocated scarce object flag bits to represent a narrow\n> special case (iirc, only a freestanding \"A\" on the command line but\n> not \"A\" spelled in \"B..A\", or something), making it more expensive\n> to record other kinds of command line information in a way\n> consistent with the approach chosen (we do not want to waste object\n> flag bits in order to record \"this was right hand side tip of the\n> symmetric difference range\" and such).\n>\n> If you are calling \"do not waste object flags to represent one\n> special case among endless number of possibilities, as it will make\n> it impossible to extend it\" my stance, that hasn't changed.\n\nThe problem with those patches is that they were doing many things at\nthe same time.\n\nYou are correct that one of the problems being solved was the fact\nthat we wanted to differentiate B from A in B..A independently of the\nobject, because it might have been referenced by ^C. My latest patch\nseries deals with that by using rev cmdline_info.\n\nBut there's another problem that series tried to fix: weather or not A\nwas exported by fast-export, which is not strictly the same as SHOWN.\n\nThis becomes a non-issue if my patch series is applied because it\nproperly identifies when an object has been marked or not. But it's\nnot when marks are not used.\n\nFor example:\n\n% git branch A v1\n% git branch B v0\n% git branch C v0\n% git branch D v1\n% git fast-export B..A ^C D\n\nA would be updated through a 'commit refs/heads/A' command, D would be\nupdated through 'reset refs/heads/D'.\n\nBut what if C points to v1? The code will assume A will be exported,\nand it will be skipped, and there will be only one reset: 'reset\nrefs/heads/D'. Either way it doesn't matter, because the reset would\nbe to mark :0, so even if there was a 'reset refs/heads/A' (because A\nwas never exported), a mark :0 would be useless.\n\nWhen marks are used my patch fixes the problem because it doesn't care\nif A was exporeted or not; by knowing it was marked, it knows it was\nnever intended to be exported, so we get resets for both A and D, with\nreal marks.\n\n> We added rev_cmdline_info since then so that we can tell what refs\n> were given from the command line in what way, and I thought that we\n> applied a patch from Sverre that uses it instead of the object\n> flags. Am I misremembering things?\n\nNo, the patch from Sverre was never merged.\n\n-- \nFelipe Contreras\n"},{"id":"203932","messageId":"alpine.DEB.1.00.1211262024520.7256@s15462909.onlinehome-server.info","threadId":"32071","inReplyTo":"7vd2z0tfhz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-26T19:26:16Z","receivedAt":"2012-11-26T19:26:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 26 Nov 2012, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > If you changed your stance on the patch Sverre and I sent to fix this,\n> > we could get a non-partial fix for this.\n> \n> This is long time ago so I may be misremembering the details, but I\n> thought the original patch was (ab)using object flags to mark \"this was\n> explicitly asked for, even though some other range operation may have\n> marked it uninteresting\". Because it predated the introduction of the\n> rev_cmdline_info mechanism to record what was mentioned on the command\n> line separately from what objects are uninteresting (i.e. object flags),\n> it may have been one convenient way to record this information, but it\n> still looked unnecessarily ugly hack to me, in that it allocated scarce\n> object flag bits to represent a narrow special case (iirc, only a\n> freestanding \"A\" on the command line but not \"A\" spelled in \"B..A\", or\n> something), making it more expensive to record other kinds of command\n> line information in a way consistent with the approach chosen (we do not\n> want to waste object flag bits in order to record \"this was right hand\n> side tip of the symmetric difference range\" and such).\n\nGood to know. I will find some time to look at rev_cmdline_info and patch\nmy patch.\n\n> If you are calling \"do not waste object flags to represent one\n> special case among endless number of possibilities, as it will make\n> it impossible to extend it\" my stance, that hasn't changed.\n> \n> We added rev_cmdline_info since then so that we can tell what refs\n> were given from the command line in what way, and I thought that we\n> applied a patch from Sverre that uses it instead of the object\n> flags. Am I misremembering things?\n\nIt does sound so familiar that I am intended to claim that you remember\nthings correctly.\n\nCiao,\nJohannes\n"},{"id":"203938","messageId":"CAGdFq_iLYHs_tUDRsT9X1J12vSp3TUoMJQVbjw4ZgxONL6tMCA@mail.gmail.com","threadId":"32071","inReplyTo":"alpine.DEB.1.00.1211262024520.7256@s15462909.onlinehome-server.info","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2012-11-26T21:46:55Z","receivedAt":"2012-11-26T21:46:55Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Mon, Nov 26, 2012 at 11:26 AM, Johannes Schindelin\n wrote:\n>> We added rev_cmdline_info since then so that we can tell what refs\n>> were given from the command line in what way, and I thought that we\n>> applied a patch from Sverre that uses it instead of the object\n>> flags. Am I misremembering things?\n>\n> It does sound so familiar that I am intended to claim that you remember\n> things correctly.\n\nFWIW, I implemented that in\nhttp://thread.gmane.org/gmane.comp.version-control.git/184874 but\ndidn't do the work to get it merged.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"203942","messageId":"7vsj7wovhn.fsf@alter.siamese.dyndns.org","threadId":"32071","inReplyTo":"CAGdFq_iLYHs_tUDRsT9X1J12vSp3TUoMJQVbjw4ZgxONL6tMCA@mail.gmail.com","subject":"Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T22:22:12Z","receivedAt":"2012-11-26T22:22:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Mon, Nov 26, 2012 at 11:26 AM, Johannes Schindelin\n> wrote:\n>>> We added rev_cmdline_info since then so that we can tell what refs\n>>> were given from the command line in what way, and I thought that we\n>>> applied a patch from Sverre that uses it instead of the object\n>>> flags. Am I misremembering things?\n>>\n>> It does sound so familiar that I am intended to claim that you remember\n>> things correctly.\n>\n> FWIW, I implemented that in\n> http://thread.gmane.org/gmane.comp.version-control.git/184874 but\n> didn't do the work to get it merged.\n\nAh, OK. Should I expect an updated series then? How would it\ninteract with the recent work by Felipe?\n"}]}