{"thread":{"id":"31998","subject":"[PATCH v4 00/14] fast-export and remote-testgit improvements","startedAt":"2012-11-02T02:02:04Z","lastAt":"2012-11-02T17:19:50Z","messageCount":26,"participants":["Felipe Contreras","Stefano Lattarini","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":14},"messages":[{"id":"202386","messageId":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":null,"subject":"[PATCH v4 00/14] fast-export and remote-testgit improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:04Z","receivedAt":"2012-11-02T02:02:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\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\nFelipe Contreras (14):\n fast-export: avoid importing blob marks\n remote-testgit: fix direction of marks\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 | 153 ++++++++++++++++++++\n t/t9350-fast-export.sh | 41 +++++-\n 12 files changed, 712 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":"202387","messageId":"1351821738-17526-2-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 01/14] fast-export: avoid importing blob marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:05Z","receivedAt":"2012-11-02T02:02:05Z","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 were 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":"202388","messageId":"1351821738-17526-3-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 02/14] remote-testgit: fix direction of marks","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:06Z","receivedAt":"2012-11-02T02:02:06Z","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":"202392","messageId":"1351821738-17526-4-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 03/14] Rename git-remote-testgit to git-remote-testpy","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:07Z","receivedAt":"2012-11-02T02:02:07Z","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 e7dc668..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 git-remote-hg tests, python not available'\n-\ttest_done\n-fi\n-\n-\"$PYTHON_PATH\" -c '\n-import sys\n-if sys.hexversion < 0x02040000:\n- sys.exit(1)\n-' || {\n-\tskip_all='skipping git-remote-hg tests, python version < 2.4'\n-\ttest_done\n-}\n-\n-compare_refs() {\n-\tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n-\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..927eef1\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 git-remote-hg tests, python not available'\n+\ttest_done\n+fi\n+\n+\"$PYTHON_PATH\" -c '\n+import sys\n+if sys.hexversion < 0x02040000:\n+ sys.exit(1)\n+' || {\n+\tskip_all='skipping git-remote-hg tests, python version < 2.4'\n+\ttest_done\n+}\n+\n+compare_refs() {\n+\tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n+\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":"202391","messageId":"1351821738-17526-5-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:08Z","receivedAt":"2012-11-02T02:02:08Z","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 | 134 +++++++++++++++++++++++++++++++++++\n 3 files changed, 197 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..6650402\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\" || echo -n > \"$gitmarks\"\n+test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n+\n+while read line; do\n+ case \"$line\" in\n+ capabilities)\n+ echo 'import'\n+ echo 'export'\n+ echo \"refspec $refspec\"\n+ echo \"*import-marks $gitmarks\"\n+ echo \"*export-marks $gitmarks\"\n+ echo\n+ ;;\n+ list)\n+ git for-each-ref --format='? %(refname)' 'refs/heads/'\n+ head=$(git symbolic-ref HEAD)\n+ echo \"@$head HEAD\"\n+ echo\n+ ;;\n+ import*)\n+ # read all import lines\n+ while true; do\n+ ref=\"${line#* }\"\n+ refs=\"$refs $ref\"\n+ read line\n+ test \"${line%% *}\" != \"import\" && break\n+ done\n+\n+ echo \"feature import-marks=$gitmarks\"\n+ echo \"feature export-marks=$gitmarks\"\n+ git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n+ sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n+ ;;\n+ export)\n+ git fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n+ echo\n+ ;;\n+ '')\n+ exit\n+ ;;\n+ esac\n+done\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nnew file mode 100755\nindex 0000000..67bc8eb\n--- /dev/null\n+++ b/t/t5801-remote-helpers.sh\n@@ -0,0 +1,134 @@\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+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":"202389","messageId":"1351821738-17526-6-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 05/14] remote-testgit: get rid of non-local functionality","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:09Z","receivedAt":"2012-11-02T02:02:09Z","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 | 46 +++++++++++++++++-----------------------------\n 2 files changed, 17 insertions(+), 32 deletions(-)\n\ndiff --git a/git-remote-testgit b/git-remote-testgit\nindex 6650402..b9810fd 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 67bc8eb..2eca42e 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -28,11 +28,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@@ -45,11 +40,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@@ -71,19 +61,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@@ -97,6 +74,16 @@ test_expect_success 'fetch new branch' '\n \tcompare_refs public HEAD localclone FETCH_HEAD\n '\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@@ -106,29 +93,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":"202390","messageId":"1351821738-17526-7-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 06/14] remote-testgit: remove irrelevant test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:10Z","receivedAt":"2012-11-02T02:02:10Z","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 2eca42e..a03d087 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -48,19 +48,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":"202394","messageId":"1351821738-17526-8-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 07/14] remote-testgit: cleanup tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:11Z","receivedAt":"2012-11-02T02:02:11Z","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 | 67 +++++++++++++++++++++++------------------------\n 1 file changed, 33 insertions(+), 34 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex a03d087..3c4e09a 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -14,96 +14,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-test_expect_success 'bump commit in public' '\n-\t(cd public &&\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-\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":"202393","messageId":"1351821738-17526-9-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 08/14] remote-testgit: exercise more features","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:12Z","receivedAt":"2012-11-02T02:02:12Z","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 b9810fd..6c348b0 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\" || echo -n > \"$gitmarks\"\n-test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n+if [ -z \"$GIT_REMOTE_TESTGIT_NO_MARKS\" ]; then\n+ test -e \"$gitmarks\" || echo -n > \"$gitmarks\"\n+ test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n+else\n+ echo -n > \"$gitmarks\"\n+ echo -n > \"$testgitmarks\"\n+fi\n \n while read line; do\n case \"$line\" in\n capabilities)\n echo 'import'\n echo 'export'\n- echo \"refspec $refspec\"\n+ test -n \"$refspec\" && echo \"refspec $refspec\"\n echo \"*import-marks $gitmarks\"\n echo \"*export-marks $gitmarks\"\n echo\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nold mode 100755\nnew mode 100644\nindex 3c4e09a..83561f8\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -105,4 +105,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":"202396","messageId":"1351821738-17526-10-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:13Z","receivedAt":"2012-11-02T02:02:13Z","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 6c348b0..4e8b356 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -59,7 +59,18 @@ while read line; do\n sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n ;;\n export)\n+ declare -A before after\n+\n+ eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')\n+\n git fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n+\n+ eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')\n+\n+ for ref in \"${!after[@]}\"; do\n+ test \"${before[$ref]}\" == \"${after[$ref]}\" && continue\n+ echo \"ok $ref\"\n+ done\n echo\n ;;\n '')\n-- \n1.8.0\n"},{"id":"202395","messageId":"1351821738-17526-11-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 10/14] remote-testgit: make clear the 'done' feature","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:14Z","receivedAt":"2012-11-02T02:02:14Z","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 4e8b356..1116587 100755\n--- a/git-remote-testgit\n+++ b/git-remote-testgit\n@@ -55,8 +55,10 @@ while read line; do\n \n echo \"feature import-marks=$gitmarks\"\n echo \"feature export-marks=$gitmarks\"\n- git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n+ echo \"feature done\"\n+ git fast-export --{import,export}-marks=\"$testgitmarks\" $refs | \\\n sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n+ echo \"done\"\n ;;\n export)\n declare -A before after\n-- \n1.8.0\n"},{"id":"202397","messageId":"1351821738-17526-12-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 11/14] fast-export: trivial cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:15Z","receivedAt":"2012-11-02T02:02:15Z","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":"202400","messageId":"1351821738-17526-13-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 12/14] fast-export: fix comparison in tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:16Z","receivedAt":"2012-11-02T02:02:16Z","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":"202399","messageId":"1351821738-17526-14-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 13/14] fast-export: make sure updated refs get updated","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:17Z","receivedAt":"2012-11-02T02:02:17Z","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 83561f8..6e4e078 100644\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -58,18 +58,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@@ -105,13 +93,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@@ -154,4 +142,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":"202398","messageId":"1351821738-17526-15-git-send-email-felipe.contreras@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 14/14] fast-export: don't handle uninteresting refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T02:02:18Z","receivedAt":"2012-11-02T02:02:18Z","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 patch,\nnothing gets exported, because nothing was selected (everything is\nUNINTERESTING).\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":"202422","messageId":"5093D0DD.3050801@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-11-02T13:55:41Z","receivedAt":"2012-11-02T13:55:41Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 11/02/2012 03:02 AM, Felipe Contreras wrote:\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> For now let's just copy the old remote-helpers test script, although\n> some of those tests don't make sense for this testgit (they still pass).\n> \n> In addition, this script would be able to test other features not\n> currently being tested.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/git-remote-testgit.txt | 2 +-\n> git-remote-testgit | 62 ++++++++++++++++\n> t/t5801-remote-helpers.sh | 134 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 197 insertions(+), 1 deletion(-)\n> create mode 100755 git-remote-testgit\n> create mode 100755 t/t5801-remote-helpers.sh\n> \n> diff --git a/git-remote-testgit b/git-remote-testgit\n> new file mode 100755\n> index 0000000..6650402\n> --- /dev/null\n> +++ b/git-remote-testgit\n> @@ -0,0 +1,62 @@\n> +#!/bin/bash\n>\nI think git can't assume the existence of bash unconditionally, neither\nin its scripts, nor in its tests (the exception being the tests on\nbash completion, of course). This script probably need to be re-written\nto be a valid POSIX shell script.\n\nIt almost is, anyway, apart from the nits below ...\n\n> +# Copyright (c) 2012 Felipe Contreras\n> +\n> +alias=\"$1\"\n>\nJust FYI: the double quoting here (and in several variable assignments\nbelow) is redundant. You can portably write it as:\n\n alias=$1\n\nand still be safe in the face of spaces and metacharacters in $1.\nI'm not sure whether the Git coding guidelines suggest the use of\nquoting in this situation though; if this is the case, feel free\nto disregard my observation.\n\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> +\nI believe this should be rewritten as:\n\n GIT_DIR=\"$url/.git\"; export GIT_DIR\n\nin order to be portable to all the POSIX shells targeted by Git.\n\n> +mkdir -p \"$dir\"\n> +\n> +test -e \"$gitmarks\" || echo -n > \"$gitmarks\"\n> +test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n> +\nThe '-n' option to echo is not portable. To create an empty\nfile, you can just use\n\n : > file\n\nor\n\n true > file\n\n> +while read line; do\n> + case \"$line\" in\n>\nUseless double quoting (my previous observation about Git coding\nguidelines applies here as well, of course).\n\n> + capabilities)\n> + echo 'import'\n> + echo 'export'\n> + echo \"refspec $refspec\"\n> + echo \"*import-marks $gitmarks\"\n> + echo \"*export-marks $gitmarks\"\n> + echo\n> + ;;\n> + list)\n> + git for-each-ref --format='? %(refname)' 'refs/heads/'\n> + head=$(git symbolic-ref HEAD)\n> + echo \"@$head HEAD\"\n> + echo\n> + ;;\n> + import*)\n> + # read all import lines\n> + while true; do\n> + ref=\"${line#* }\"\n> + refs=\"$refs $ref\"\n> + read line\n> + test \"${line%% *}\" != \"import\" && break\n> + done\n> +\n> + echo \"feature import-marks=$gitmarks\"\n> + echo \"feature export-marks=$gitmarks\"\n> + git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n>\nBetter avoid the tricky {foo,bar} bashism:\n\n git fast-export --use-done-feature \\\n --import-marks=\"$testgitmarks\" \\\n --export-marks=\"$testgitmarks\" \\\n\t\t $refs | \\\n\n> + sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n> + ;;\n> + export)\n> + git fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n>\nHere too.\n\n> + echo\n> + ;;\n> + '')\n> + exit\n>\nI'd put an explicit exit status here, for clarity (this is a matter\nof personal preference, so feel free to disregard this nit).\n\n> + ;;\n> + esac\n> +done\n\nRegards,\n Stefano\n"},{"id":"202423","messageId":"5093D193.3030108@gmail.com","threadId":"31998","inReplyTo":"1351821738-17526-10-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-11-02T13:58:43Z","receivedAt":"2012-11-02T13:58:43Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 11/02/2012 03:02 AM, Felipe Contreras wrote:\n> Doesn't make a difference for the tests, but it does for the ones\n> seeking reference.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> git-remote-testgit | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n> \n> diff --git a/git-remote-testgit b/git-remote-testgit\n> index 6c348b0..4e8b356 100755\n> --- a/git-remote-testgit\n> +++ b/git-remote-testgit\n> @@ -59,7 +59,18 @@ while read line; do\n> sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n> ;;\n> export)\n> + declare -A before after\n> +\nIf you convert this script to be a valid POSIX shell script (as I've\nsuggested in my reply to [PATCH v4 04/14]), you'll need to get rid of\nthis bashism (and those below) as well.\n\n> + eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')\n> +\n> git fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n> +\n> + eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')\n> +\n> + for ref in \"${!after[@]}\"; do\n> + test \"${before[$ref]}\" == \"${after[$ref]}\" && continue\n> + echo \"ok $ref\"\n> + done\n> echo\n> ;;\n> '')\n\nRegards,\n Stefano\n"},{"id":"202424","messageId":"20121102140017.GA12040@sigill.intra.peff.net","threadId":"31998","inReplyTo":"5093D0DD.3050801@gmail.com","subject":"Re: [PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-02T14:00:17Z","receivedAt":"2012-11-02T14:00:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 02, 2012 at 02:55:41PM +0100, Stefano Lattarini wrote:\n\n> > --- /dev/null\n> > +++ b/git-remote-testgit\n> > @@ -0,0 +1,62 @@\n> > +#!/bin/bash\n> >\n> I think git can't assume the existence of bash unconditionally, neither\n> in its scripts, nor in its tests (the exception being the tests on\n> bash completion, of course). This script probably need to be re-written\n> to be a valid POSIX shell script.\n\nNo, we can't assume bash. But this is replacing a script written in\npython, which we also can't assume. So if it were optional, and skipped\ngracefully when bash was not available, that would be OK. Of course if\nit could be done in portable bourne shell, that would be even better (I\nhaven't looked closely, but your comments all seem reasonable).\n\n-Peff\n"},{"id":"202443","messageId":"CAMP44s0n_O-7j329WyTxsWgVbCV2mEAnPG-WQeWBFdmHdfOjnQ@mail.gmail.com","threadId":"31998","inReplyTo":"5093D0DD.3050801@gmail.com","subject":"Re: [PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T15:42:42Z","receivedAt":"2012-11-02T15:42:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini\n wrote:\n\n>> +#!/bin/bash\n>>\n> I think git can't assume the existence of bash unconditionally, neither\n> in its scripts, nor in its tests (the exception being the tests on\n> bash completion, of course). This script probably need to be re-written\n> to be a valid POSIX shell script.\n\nWell, this is a _reference_ script, and that is used only for testing\npurposes. The test itself can be like the bash completion tests, and\nsimply be skipped.\n\nThe reason I chose bash is because associative arrays, which you see\nin a later patch.\n\n> It almost is, anyway, apart from the nits below ...\n>\n>> +# Copyright (c) 2012 Felipe Contreras\n>> +\n>> +alias=\"$1\"\n>>\n> Just FYI: the double quoting here (and in several variable assignments\n> below) is redundant. You can portably write it as:\n>\n> alias=$1\n>\n> and still be safe in the face of spaces and metacharacters in $1.\n> I'm not sure whether the Git coding guidelines suggest the use of\n> quoting in this situation though; if this is the case, feel free\n> to disregard my observation.\n\nWhat happens when you call this with:\n\n ./script \"alias with spaces\"\n\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> I believe this should be rewritten as:\n>\n> GIT_DIR=\"$url/.git\"; export GIT_DIR\n>\n> in order to be portable to all the POSIX shells targeted by Git.\n\n_If_ we want this as POSIX, yeah.\n\n>> +mkdir -p \"$dir\"\n>> +\n>> +test -e \"$gitmarks\" || echo -n > \"$gitmarks\"\n>> +test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n>> +\n> The '-n' option to echo is not portable. To create an empty\n> file, you can just use\n>\n> : > file\n>\n> or\n>\n> true > file\n\nAll right, thanks.\n\n>> +while read line; do\n>> + case \"$line\" in\n>>\n> Useless double quoting (my previous observation about Git coding\n> guidelines applies here as well, of course).\n\nWhat if line has multiple spaces? To me it makes sense to quote it.\n\n>> + echo \"feature import-marks=$gitmarks\"\n>> + echo \"feature export-marks=$gitmarks\"\n>> + git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n>>\n> Better avoid the tricky {foo,bar} bashism:\n>\n> git fast-export --use-done-feature \\\n> --import-marks=\"$testgitmarks\" \\\n> --export-marks=\"$testgitmarks\" \\\n> $refs | \\\n\nIf that's what we want, yeah.\n\n-- \nFelipe Contreras\n"},{"id":"202444","messageId":"CAMP44s2ZPbda7yJ9UtOhWMqaKp4TaAgoyeUWUSrt0vco7RK+Tw@mail.gmail.com","threadId":"31998","inReplyTo":"5093D193.3030108@gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T15:46:39Z","receivedAt":"2012-11-02T15:46:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 2, 2012 at 2:58 PM, Stefano Lattarini\n wrote:\n> On 11/02/2012 03:02 AM, Felipe Contreras wrote:\n>> Doesn't make a difference for the tests, but it does for the ones\n>> seeking reference.\n>>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> git-remote-testgit | 11 +++++++++++\n>> 1 file changed, 11 insertions(+)\n>>\n>> diff --git a/git-remote-testgit b/git-remote-testgit\n>> index 6c348b0..4e8b356 100755\n>> --- a/git-remote-testgit\n>> +++ b/git-remote-testgit\n>> @@ -59,7 +59,18 @@ while read line; do\n>> sed -e \"s#refs/heads/#${prefix}/heads/#g\"\n>> ;;\n>> export)\n>> + declare -A before after\n>> +\n> If you convert this script to be a valid POSIX shell script (as I've\n> suggested in my reply to [PATCH v4 04/14]), you'll need to get rid of\n> this bashism (and those below) as well.\n\nYeah, but I don't want to. I originally used transitory files, and\nused esoteric options of diff to do the same (which were probably not\nportable).\n\nIn the end I liked this approach much better.\n\nIf you have a solution for this that works in POSIX shell, I'll be\nglad to consider it, but for the moment, I think a simple, easy to\nunderstand and maintain code is more important, and if it needs bash,\nso be it.\n\n>> + eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')\n>> +\n>> git fast-import --{import,export}-marks=\"$testgitmarks\" --quiet\n>> +\n>> + eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')\n>> +\n>> + for ref in \"${!after[@]}\"; do\n>> + test \"${before[$ref]}\" == \"${after[$ref]}\" && continue\n>> + echo \"ok $ref\"\n>> + done\n>> echo\n>> ;;\n>> '')\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202445","messageId":"5093EEB4.3040402@gmail.com","threadId":"31998","inReplyTo":"CAMP44s0n_O-7j329WyTxsWgVbCV2mEAnPG-WQeWBFdmHdfOjnQ@mail.gmail.com","subject":"Re: [PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-11-02T16:03:00Z","receivedAt":"2012-11-02T16:03:00Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 11/02/2012 04:42 PM, Felipe Contreras wrote:\n> On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini\n> wrote:\n> \n>>> +#!/bin/bash\n>>>\n>> I think git can't assume the existence of bash unconditionally, neither\n>> in its scripts, nor in its tests (the exception being the tests on\n>> bash completion, of course). This script probably need to be re-written\n>> to be a valid POSIX shell script.\n> \n> Well, this is a _reference_ script, and that is used only for testing\n> purposes. The test itself can be like the bash completion tests, and\n> simply be skipped.\n> \n> The reason I chose bash is because associative arrays, which you see\n> in a later patch.\n> \n>> It almost is, anyway, apart from the nits below ...\n>>\n>>> +# Copyright (c) 2012 Felipe Contreras\n>>> +\n>>> +alias=\"$1\"\n>>>\n>> Just FYI: the double quoting here (and in several variable assignments\n>> below) is redundant. You can portably write it as:\n>>\n>> alias=$1\n>>\n>> and still be safe in the face of spaces and metacharacters in $1.\n>> I'm not sure whether the Git coding guidelines suggest the use of\n>> quoting in this situation though; if this is the case, feel free\n>> to disregard my observation.\n> \n> What happens when you call this with:\n> \n> ./script \"alias with spaces\"\n>\n'$alias' will correctly expand to \"alias with spaces\". Try out:\n\n $ sh -c 'alias=$1; echo \"$alias\"' dummy '1 2*3'\n 1 2*3\n\nThis works consistently with every known shell (even non-POSIX\nrelics like Solaris /bin/sh).\n\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>> I believe this should be rewritten as:\n>>\n>> GIT_DIR=\"$url/.git\"; export GIT_DIR\n>>\n>> in order to be portable to all the POSIX shells targeted by Git.\n> \n> _If_ we want this as POSIX, yeah.\n>\nWhy don't we? Why add an extra requirement for a test that\n\n 1. can be easily written in POSIX shell, and\n 2. tests a feature that doesn't require bash to work (unless\n I'm sorely mistaken, that is)?\n\nHonest question. But of course, if the Git active contributors\ndeem the extra requirement (which is not an invasive one, given\nhow often bash is installed even on non-Linux systems) acceptable\nin order to have the test case simpler and clearer, feel free to\ndisregard all my observations in this thread.\n\n>>> +mkdir -p \"$dir\"\n>>> +\n>>> +test -e \"$gitmarks\" || echo -n > \"$gitmarks\"\n>>> +test -e \"$testgitmarks\" || echo -n > \"$testgitmarks\"\n>>> +\n>> The '-n' option to echo is not portable. To create an empty\n>> file, you can just use\n>>\n>> : > file\n>>\n>> or\n>>\n>> true > file\n> \n> All right, thanks.\n> \n>>> +while read line; do\n>>> + case \"$line\" in\n>>>\n>> Useless double quoting (my previous observation about Git coding\n>> guidelines applies here as well, of course).\n> \n> What if line has multiple spaces?\n>\nStill no problem, as in the case of the 'alias=$1' assignment before:\n\n $ sh -c 'case $1 in *x\" \"x*) echo ok;; *) exit 1;; esac' dummy 'x x'\n ok\n\n> To me it makes sense to quote it.\n>\nSurely it doesn't cause any problem to \"over-quote\" in this case;\nit's better than risking to under-quote in other. I just pointed\nout that the quoting it's not really necessary, in case you weren't\naware of that.\n\n>>> + echo \"feature import-marks=$gitmarks\"\n>>> + echo \"feature export-marks=$gitmarks\"\n>>> + git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n>>>\n>> Better avoid the tricky {foo,bar} bashism:\n>>\n>> git fast-export --use-done-feature \\\n>> --import-marks=\"$testgitmarks\" \\\n>> --export-marks=\"$testgitmarks\" \\\n>> $refs | \\\n> \n> If that's what we want, yeah.\n>\nHonestly, I find my longer-and-more-explicit version clearer, even\nif you can assume bash for your script. But that's a matter of\npersonal preference (sorry for not stating that right away), so\nfeel free to ignore it if you decide to keep the bash requirement\nin the end.\n\nRegards,\n Stefano\n"},{"id":"202446","messageId":"5093EFF4.5080308@gmail.com","threadId":"31998","inReplyTo":"CAMP44s2ZPbda7yJ9UtOhWMqaKp4TaAgoyeUWUSrt0vco7RK+Tw@mail.gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-11-02T16:08:20Z","receivedAt":"2012-11-02T16:08:20Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 11/02/2012 04:46 PM, Felipe Contreras wrote:\n>\n> In the end I liked this approach much better.\n> \n> If you have a solution for this that works in POSIX shell, I'll be\n> glad to consider it, but for the moment, I think a simple, easy to\n> understand and maintain code is more important, and if it needs bash,\n> so be it.\n>\nIf this is a deliberate decision, it's ok with me. I'm just a \"casual\"\nreviewer here, not an active contributor, so I'll gladly accept\npreferences and decisions of the \"active crew\", once it's clear that\nthey are deliberate and not the result of mistakes or confusion.\n\nIn any case, I agree that having a clean, understandable code as a\nstarting point is better than having a more \"portable\" but trickier\none right away. If it will need converting to POSIX, that can be\ndone as a follow up (and as we've both noticed, this would be the\nonly point where such a conversion might be problematic -- the other\nchanges would be trivial, almost automatic).\n\nRegards,\n Stefano\n"},{"id":"202448","messageId":"CAMP44s38oBadxCew+mWzQJG1o1g++Pi4G50FuHzRO-c1sAk53Q@mail.gmail.com","threadId":"31998","inReplyTo":"5093EEB4.3040402@gmail.com","subject":"Re: [PATCH v4 04/14] Add new simplified git-remote-testgit","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T16:16:18Z","receivedAt":"2012-11-02T16:16:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 2, 2012 at 5:03 PM, Stefano Lattarini\n wrote:\n> On 11/02/2012 04:42 PM, Felipe Contreras wrote:\n\n>> What happens when you call this with:\n>>\n>> ./script \"alias with spaces\"\n>>\n> '$alias' will correctly expand to \"alias with spaces\". Try out:\n>\n> $ sh -c 'alias=$1; echo \"$alias\"' dummy '1 2*3'\n> 1 2*3\n>\n> This works consistently with every known shell (even non-POSIX\n> relics like Solaris /bin/sh).\n\nAll right.\n\n>> _If_ we want this as POSIX, yeah.\n>>\n> Why don't we? Why add an extra requirement for a test that\n>\n> 1. can be easily written in POSIX shell, and\n> 2. tests a feature that doesn't require bash to work (unless\n> I'm sorely mistaken, that is)?\n>\n> Honest question. But of course, if the Git active contributors\n> deem the extra requirement (which is not an invasive one, given\n> how often bash is installed even on non-Linux systems) acceptable\n> in order to have the test case simpler and clearer, feel free to\n> disregard all my observations in this thread.\n\nBecause the code will be more complicated. Most of the changes\nrequired are relatively small, so it's not a big issue, but I would\nlike to see how you replace the code that uses associative arrays. I\ndon't know know of a clean, simple way to do it in POSIX. If you can\nfind one, I don't see any strong reason to use bash.\n\n>>>> +while read line; do\n>>>> + case \"$line\" in\n>>>>\n>>> Useless double quoting (my previous observation about Git coding\n>>> guidelines applies here as well, of course).\n>>\n>> What if line has multiple spaces?\n>>\n> Still no problem, as in the case of the 'alias=$1' assignment before:\n>\n> $ sh -c 'case $1 in *x\" \"x*) echo ok;; *) exit 1;; esac' dummy 'x x'\n> ok\n\nAll right.\n\n>>>> + echo \"feature import-marks=$gitmarks\"\n>>>> + echo \"feature export-marks=$gitmarks\"\n>>>> + git fast-export --use-done-feature --{import,export}-marks=\"$testgitmarks\" $refs | \\\n>>>>\n>>> Better avoid the tricky {foo,bar} bashism:\n>>>\n>>> git fast-export --use-done-feature \\\n>>> --import-marks=\"$testgitmarks\" \\\n>>> --export-marks=\"$testgitmarks\" \\\n>>> $refs | \\\n>>\n>> If that's what we want, yeah.\n>>\n> Honestly, I find my longer-and-more-explicit version clearer, even\n> if you can assume bash for your script. But that's a matter of\n> personal preference (sorry for not stating that right away), so\n> feel free to ignore it if you decide to keep the bash requirement\n> in the end.\n\nAnd I prefer the other form. I fact, I would prefer if both tools\nsimply had a --marks option set both, and didn't require the file to\nbe created beforehand. I've _never_ seen a situation where separate\nmarks for import and export made sense. But that's off-topic.\n\nIf you find a way to replace the associative arrays code, I have no\ntrouble in switching this to the POSIX version.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202449","messageId":"CAMP44s2T7iOdn6mWXqcUw97d8_-=pR=7ZQ5t_ygPGOphXW5cQw@mail.gmail.com","threadId":"31998","inReplyTo":"5093EFF4.5080308@gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-02T16:19:33Z","receivedAt":"2012-11-02T16:19:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 2, 2012 at 5:08 PM, Stefano Lattarini\n wrote:\n> On 11/02/2012 04:46 PM, Felipe Contreras wrote:\n>>\n>> In the end I liked this approach much better.\n>>\n>> If you have a solution for this that works in POSIX shell, I'll be\n>> glad to consider it, but for the moment, I think a simple, easy to\n>> understand and maintain code is more important, and if it needs bash,\n>> so be it.\n>>\n> If this is a deliberate decision, it's ok with me. I'm just a \"casual\"\n> reviewer here, not an active contributor, so I'll gladly accept\n> preferences and decisions of the \"active crew\", once it's clear that\n> they are deliberate and not the result of mistakes or confusion.\n>\n> In any case, I agree that having a clean, understandable code as a\n> starting point is better than having a more \"portable\" but trickier\n> one right away. If it will need converting to POSIX, that can be\n> done as a follow up (and as we've both noticed, this would be the\n> only point where such a conversion might be problematic -- the other\n> changes would be trivial, almost automatic).\n\nAs things are the options are:\n\n1) Remove this code and move to POSIX sh. People looking for reference\nmight scratch their heads as to why 'git push' is not showing the\nupdate.\n2) Keep this code and remain in bash.\n\nUntil we have a:\n\n3) Replace this code with a clean POSIX sh alternative\n\nI would rather vote for 2)\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202450","messageId":"5093F37D.7070705@gmail.com","threadId":"31998","inReplyTo":"CAMP44s2T7iOdn6mWXqcUw97d8_-=pR=7ZQ5t_ygPGOphXW5cQw@mail.gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-11-02T16:23:25Z","receivedAt":"2012-11-02T16:23:25Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 11/02/2012 05:19 PM, Felipe Contreras wrote:\n>\n> [SNIP]\n>\n> As things are the options are:\n> \n> 1) Remove this code and move to POSIX sh. People looking for reference\n> might scratch their heads as to why 'git push' is not showing the\n> update.\n> 2) Keep this code and remain in bash.\n> \n> Until we have a:\n> \n> 3) Replace this code with a clean POSIX sh alternative\n> \n> I would rather vote for 2)\n> \nThat's perfectly fine with me, now that you've explained the rationale\nfor requiring bash in the first place (maybe adding a comment to the\nscript or the commit messages that reports this rationale for choosing\nto require bash might be worthwhile; but it's no big deal anyway).\n\nThanks,\n Stefano\n"},{"id":"202452","messageId":"20121102171949.GA28958@sigill.intra.peff.net","threadId":"31998","inReplyTo":"CAMP44s2T7iOdn6mWXqcUw97d8_-=pR=7ZQ5t_ygPGOphXW5cQw@mail.gmail.com","subject":"Re: [PATCH v4 09/14] remote-testgit: report success after an import","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-02T17:19:50Z","receivedAt":"2012-11-02T17:19:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 02, 2012 at 05:19:33PM +0100, Felipe Contreras wrote:\n\n> > In any case, I agree that having a clean, understandable code as a\n> > starting point is better than having a more \"portable\" but trickier\n> > one right away. If it will need converting to POSIX, that can be\n> > done as a follow up (and as we've both noticed, this would be the\n> > only point where such a conversion might be problematic -- the other\n> > changes would be trivial, almost automatic).\n> \n> As things are the options are:\n> \n> 1) Remove this code and move to POSIX sh. People looking for reference\n> might scratch their heads as to why 'git push' is not showing the\n> update.\n> 2) Keep this code and remain in bash.\n> \n> Until we have a:\n> \n> 3) Replace this code with a clean POSIX sh alternative\n> \n> I would rather vote for 2)\n\nI'm fine with bash. The critical thing is that it not break people's\n\"make test\" if they do not have bash (or do not have bash as /bin/bash).\n\n-Peff\n"}]}