[PATCH] remote-testgit: properly check for errors

[Felipe Contreras]

'feature done' was missing, which allowed fast-import exit properly, and
transport-helper to continue checking for refs and what not when in fact
the remote-helper died.

Let's enable that, and make sure the error paths are triggered.

Now transport-helper correctly detects the errors from fast-import,
unfortunately, not from fast-export because it might finish before
detecting a SIGPIPE. This means transport-helper will quit silently and
the user will not see any errors, which is bad. Hopefully the helper
will print the error before dying anyway, so not all is lost.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 git-remote-testgit.py |  8 
 t/t5800-remote-helpers.sh | 21 +
 2 files changed, 29 insertions(+)

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.environ.get(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.environ.get(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..446cc8e 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -145,4 +145,25 @@ 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' '
+   (GIT_REMOTE_TESTGIT_FAILURE=1 
+   export GIT_REMOTE_TESTGIT_FAILURE 
+   cd localclone 
+   test_must_fail git fetch 21 | \
+   grep Error while running fast-import
+   )
+'
+
+# We sleep to give fast-export a chance to catch the SIGPIPE
+test_expect_failure 'proper failure checks for pushing' '
+   (GIT_REMOTE_TESTGIT_FAILURE=1 
+   export GIT_REMOTE_TESTGIT_FAILURE 
+   GIT_REMOTE_TESTGIT_SLEEPY=1 
+   export GIT_REMOTE_TESTGIT_SLEEPY 
+   cd localclone 
+   test_must_fail git push --all 21 | \
+   grep Error while running fast-export
+   )
+'
+
 test_done
-- 
1.8.0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remote-testgit: properly check for errors

[Sverre Rabbelier]

On Mon, Oct 22, 2012 at 1:56 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 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

There's not enough context for me to see in which part of the code
this is, import or export? If you advertise 'feature done', shouldn't
you also print 'done' when you finish writing the fast-export stream?
If that's already the case feel free to ignore me :)

-- 
Cheers,

Sverre Rabbelier
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remote-testgit: properly check for errors

[Felipe Contreras]

On Mon, Oct 22, 2012 at 11:01 PM, Sverre Rabbelier srabbel...@gmail.com wrote:
 On Mon, Oct 22, 2012 at 1:56 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 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

 There's not enough context for me to see in which part of the code
 this is, import or export?

Isn't this enough?
 @@ -159,6 +159,11 @@ def do_import(repo, args):

It's import.

 If you advertise 'feature done', shouldn't
 you also print 'done' when you finish writing the fast-export stream?
 If that's already the case feel free to ignore me :)

It's already there:
http://git.kernel.org/?p=git/git.git;a=blob;f=git-remote-testgit.py#l165

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html