From 2faa15274d7bf474d3c382ce36a3e4b7b21bdaa4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:21 +0200 Subject: transport-helper: fix minor leak in push_refs_with_export Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport-helper.c b/transport-helper.c index 660147f..b560b64 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -728,6 +728,7 @@ static int push_refs_with_export(struct transport *transport, strbuf_addf(&buf, "^%s", private); string_list_append(&revlist_args, strbuf_detach(&buf, NULL)); } + free(private); string_list_append(&revlist_args, ref->name); -- cgit v0.10.2-6-g49f6 From 5cf5ade371d60f97fef2eb7ef37780ed86fa4963 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:22 +0200 Subject: t5800: factor out some ref tests These are a little hard to read, and I'm about to add more just like them. Plus the failure output is nicer if we use test_cmp than a comparison with "test". Signed-off-by: Jeff King Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index 1fb6380..3a37ad0 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -17,6 +17,12 @@ then test_set_prereq PYTHON_24 fi +compare_refs() { + git --git-dir="$1/.git" rev-parse --verify $2 >expect && + git --git-dir="$3/.git" rev-parse --verify $4 >actual && + test_cmp expect actual +} + test_expect_success PYTHON_24 'setup repository' ' git init --bare server/.git && git clone server public && @@ -59,8 +65,7 @@ test_expect_success PYTHON_24 'pushing to local repo' ' echo content >>file && git commit -a -m three && git push) && - HEAD=$(git --git-dir=localclone/.git rev-parse --verify HEAD) && - test $HEAD = $(git --git-dir=server/.git rev-parse --verify HEAD) + compare_refs localclone HEAD server HEAD ' test_expect_success PYTHON_24 'synch with changes from localclone' ' @@ -73,8 +78,7 @@ test_expect_success PYTHON_24 'pushing remote local repo' ' echo content >>file && git commit -a -m four && git push) && - HEAD=$(git --git-dir=clone/.git rev-parse --verify HEAD) && - test $HEAD = $(git --git-dir=server/.git rev-parse --verify HEAD) + compare_refs clone HEAD server HEAD ' test_done -- cgit v0.10.2-6-g49f6 From 760fec7d7e488899137d662c0b0d1addd20c516e Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:23 +0200 Subject: t5800: use skip_all instead of prereq All tests require python 2.4 or higher. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index 3a37ad0..f6796e3 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -7,15 +7,19 @@ test_description='Test remote-helper import and export commands' . ./test-lib.sh -if test_have_prereq PYTHON && "$PYTHON_PATH" -c ' +if ! test_have_prereq PYTHON ; then + skip_all='skipping git-remote-hg tests, python not available' + test_done +fi + +"$PYTHON_PATH" -c ' import sys if sys.hexversion < 0x02040000: sys.exit(1) -' -then - # Requires Python 2.4 or newer - test_set_prereq PYTHON_24 -fi +' || { + skip_all='skipping git-remote-hg tests, python version < 2.4' + test_done +} compare_refs() { git --git-dir="$1/.git" rev-parse --verify $2 >expect && @@ -23,7 +27,7 @@ compare_refs() { test_cmp expect actual } -test_expect_success PYTHON_24 'setup repository' ' +test_expect_success 'setup repository' ' git init --bare server/.git && git clone server public && (cd public && @@ -33,34 +37,34 @@ test_expect_success PYTHON_24 'setup repository' ' git push origin master) ' -test_expect_success PYTHON_24 'cloning from local repo' ' +test_expect_success 'cloning from local repo' ' git clone "testgit::${PWD}/server" localclone && test_cmp public/file localclone/file ' -test_expect_success PYTHON_24 'cloning from remote repo' ' +test_expect_success 'cloning from remote repo' ' git clone "testgit::file://${PWD}/server" clone && test_cmp public/file clone/file ' -test_expect_success PYTHON_24 'create new commit on remote' ' +test_expect_success 'create new commit on remote' ' (cd public && echo content >>file && git commit -a -m two && git push) ' -test_expect_success PYTHON_24 'pulling from local repo' ' +test_expect_success 'pulling from local repo' ' (cd localclone && git pull) && test_cmp public/file localclone/file ' -test_expect_success PYTHON_24 'pulling from remote remote' ' +test_expect_success 'pulling from remote remote' ' (cd clone && git pull) && test_cmp public/file clone/file ' -test_expect_success PYTHON_24 'pushing to local repo' ' +test_expect_success 'pushing to local repo' ' (cd localclone && echo content >>file && git commit -a -m three && @@ -68,12 +72,12 @@ test_expect_success PYTHON_24 'pushing to local repo' ' compare_refs localclone HEAD server HEAD ' -test_expect_success PYTHON_24 'synch with changes from localclone' ' +test_expect_success 'synch with changes from localclone' ' (cd clone && git pull) ' -test_expect_success PYTHON_24 'pushing remote local repo' ' +test_expect_success 'pushing remote local repo' ' (cd clone && echo content >>file && git commit -a -m four && -- cgit v0.10.2-6-g49f6 From c00dd33b1fe602520f7b0e54b2c4d83b03571053 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:24 +0200 Subject: t5800: document some non-functional parts of remote helpers These are all things one might expect to work in a helper that is capable of handling multiple branches (which our testgit helper in theory should be able to do, as it is backed by git). All of these bugs are specific to the import/export codepaths, so they don't affect helpers like git-remote-curl that use fetch/push commands. The first and fourth tests are about fetching and pushing new refs, and demonstrate bugs in the git_remote_helpers library (so they would be most likely to impact helpers for other VCSs which import/export git). The second test is about importing multiple refs; it demonstrates a bug in git-remote-testgit, which is mostly for exercising the test code. Therefore it probably doesn't affect anyone in practice. The third test demonstrates a bug in git's side of the helper code when the upstream has added refs that we do not have locally. This could impact git users who use remote helpers to access foreign VCSs. All of those bugs have fixes later in this series. The fifth test is the most complex, and does not have a fix in this series. It tests pushing a ref via the export mechanism to a new name on the remote side (i.e., "git push $remote old:new"). The problem is that we push all of the work of generating the export stream onto fast-export, but we have no way of communicating to fast-export that this name mapping is happening. So we tell fast-export to generate a stream with the commits for "old", but we can't tell it to label them all as "new". Signed-off-by: Jeff King Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index f6796e3..9db8ca8 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -85,4 +85,51 @@ test_expect_success 'pushing remote local repo' ' compare_refs clone HEAD server HEAD ' +test_expect_failure 'fetch new branch' ' + (cd public && + git checkout -b new && + echo content >>file && + git commit -a -m five && + git push origin new + ) && + (cd localclone && + git fetch origin new + ) && + compare_refs public HEAD localclone FETCH_HEAD +' + +test_expect_failure 'fetch multiple branches' ' + (cd localclone && + git fetch + ) && + compare_refs server master localclone refs/remotes/origin/master && + compare_refs server new localclone refs/remotes/origin/new +' + +test_expect_failure 'push when remote has extra refs' ' + (cd clone && + echo content >>file && + git commit -a -m six && + git push + ) && + compare_refs clone master server master +' + +test_expect_failure 'push new branch by name' ' + (cd clone && + git checkout -b new-name && + echo content >>file && + git commit -a -m seven && + git push origin new-name + ) && + compare_refs clone HEAD server refs/heads/new-name +' + +test_expect_failure 'push new branch with old:new refspec' ' + (cd clone && + git push origin new-name:new-refspec + ) && + compare_refs clone HEAD server refs/heads/new-refspec +' + test_done -- cgit v0.10.2-6-g49f6 From 4e51ba238fb92ad732b4d34200fc8f53e29b333f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:25 +0200 Subject: git-remote-testgit: import non-HEAD refs Upon receiving an "import" command, the testgit remote helper would ignore the ref asked for by git and generate a fast-export stream based on HEAD. Instead, we should actually give git the ref it asked for. This requires adding a new parameter to the export_repo method in the remote-helpers python library, which may be used by code outside of git.git. We use a default parameter so that callers without the new parameter will get the same behavior as before. Signed-off-by: Jeff King Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index df9d512..e4a99a3 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -122,7 +122,7 @@ def do_import(repo, args): die("Need gitdir to import") repo = update_local_repo(repo) - repo.exporter.export_repo(repo.gitdir) + repo.exporter.export_repo(repo.gitdir, args) def do_export(repo, args): diff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py index f40f9d6..bc39163 100644 --- a/git_remote_helpers/git/exporter.py +++ b/git_remote_helpers/git/exporter.py @@ -15,7 +15,7 @@ class GitExporter(object): self.repo = repo - def export_repo(self, base): + def export_repo(self, base, refs=None): """Exports a fast-export stream for the given directory. Simply delegates to git fast-epxort and pipes it through sed @@ -23,8 +23,13 @@ class GitExporter(object): default refs/heads. This is to demonstrate how the export data can be stored under it's own ref (using the refspec capability). + + If None, refs defaults to ["HEAD"]. """ + if not refs: + refs = ["HEAD"] + dirname = self.repo.get_base_path(base) path = os.path.abspath(os.path.join(dirname, 'testgit.marks')) @@ -42,7 +47,7 @@ class GitExporter(object): if os.path.exists(path): args.append("--import-marks=" + path) - args.append("HEAD") + args.extend(refs) p1 = subprocess.Popen(args, stdout=subprocess.PIPE) diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index 9db8ca8..ca115cc 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -85,7 +85,7 @@ test_expect_success 'pushing remote local repo' ' compare_refs clone HEAD server HEAD ' -test_expect_failure 'fetch new branch' ' +test_expect_success 'fetch new branch' ' (cd public && git checkout -b new && echo content >>file && -- cgit v0.10.2-6-g49f6 From 3ea7d09461e4e1b95f6a55f04b2eb66d929464bd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:26 +0200 Subject: transport-helper: don't feed bogus refs to export push When we want to push to a remote helper that has the "export" capability, we collect all of the refs we want to push and then feed them to fast-export. However, the list of refs is actually a list of remote refs, not local refs. The mapped local refs are included via the peer_ref pointer. So when we add an argument to our fast-export command line, we must be sure to use the local peer_ref name (and if there is no local name, it is because we are not actually sending that ref, or we may not even have the ref at all). Signed-off-by: Jeff King Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index ca115cc..ceb0010 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -106,7 +106,7 @@ test_expect_failure 'fetch multiple branches' ' compare_refs server new localclone refs/remotes/origin/new ' -test_expect_failure 'push when remote has extra refs' ' +test_expect_success 'push when remote has extra refs' ' (cd clone && echo content >>file && git commit -a -m six && diff --git a/transport-helper.c b/transport-helper.c index b560b64..34d18aa 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -730,7 +730,8 @@ static int push_refs_with_export(struct transport *transport, } free(private); - string_list_append(&revlist_args, ref->name); + if (ref->peer_ref) + string_list_append(&revlist_args, ref->peer_ref->name); } -- cgit v0.10.2-6-g49f6 From b4b872994b59be397519ff76354ba4002e74de48 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 16 Jul 2011 15:03:27 +0200 Subject: git_remote_helpers: push all refs during a non-local export When a remote helper exports to a non-local git repo, the steps are roughly: 1. fast-export into a local staging area; the set of interesting refs is defined by what is in the fast-export stream 2. git push from the staging area to the non-local repo In the second step, we should explicitly push all refs, not just matching ones. This will let us push refs that do not yet exist in the remote repo. Signed-off-by: Jeff King Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py index f27389b..c53e074 100644 --- a/git_remote_helpers/git/non_local.py +++ b/git_remote_helpers/git/non_local.py @@ -63,7 +63,7 @@ class NonLocalGit(object): if not os.path.exists(path): die("could not find repo at %s", path) - args = ["git", "--git-dir=" + path, "push", "--quiet", self.repo.gitpath] + args = ["git", "--git-dir=" + path, "push", "--quiet", self.repo.gitpath, "--all"] child = subprocess.Popen(args) if child.wait() != 0: raise CalledProcessError diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index ceb0010..12f471c 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -115,7 +115,7 @@ test_expect_success 'push when remote has extra refs' ' compare_refs clone master server master ' -test_expect_failure 'push new branch by name' ' +test_expect_success 'push new branch by name' ' (cd clone && git checkout -b new-name && echo content >>file && -- cgit v0.10.2-6-g49f6 From e173587252ea0db16efc5c64c2cb165ccb406495 Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Sat, 16 Jul 2011 15:03:28 +0200 Subject: remote-helpers: export GIT_DIR variable to helpers The gitdir capability is recognized by git and can be used to tell the helper where the .git directory is. But it is not mentioned in the documentation and considered worse than if gitdir was passed via GIT_DIR environment variable. Remove support for the gitdir capability and export GIT_DIR instead. Teach testgit to use env instead of the now-removed gitdir command. [sr: fixed up documentation] Signed-off-by: Dmitry Ivankov Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt index 58f6ad4..18b8341 100644 --- a/Documentation/git-remote-helpers.txt +++ b/Documentation/git-remote-helpers.txt @@ -47,6 +47,9 @@ arguments. The first argument specifies a remote repository as in git; it is either the name of a configured remote or a URL. The second argument specifies a URL; it is usually of the form '://
', but any arbitrary string is possible. +The 'GIT_DIR' environment variable is set up for the remote helper +and can be used to determine where to store additional data or from +which directory to invoke auxiliary git commands. When git encounters a URL of the form '://
', where '' is a protocol that it cannot handle natively, it diff --git a/git-remote-testgit.py b/git-remote-testgit.py index e4a99a3..b0c1e9b 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -35,7 +35,7 @@ def get_repo(alias, url): prefix = 'refs/testgit/%s/' % alias debug("prefix: '%s'", prefix) - repo.gitdir = "" + repo.gitdir = os.environ["GIT_DIR"] repo.alias = alias repo.prefix = prefix @@ -70,7 +70,6 @@ def do_capabilities(repo, args): print "import" print "export" - print "gitdir" print "refspec refs/heads/*:%s*" % repo.prefix print # end capabilities @@ -150,22 +149,11 @@ def do_export(repo, args): repo.non_local.push(repo.gitdir) -def do_gitdir(repo, args): - """Stores the location of the gitdir. - """ - - if not args: - die("gitdir needs an argument") - - repo.gitdir = ' '.join(args) - - COMMANDS = { 'capabilities': do_capabilities, 'list': do_list, 'import': do_import, 'export': do_export, - 'gitdir': do_gitdir, } diff --git a/transport-helper.c b/transport-helper.c index 34d18aa..6cccb20 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -105,6 +105,12 @@ static struct child_process *get_helper(struct transport *transport) int refspec_alloc = 0; int duped; int code; + char git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1]; + const char *helper_env[] = { + git_dir_buf, + NULL + }; + if (data->helper) return data->helper; @@ -120,6 +126,10 @@ static struct child_process *get_helper(struct transport *transport) helper->argv[2] = remove_ext_force(transport->url); helper->git_cmd = 0; helper->silent_exec_failure = 1; + + snprintf(git_dir_buf, sizeof(git_dir_buf), "%s=%s", GIT_DIR_ENVIRONMENT, get_git_dir()); + helper->env = helper_env; + code = start_command(helper); if (code < 0 && errno == ENOENT) die("Unable to find remote helper for '%s'", data->name); @@ -174,11 +184,6 @@ static struct child_process *get_helper(struct transport *transport) refspecs[refspec_nr++] = strdup(buf.buf + strlen("refspec ")); } else if (!strcmp(capname, "connect")) { data->connect = 1; - } else if (!strcmp(buf.buf, "gitdir")) { - struct strbuf gitdir = STRBUF_INIT; - strbuf_addf(&gitdir, "gitdir %s\n", get_git_dir()); - sendline(data, &gitdir); - strbuf_release(&gitdir); } else if (mandatory) { die("Unknown mandatory capability %s. This remote " "helper probably needs newer version of Git.\n", -- cgit v0.10.2-6-g49f6 From 1843f0ce4d31e3c37f7d07dd1d13f6fb034ac2b0 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:29 +0200 Subject: remote-curl: accept empty line as terminator This went unnoticed because the transport helper infrastructore did not check the return value of the helper, nor did the helper print anything before exiting. While at it also make sure that the stream doesn't end unexpectedly. Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/remote-curl.c b/remote-curl.c index 17d8a9b..7b3c113 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -855,7 +855,14 @@ int main(int argc, const char **argv) http_init(remote); do { - if (strbuf_getline(&buf, stdin, '\n') == EOF) + if (strbuf_getline(&buf, stdin, '\n') == EOF) { + if (ferror(stdin)) + fprintf(stderr, "Error reading command stream\n"); + else + fprintf(stderr, "Unexpected end of command stream\n"); + return 1; + } + if (buf.len == 0) break; if (!prefixcmp(buf.buf, "fetch ")) { if (nongit) @@ -895,6 +902,7 @@ int main(int argc, const char **argv) printf("\n"); fflush(stdout); } else { + fprintf(stderr, "Unknown command '%s'\n", buf.buf); return 1; } strbuf_reset(&buf); -- cgit v0.10.2-6-g49f6 From 0fb56ce716090248ed4895aff69dd3953b00882f Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:30 +0200 Subject: git-remote-testgit: only push for non-local repositories Trying to push for local repositories will fail since there is no local checkout in .git/info/... to push from as that is only used for non-local repositories (local repositories are pushed to directly). This went unnoticed because the transport helper infrastructure does not check the return value of the helper. Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index b0c1e9b..cdbc494 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -146,7 +146,9 @@ def do_export(repo, args): update_local_repo(repo) repo.importer.do_import(repo.gitdir) - repo.non_local.push(repo.gitdir) + + if not repo.local: + repo.non_local.push(repo.gitdir) COMMANDS = { -- cgit v0.10.2-6-g49f6 From 460d10262dae14b54123ff45e7548d872ff63983 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:31 +0200 Subject: git-remote-testgit: fix error handling If fast-export did not complete successfully the error handling code itself would error out. This was broken in commit 23b093ee0 (Brandon Casey, Wed Jun 9 2010, Remove python 2.5'isms). Revert that commit an introduce our own copy of check_call in util.py instead. Tested by changing 'if retcode' to 'if not retcode' temporarily. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py index bc39163..9ee5f96 100644 --- a/git_remote_helpers/git/exporter.py +++ b/git_remote_helpers/git/exporter.py @@ -2,6 +2,8 @@ import os import subprocess import sys +from git_remote_helpers.util import check_call + class GitExporter(object): """An exporter for testgit repositories. @@ -53,6 +55,4 @@ class GitExporter(object): args = ["sed", "s_refs/heads/_" + self.repo.prefix + "_g"] - child = subprocess.Popen(args, stdin=p1.stdout) - if child.wait() != 0: - raise CalledProcessError + check_call(args, stdin=p1.stdout) diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py index 70a7127..02a719a 100644 --- a/git_remote_helpers/git/importer.py +++ b/git_remote_helpers/git/importer.py @@ -1,6 +1,8 @@ import os import subprocess +from git_remote_helpers.util import check_call + class GitImporter(object): """An importer for testgit repositories. @@ -35,6 +37,4 @@ class GitImporter(object): if os.path.exists(path): args.append("--import-marks=" + path) - child = subprocess.Popen(args) - if child.wait() != 0: - raise CalledProcessError + check_call(args) diff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py index c53e074..e700250 100644 --- a/git_remote_helpers/git/non_local.py +++ b/git_remote_helpers/git/non_local.py @@ -1,7 +1,7 @@ import os import subprocess -from git_remote_helpers.util import die, warn +from git_remote_helpers.util import check_call, die, warn class NonLocalGit(object): @@ -29,9 +29,7 @@ class NonLocalGit(object): os.makedirs(path) args = ["git", "clone", "--bare", "--quiet", self.repo.gitpath, path] - child = subprocess.Popen(args) - if child.wait() != 0: - raise CalledProcessError + check_call(args) return path @@ -45,14 +43,10 @@ class NonLocalGit(object): die("could not find repo at %s", path) args = ["git", "--git-dir=" + path, "fetch", "--quiet", self.repo.gitpath] - child = subprocess.Popen(args) - if child.wait() != 0: - raise CalledProcessError + check_call(args) args = ["git", "--git-dir=" + path, "update-ref", "refs/heads/master", "FETCH_HEAD"] - child = subprocess.Popen(args) - if child.wait() != 0: - raise CalledProcessError + child = check_call(args) def push(self, base): """Pushes from the non-local repo to base. @@ -64,6 +58,4 @@ class NonLocalGit(object): die("could not find repo at %s", path) args = ["git", "--git-dir=" + path, "push", "--quiet", self.repo.gitpath, "--all"] - child = subprocess.Popen(args) - if child.wait() != 0: - raise CalledProcessError + child = check_call(args) diff --git a/git_remote_helpers/git/repo.py b/git_remote_helpers/git/repo.py index 58e1cdb..acbf8d7 100644 --- a/git_remote_helpers/git/repo.py +++ b/git_remote_helpers/git/repo.py @@ -1,6 +1,9 @@ import os import subprocess +from git_remote_helpers.util import check_call + + def sanitize(rev, sep='\t'): """Converts a for-each-ref line to a name/value pair. """ @@ -53,9 +56,7 @@ class GitRepo(object): path = ".cached_revs" ofile = open(path, "w") - child = subprocess.Popen(args, stdout=ofile) - if child.wait() != 0: - raise CalledProcessError + check_call(args, stdout=ofile) output = open(path).readlines() self.revmap = dict(sanitize(i) for i in output) if "HEAD" in self.revmap: diff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py index dce83e6..1652c65 100644 --- a/git_remote_helpers/util.py +++ b/git_remote_helpers/util.py @@ -11,6 +11,21 @@ import sys import os import subprocess +try: + from subprocess import CalledProcessError +except ImportError: + # from python2.7:subprocess.py + # Exception classes used by this module. + class CalledProcessError(Exception): + """This exception is raised when a process run by check_call() returns + a non-zero exit status. The exit status will be stored in the + returncode attribute.""" + def __init__(self, returncode, cmd): + self.returncode = returncode + self.cmd = cmd + def __str__(self): + return "Command '%s' returned non-zero exit status %d" % (self.cmd, self.returncode) + # Whether or not to show debug messages DEBUG = False @@ -128,6 +143,38 @@ def run_command (args, cwd = None, shell = False, add_env = None, return (exit_code, output, errors) +# from python2.7:subprocess.py +def call(*popenargs, **kwargs): + """Run command with arguments. Wait for command to complete, then + return the returncode attribute. + + The arguments are the same as for the Popen constructor. Example: + + retcode = call(["ls", "-l"]) + """ + return subprocess.Popen(*popenargs, **kwargs).wait() + + +# from python2.7:subprocess.py +def check_call(*popenargs, **kwargs): + """Run command with arguments. Wait for command to complete. If + the exit code was zero then return, otherwise raise + CalledProcessError. The CalledProcessError object will have the + return code in the returncode attribute. + + The arguments are the same as for the Popen constructor. Example: + + check_call(["ls", "-l"]) + """ + retcode = call(*popenargs, **kwargs) + if retcode: + cmd = kwargs.get("args") + if cmd is None: + cmd = popenargs[0] + raise CalledProcessError(retcode, cmd) + return 0 + + def file_reader_method (missing_ok = False): """Decorator for simplifying reading of files. -- cgit v0.10.2-6-g49f6 From be56862f198d946dc75ac7092606e78a4f2ff1d3 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:32 +0200 Subject: fast-import: introduce 'done' command Add a 'done' command that causes fast-import to stop reading from the stream and exit. If the new --done command line flag was passed on the command line (or a "feature done" declaration included at the start of the stream), make the 'done' command mandatory. So "git fast-import --done"'s input format will be prefix-free, making errors easier to detect when they show up as early termination at some convenient time of the upstream of a pipe writing to fast-import. Another possible application of the 'done' command would to be allow a fast-import stream that is only a small part of a larger encapsulating stream to be easily parsed, leaving the file offset after the "done\n" so the other application can pick up from there. This patch does not teach fast-import to do that --- fast-import still uses buffered input (stdio). Signed-off-by: Jonathan Nieder Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 249249a..0fc68a9 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -101,6 +101,12 @@ OPTIONS when the `cat-blob` command is encountered in the stream. The default behaviour is to write to `stdout`. +--done:: + Require a `done` command at the end of the stream. + This option might be useful for detecting errors that + cause the frontend to terminate before it has started to + write a stream. + --export-pack-edges=:: After creating a packfile, print a line of data to listing the filename of the packfile and the last @@ -330,6 +336,11 @@ and control the current import process. More detailed discussion standard output. This command is optional and is not needed to perform an import. +`done`:: + Marks the end of the stream. This command is optional + unless the `done` feature was requested using the + `--done` command line option or `feature done` command. + `cat-blob`:: Causes fast-import to print a blob in 'cat-file --batch' format to the file descriptor set with `--cat-blob-fd` or @@ -1015,6 +1026,11 @@ notes:: Versions of fast-import not supporting notes will exit with a message indicating so. +done:: + Error out if the stream ends without a 'done' command. + Without this feature, errors causing the frontend to end + abruptly at a convenient point in the stream can go + undetected. `option` ~~~~~~~~ @@ -1044,6 +1060,15 @@ not be passed as option: * cat-blob-fd * force +`done` +~~~~~~ +If the `done` feature is not in use, treated as if EOF was read. +This can be used to tell fast-import to finish early. + +If the `--done` command line option or `feature done` command is +in use, the `done` command is mandatory and marks the end of the +stream. + Crash Reports ------------- If fast-import is supplied invalid input it will terminate with a diff --git a/fast-import.c b/fast-import.c index 78d9786..8a8a915 100644 --- a/fast-import.c +++ b/fast-import.c @@ -354,6 +354,7 @@ static unsigned int cmd_save = 100; static uintmax_t next_mark; static struct strbuf new_data = STRBUF_INIT; static int seen_data_command; +static int require_explicit_termination; /* Signal handling */ static volatile sig_atomic_t checkpoint_requested; @@ -3139,6 +3140,8 @@ static int parse_one_feature(const char *feature, int from_stream) relative_marks_paths = 1; } else if (!strcmp(feature, "no-relative-marks")) { relative_marks_paths = 0; + } else if (!strcmp(feature, "done")) { + require_explicit_termination = 1; } else if (!strcmp(feature, "force")) { force_update = 1; } else if (!strcmp(feature, "notes") || !strcmp(feature, "ls")) { @@ -3288,6 +3291,8 @@ int main(int argc, const char **argv) parse_reset_branch(); else if (!strcmp("checkpoint", command_buf.buf)) parse_checkpoint(); + else if (!strcmp("done", command_buf.buf)) + break; else if (!prefixcmp(command_buf.buf, "progress ")) parse_progress(); else if (!prefixcmp(command_buf.buf, "feature ")) @@ -3307,6 +3312,9 @@ int main(int argc, const char **argv) if (!seen_data_command) parse_argv(); + if (require_explicit_termination && feof(stdin)) + die("stream ends early"); + end_packfile(); dump_branches(); diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 6b1ba6c..526231b 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2197,6 +2197,48 @@ test_expect_success 'R: quiet option results in no stats being output' ' test_cmp empty output ' +test_expect_success 'R: feature done means terminating "done" is mandatory' ' + echo feature done | test_must_fail git fast-import && + test_must_fail git fast-import --done expect <<-\EOF && + OBJID + :000000 100644 OBJID OBJID A hello.c + :000000 100644 OBJID OBJID A hello2.c + EOF + git fast-import <<-EOF && + commit refs/heads/done-ends + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <actual && + test_cmp expect actual +' + cat >input < Date: Sat, 16 Jul 2011 15:03:33 +0200 Subject: fast-export: support done feature If fast-export is being used to generate a fast-import stream that will be used afterwards it is desirable to indicate the end of the stream with the new 'done' command. Add a flag that causes fast-export to end with 'done'. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt index 781bd6e..e3f8453 100644 --- a/Documentation/git-fast-export.txt +++ b/Documentation/git-fast-export.txt @@ -82,6 +82,10 @@ marks the same across runs. allow that. So fake a tagger to be able to fast-import the output. +--use-done-feature:: + Start the stream with a 'feature done' stanza, and terminate + it with a 'done' command. + --no-data:: Skip output of blob objects and instead refer to blobs via their original SHA-1 hash. This is useful when rewriting the diff --git a/builtin/fast-export.c b/builtin/fast-export.c index daf1945..becef85 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -26,6 +26,7 @@ static int progress; static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT; static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT; static int fake_missing_tagger; +static int use_done_feature; static int no_data; static int full_tree; @@ -627,6 +628,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix) "Fake a tagger when tags lack one"), OPT_BOOLEAN(0, "full-tree", &full_tree, "Output full tree for each commit"), + OPT_BOOLEAN(0, "use-done-feature", &use_done_feature, + "Use the done feature to terminate the stream"), { OPTION_NEGBIT, 0, "data", &no_data, NULL, "Skip output of blob data", PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 }, @@ -648,6 +651,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix) if (argc > 1) usage_with_options (fast_export_usage, options); + if (use_done_feature) + printf("feature done\n"); + if (import_filename) import_marks(import_filename); @@ -675,5 +681,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix) if (export_filename) export_marks(export_filename); + if (use_done_feature) + printf("done\n"); + return 0; } -- cgit v0.10.2-6-g49f6 From d2e73c6f2ac3e2d32cd27afd80bfa7c1661a52d4 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:34 +0200 Subject: transport-helper: factor out push_update_refs_status The update ref status part of push is useful for the export command as well, factor it out into it's own function. Also factor out push_update_ref_status to avoid a long loop without an explicit condition with a non-trivial body. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport-helper.c b/transport-helper.c index 6cccb20..dd8dd2c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -559,6 +559,88 @@ static int fetch(struct transport *transport, return -1; } +static void push_update_ref_status(struct strbuf *buf, + struct ref **ref, + struct ref *remote_refs) +{ + char *refname, *msg; + int status; + + if (!prefixcmp(buf->buf, "ok ")) { + status = REF_STATUS_OK; + refname = buf->buf + 3; + } else if (!prefixcmp(buf->buf, "error ")) { + status = REF_STATUS_REMOTE_REJECT; + refname = buf->buf + 6; + } else + die("expected ok/error, helper said '%s'\n", buf->buf); + + msg = strchr(refname, ' '); + if (msg) { + struct strbuf msg_buf = STRBUF_INIT; + const char *end; + + *msg++ = '\0'; + if (!unquote_c_style(&msg_buf, msg, &end)) + msg = strbuf_detach(&msg_buf, NULL); + else + msg = xstrdup(msg); + strbuf_release(&msg_buf); + + if (!strcmp(msg, "no match")) { + status = REF_STATUS_NONE; + free(msg); + msg = NULL; + } + else if (!strcmp(msg, "up to date")) { + status = REF_STATUS_UPTODATE; + free(msg); + msg = NULL; + } + else if (!strcmp(msg, "non-fast forward")) { + status = REF_STATUS_REJECT_NONFASTFORWARD; + free(msg); + msg = NULL; + } + } + + if (*ref) + *ref = find_ref_by_name(*ref, refname); + if (!*ref) + *ref = find_ref_by_name(remote_refs, refname); + if (!*ref) { + warning("helper reported unexpected status of %s", refname); + return; + } + + if ((*ref)->status != REF_STATUS_NONE) { + /* + * Earlier, the ref was marked not to be pushed, so ignore the ref + * status reported by the remote helper if the latter is 'no match'. + */ + if (status == REF_STATUS_NONE) + return; + } + + (*ref)->status = status; + (*ref)->remote_status = msg; +} + +static void push_update_refs_status(struct helper_data *data, + struct ref *remote_refs) +{ + struct strbuf buf = STRBUF_INIT; + struct ref *ref = remote_refs; + for (;;) { + recvline(data, &buf); + if (!buf.len) + break; + + push_update_ref_status(&buf, &ref, remote_refs); + } + strbuf_release(&buf); +} + static int push_refs_with_push(struct transport *transport, struct ref *remote_refs, int flags) { @@ -613,76 +695,9 @@ static int push_refs_with_push(struct transport *transport, strbuf_addch(&buf, '\n'); sendline(data, &buf); - - ref = remote_refs; - while (1) { - char *refname, *msg; - int status; - - recvline(data, &buf); - if (!buf.len) - break; - - if (!prefixcmp(buf.buf, "ok ")) { - status = REF_STATUS_OK; - refname = buf.buf + 3; - } else if (!prefixcmp(buf.buf, "error ")) { - status = REF_STATUS_REMOTE_REJECT; - refname = buf.buf + 6; - } else - die("expected ok/error, helper said '%s'\n", buf.buf); - - msg = strchr(refname, ' '); - if (msg) { - struct strbuf msg_buf = STRBUF_INIT; - const char *end; - - *msg++ = '\0'; - if (!unquote_c_style(&msg_buf, msg, &end)) - msg = strbuf_detach(&msg_buf, NULL); - else - msg = xstrdup(msg); - strbuf_release(&msg_buf); - - if (!strcmp(msg, "no match")) { - status = REF_STATUS_NONE; - free(msg); - msg = NULL; - } - else if (!strcmp(msg, "up to date")) { - status = REF_STATUS_UPTODATE; - free(msg); - msg = NULL; - } - else if (!strcmp(msg, "non-fast forward")) { - status = REF_STATUS_REJECT_NONFASTFORWARD; - free(msg); - msg = NULL; - } - } - - if (ref) - ref = find_ref_by_name(ref, refname); - if (!ref) - ref = find_ref_by_name(remote_refs, refname); - if (!ref) { - warning("helper reported unexpected status of %s", refname); - continue; - } - - if (ref->status != REF_STATUS_NONE) { - /* - * Earlier, the ref was marked not to be pushed, so ignore the ref - * status reported by the remote helper if the latter is 'no match'. - */ - if (status == REF_STATUS_NONE) - continue; - } - - ref->status = status; - ref->remote_status = msg; - } strbuf_release(&buf); + + push_update_refs_status(data, remote_refs); return 0; } -- cgit v0.10.2-6-g49f6 From cc567322acbfd5b32e61ab5d005352347cd7eeaf Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:35 +0200 Subject: transport-helper: check status code of finish_command Previously the status code of all helpers were ignored, allowing errors that occur to go unnoticed if the error text output by the helper is not noticed. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport-helper.c b/transport-helper.c index dd8dd2c..e02f4a3 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -209,6 +209,7 @@ static int disconnect_helper(struct transport *transport) { struct helper_data *data = transport->data; struct strbuf buf = STRBUF_INIT; + int res = 0; if (data->helper) { if (debug) @@ -220,13 +221,13 @@ static int disconnect_helper(struct transport *transport) close(data->helper->in); close(data->helper->out); fclose(data->out); - finish_command(data->helper); + res = finish_command(data->helper); free((char *)data->helper->argv[0]); free(data->helper->argv); free(data->helper); data->helper = NULL; } - return 0; + return res; } static const char *unsupported_options[] = { @@ -304,12 +305,13 @@ static void standard_options(struct transport *t) static int release_helper(struct transport *transport) { + int res = 0; struct helper_data *data = transport->data; free_refspec(data->refspec_nr, data->refspecs); data->refspecs = NULL; - disconnect_helper(transport); + res = disconnect_helper(transport); free(transport->data); - return 0; + return res; } static int fetch_with_fetch(struct transport *transport, @@ -415,8 +417,11 @@ static int fetch_with_import(struct transport *transport, sendline(data, &buf); strbuf_reset(&buf); } - disconnect_helper(transport); - finish_command(&fastimport); + if (disconnect_helper(transport)) + die("Error while disconnecting helper"); + if (finish_command(&fastimport)) + die("Error while running fast-import"); + free(fastimport.argv); fastimport.argv = NULL; @@ -760,8 +765,10 @@ static int push_refs_with_export(struct transport *transport, die("Couldn't run fast-export"); data->no_disconnect_req = 1; - finish_command(&exporter); - disconnect_helper(transport); + if (finish_command(&exporter)) + die("Error while running fast-export"); + if (disconnect_helper(transport)) + die("Error while disconnecting helper"); return 0; } -- cgit v0.10.2-6-g49f6 From 1f25c50419c5f46cd6b818438fe641cf942ee6ad Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:36 +0200 Subject: transport-helper: use the new done feature where possible In other words, use fast-export --use-done-feature to add a 'done' command at the end of streams passed to remote helpers' "import" commands, and teach the remote helpers implementing "export" to use the 'done' command in turn when producing their streams. The trailing \n in the protocol signals the helper that the connection is about to close, allowing it to do whatever cleanup neccesary. Previously, the connection would already be closed by the time the trailing \n was to be written. Now that the remote-helper protocol uses the new done command in its fast-import streams, this is no longer the case and we can safely write the trailing \n. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index cdbc494..af4d040 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -123,6 +123,8 @@ def do_import(repo, args): repo = update_local_repo(repo) repo.exporter.export_repo(repo.gitdir, args) + print "done" + def do_export(repo, args): """Imports a fast-import stream from git to testgit. diff --git a/transport-helper.c b/transport-helper.c index e02f4a3..4c0d861 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -380,8 +380,9 @@ static int get_exporter(struct transport *transport, /* we need to duplicate helper->in because we want to use it after * fastexport is done with it. */ fastexport->out = dup(helper->in); - fastexport->argv = xcalloc(4 + revlist_args->nr, sizeof(*fastexport->argv)); + fastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv)); fastexport->argv[argc++] = "fast-export"; + fastexport->argv[argc++] = "--use-done-feature"; if (export_marks) fastexport->argv[argc++] = export_marks; if (import_marks) @@ -417,11 +418,8 @@ static int fetch_with_import(struct transport *transport, sendline(data, &buf); strbuf_reset(&buf); } - if (disconnect_helper(transport)) - die("Error while disconnecting helper"); if (finish_command(&fastimport)) die("Error while running fast-import"); - free(fastimport.argv); fastimport.argv = NULL; @@ -764,11 +762,8 @@ static int push_refs_with_export(struct transport *transport, export_marks, import_marks, &revlist_args)) die("Couldn't run fast-export"); - data->no_disconnect_req = 1; if (finish_command(&exporter)) die("Error while running fast-export"); - if (disconnect_helper(transport)) - die("Error while disconnecting helper"); return 0; } -- cgit v0.10.2-6-g49f6 From 6c8151a32e59c3109b3acc886358bfe6c14612fb Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:37 +0200 Subject: transport-helper: update ref status after push with export Also add check_output from python 2.7. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index af4d040..0b5928d 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -147,11 +147,15 @@ def do_export(repo, args): sys.stdout.flush() update_local_repo(repo) - repo.importer.do_import(repo.gitdir) + changed = repo.importer.do_import(repo.gitdir) if not repo.local: repo.non_local.push(repo.gitdir) + for ref in changed: + print "ok %s" % ref + print + COMMANDS = { 'capabilities': do_capabilities, diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py index 02a719a..5c6b595 100644 --- a/git_remote_helpers/git/importer.py +++ b/git_remote_helpers/git/importer.py @@ -1,7 +1,7 @@ import os import subprocess -from git_remote_helpers.util import check_call +from git_remote_helpers.util import check_call, check_output class GitImporter(object): @@ -16,6 +16,18 @@ class GitImporter(object): self.repo = repo + def get_refs(self, gitdir): + """Returns a dictionary with refs. + """ + args = ["git", "--git-dir=" + gitdir, "for-each-ref", "refs/heads"] + lines = check_output(args).strip().split('\n') + refs = {} + for line in lines: + value, name = line.split(' ') + name = name.strip('commit\t') + refs[name] = value + return refs + def do_import(self, base): """Imports a fast-import stream to the given directory. @@ -32,9 +44,23 @@ class GitImporter(object): if not os.path.exists(dirname): os.makedirs(dirname) + refs_before = self.get_refs(gitdir) + args = ["git", "--git-dir=" + gitdir, "fast-import", "--quiet", "--export-marks=" + path] if os.path.exists(path): args.append("--import-marks=" + path) check_call(args) + + refs_after = self.get_refs(gitdir) + + changed = {} + + for name, value in refs_after.iteritems(): + if refs_before.get(name) == value: + continue + + changed[name] = value + + return changed diff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py index 1652c65..fbbb01b 100644 --- a/git_remote_helpers/util.py +++ b/git_remote_helpers/util.py @@ -175,6 +175,40 @@ def check_call(*popenargs, **kwargs): return 0 +# from python2.7:subprocess.py +def check_output(*popenargs, **kwargs): + r"""Run command with arguments and return its output as a byte string. + + If the exit code was non-zero it raises a CalledProcessError. The + CalledProcessError object will have the return code in the returncode + attribute and output in the output attribute. + + The arguments are the same as for the Popen constructor. Example: + + >>> check_output(["ls", "-l", "/dev/null"]) + 'crw-rw-rw- 1 root root 1, 3 Oct 18 2007 /dev/null\n' + + The stdout argument is not allowed as it is used internally. + To capture standard error in the result, use stderr=STDOUT. + + >>> check_output(["/bin/sh", "-c", + ... "ls -l non_existent_file ; exit 0"], + ... stderr=STDOUT) + 'ls: non_existent_file: No such file or directory\n' + """ + if 'stdout' in kwargs: + raise ValueError('stdout argument not allowed, it will be overridden.') + process = subprocess.Popen(stdout=subprocess.PIPE, *popenargs, **kwargs) + output, unused_err = process.communicate() + retcode = process.poll() + if retcode: + cmd = kwargs.get("args") + if cmd is None: + cmd = popenargs[0] + raise subprocess.CalledProcessError(retcode, cmd) + return output + + def file_reader_method (missing_ok = False): """Decorator for simplifying reading of files. diff --git a/transport-helper.c b/transport-helper.c index 4c0d861..a8f69b0 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -764,6 +764,7 @@ static int push_refs_with_export(struct transport *transport, if (finish_command(&exporter)) die("Error while running fast-export"); + push_update_refs_status(data, remote_refs); return 0; } -- cgit v0.10.2-6-g49f6 From 9504bc9d5a1e672ce5945679f86294e61bbea3a6 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:38 +0200 Subject: transport-helper: change import semantics Currently the helper must somehow guess how many import statements to read before it starts outputting its fast-export stream. This is because the remote helper infrastructure runs fast-import only once, so the helper is forced to output one stream for all import commands it will receive. The only reason this worked in the past was because only one ref was imported at a time. Change the semantics of the import statement such that it matches that of the push statement. That is, the import statement is followed by a series of import statements that are terminated by a '\n'. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index 0b5928d..1ed7a56 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -120,8 +120,22 @@ def do_import(repo, args): if not repo.gitdir: die("Need gitdir to import") + ref = args[0] + refs = [ref] + + while True: + line = sys.stdin.readline() + if line == '\n': + break + if not line.startswith('import '): + die("Expected import line.") + + # strip of leading 'import ' + ref = line[7:].strip() + refs.append(ref) + repo = update_local_repo(repo) - repo.exporter.export_repo(repo.gitdir, args) + repo.exporter.export_repo(repo.gitdir, refs) print "done" diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh index 12f471c..1c62001 100755 --- a/t/t5800-remote-helpers.sh +++ b/t/t5800-remote-helpers.sh @@ -98,7 +98,7 @@ test_expect_success 'fetch new branch' ' compare_refs public HEAD localclone FETCH_HEAD ' -test_expect_failure 'fetch multiple branches' ' +test_expect_success 'fetch multiple branches' ' (cd localclone && git fetch ) && diff --git a/transport-helper.c b/transport-helper.c index a8f69b0..0c00be9 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -418,6 +418,9 @@ static int fetch_with_import(struct transport *transport, sendline(data, &buf); strbuf_reset(&buf); } + + write_constant(data->helper->in, "\n"); + if (finish_command(&fastimport)) die("Error while running fast-import"); free(fastimport.argv); -- cgit v0.10.2-6-g49f6 From 4d2ec306e88b4de5aeb611b18b00139c90d4fa78 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:39 +0200 Subject: transport-helper: Use capname for refspec capability too Previously the refspec capability could not be listed as required or their parsing would break. Most likely the reason the second hunk wasn't caught is because the series that added 'refspec' as capability, and the one that added required capabilities were done in parallel. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport-helper.c b/transport-helper.c index 0c00be9..0cfc9ae 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -181,7 +181,7 @@ static struct child_process *get_helper(struct transport *transport) ALLOC_GROW(refspecs, refspec_nr + 1, refspec_alloc); - refspecs[refspec_nr++] = strdup(buf.buf + strlen("refspec ")); + refspecs[refspec_nr++] = strdup(capname + strlen("refspec ")); } else if (!strcmp(capname, "connect")) { data->connect = 1; } else if (mandatory) { -- cgit v0.10.2-6-g49f6 From a515ebe9f1ac9bc248c12a291dc008570de505ca Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:40 +0200 Subject: transport-helper: implement marks location as capability Now that the gitdir location is exported as an environment variable this can be implemented elegantly without requiring any explicit flushes nor an ad-hoc exchange of values. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-remote-testgit.py b/git-remote-testgit.py index 1ed7a56..e9c832b 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -72,6 +72,17 @@ def do_capabilities(repo, args): print "export" print "refspec refs/heads/*:%s*" % repo.prefix + dirname = repo.get_base_path(repo.gitdir) + + if not os.path.exists(dirname): + os.makedirs(dirname) + + path = os.path.join(dirname, 'testgit.marks') + + print "*export-marks %s" % path + if os.path.exists(path): + print "*import-marks %s" % path + print # end capabilities @@ -147,19 +158,6 @@ def do_export(repo, args): if not repo.gitdir: die("Need gitdir to export") - dirname = repo.get_base_path(repo.gitdir) - - if not os.path.exists(dirname): - os.makedirs(dirname) - - path = os.path.join(dirname, 'testgit.marks') - print path - if os.path.exists(path): - print path - else: - print "" - sys.stdout.flush() - update_local_repo(repo) changed = repo.importer.do_import(repo.gitdir) diff --git a/transport-helper.c b/transport-helper.c index 0cfc9ae..74c3122 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -23,6 +23,8 @@ struct helper_data { push : 1, connect : 1, no_disconnect_req : 1; + char *export_marks; + char *import_marks; /* These go from remote name (as in "list") to private name */ struct refspec *refspecs; int refspec_nr; @@ -184,6 +186,16 @@ static struct child_process *get_helper(struct transport *transport) refspecs[refspec_nr++] = strdup(capname + strlen("refspec ")); } else if (!strcmp(capname, "connect")) { data->connect = 1; + } else if (!prefixcmp(capname, "export-marks ")) { + struct strbuf arg = STRBUF_INIT; + strbuf_addstr(&arg, "--export-marks="); + strbuf_addstr(&arg, capname + strlen("export-marks ")); + data->export_marks = strbuf_detach(&arg, NULL); + } else if (!prefixcmp(capname, "import-marks")) { + struct strbuf arg = STRBUF_INIT; + strbuf_addstr(&arg, "--import-marks="); + strbuf_addstr(&arg, capname + strlen("import-marks ")); + data->import_marks = strbuf_detach(&arg, NULL); } else if (mandatory) { die("Unknown mandatory capability %s. This remote " "helper probably needs newer version of Git.\n", @@ -369,10 +381,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti static int get_exporter(struct transport *transport, struct child_process *fastexport, - const char *export_marks, - const char *import_marks, struct string_list *revlist_args) { + struct helper_data *data = transport->data; struct child_process *helper = get_helper(transport); int argc = 0, i; memset(fastexport, 0, sizeof(*fastexport)); @@ -383,10 +394,10 @@ static int get_exporter(struct transport *transport, fastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv)); fastexport->argv[argc++] = "fast-export"; fastexport->argv[argc++] = "--use-done-feature"; - if (export_marks) - fastexport->argv[argc++] = export_marks; - if (import_marks) - fastexport->argv[argc++] = import_marks; + if (data->export_marks) + fastexport->argv[argc++] = data->export_marks; + if (data->import_marks) + fastexport->argv[argc++] = data->import_marks; for (i = 0; i < revlist_args->nr; i++) fastexport->argv[argc++] = revlist_args->items[i].string; @@ -713,7 +724,6 @@ static int push_refs_with_export(struct transport *transport, struct ref *ref; struct child_process *helper, exporter; struct helper_data *data = transport->data; - char *export_marks = NULL, *import_marks = NULL; struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; @@ -721,26 +731,6 @@ static int push_refs_with_export(struct transport *transport, write_constant(helper->in, "export\n"); - recvline(data, &buf); - if (debug) - fprintf(stderr, "Debug: Got export_marks '%s'\n", buf.buf); - if (buf.len) { - struct strbuf arg = STRBUF_INIT; - strbuf_addstr(&arg, "--export-marks="); - strbuf_addbuf(&arg, &buf); - export_marks = strbuf_detach(&arg, NULL); - } - - recvline(data, &buf); - if (debug) - fprintf(stderr, "Debug: Got import_marks '%s'\n", buf.buf); - if (buf.len) { - struct strbuf arg = STRBUF_INIT; - strbuf_addstr(&arg, "--import-marks="); - strbuf_addbuf(&arg, &buf); - import_marks = strbuf_detach(&arg, NULL); - } - strbuf_reset(&buf); for (ref = remote_refs; ref; ref = ref->next) { @@ -761,8 +751,7 @@ static int push_refs_with_export(struct transport *transport, } - if (get_exporter(transport, &exporter, - export_marks, import_marks, &revlist_args)) + if (get_exporter(transport, &exporter, &revlist_args)) die("Couldn't run fast-export"); if (finish_command(&exporter)) -- cgit v0.10.2-6-g49f6 From 105fe3e457fba1dc005aa649410ec3962624232a Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Sat, 16 Jul 2011 15:03:41 +0200 Subject: transport-helper: die early on encountering deleted refs Remote helpers do not support deleting refs by means of the 'export' command sincethe fast-import protocol does not support it. Check explicitly for deleted refs and die early. Signed-off-by: Sverre Rabbelier Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport-helper.c b/transport-helper.c index 74c3122..4eab844 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -746,6 +746,10 @@ static int push_refs_with_export(struct transport *transport, } free(private); + if (ref->deletion) { + die("remote-helpers do not support ref deletion"); + } + if (ref->peer_ref) string_list_append(&revlist_args, ref->peer_ref->name); -- cgit v0.10.2-6-g49f6