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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Oct 22, 2012, 17:12 UTC
Message-ID: <CAMP44s27oPMu8Goded-Tcw9_fmgRux3yiNvu0FBPUxHOdp1Zgw@mail.gmail.com>
In-Reply-To: <CAMP44s1m9eVqqrgJFuWOBa3DCZAzAqpVwG8Nxn-6MbXWbF_2fw@mail.gmail.com>

On Mon, Oct 22, 2012 at 4:31 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 14 quoted lines

> -> 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.

Show 5 quoted lines

> 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