Re: [PATCH] transport-helper: check when helpers fail

On Mon, Oct 22, 2012 at 4:31 PM, Felipe Contreras
[off-list ref] wrote:

-> import-begin
<- feature X
<- feature Y
-> import refs/heads/master
<- exported stuff
-> import refs/heads/devel
<- exported stuff
-> import-end
<- done

This would certainly makes things easier for transport-helpers that
support multiple ref selections (like my remote-hg). Maybe I should
add code that does this if certain feature is specified (so it doesn't
break other helpers)

Never mind this, it's possible to do the same by assuming that all the
imports will be together, and finished by a line feed, so the code can
do:

if import
  do import-begin stuff
  while import
    import stuff
  do import-end stuff

Of course, this could break if for some reason transport-helper
changes, but that seems unlikely.

But at least on my tests, even with 'feature done' the crash is not
detected properly, either by the transport-helper, or fast-import.

Never mind this either; I was forcing the error before exporting that
feature. See the code at the end.

And also, the msysgit branch does the same check for fast-export,
which actually uses the 'done' feature always, so it should work fine,
but perhaps because of the strange issue with fast-import I just
mentioned, it's not actually detected. I should add tests for this
too.

I have added tests for this, and the failure is detected reliably...
but only with remote-testgit, not with my remote-hg, and I've no idea
what is different.

I've tried everything, and yet a SIGPIPE is detected only with
remote-testgit, not with my code, and they both exit the same way, and
at the same time, and fast-export exits the main function (apparently
a process can finish with SIGPIPE after main?)

I have no idea what's going on, so I don't know if we need any extra
code in transport-helper at all.

Any ideas?

Here is what I have so far:

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..b8707e6 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py

@@ -159,6 +159,11 @@ def do_import(repo, args):
         ref = line[7:].strip()
         refs.append(ref)

+    print "feature done"
+
+    if os.getenv("GIT_REMOTE_TESTGIT_FAILURE"):
+        die('Told to fail')
+
     repo = update_local_repo(repo)
     repo.exporter.export_repo(repo.gitdir, refs)

@@ -172,6 +177,9 @@ def do_export(repo, args):
     if not repo.gitdir:
         die("Need gitdir to export")

+    if os.getenv("GIT_REMOTE_TESTGIT_FAILURE"):
+        die('Told to fail')
+
     update_local_repo(repo)
     changed = repo.importer.do_import(repo.gitdir)

diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..00b0cd3 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh

@@ -145,4 +145,16 @@ test_expect_failure 'push new branch with old:new

refspec' '
 	compare_refs clone HEAD server refs/heads/new-refspec
 '

+test_expect_success 'proper failure checks for fetching' '
+	export GIT_REMOTE_TESTGIT_FAILURE=1 &&
+	(cd localclone && ! git fetch) 2> errors &&
+	grep -q "Error while running fast-import" errors
+'
+
+test_expect_success 'proper failure checks for pushing' '
+	export GIT_REMOTE_TESTGIT_FAILURE=1 &&
+	(cd localclone && ! git push --all) 2> errors &&
+	grep -q "Error while running fast-export" errors
+'
+
 test_done

-- 
Felipe Contreras