{"thread":{"id":"31907","subject":"[PATCH] remote-testgit: properly check for errors","startedAt":"2012-10-22T20:56:34Z","lastAt":"2012-10-23T00:57:11Z","messageCount":3,"participants":["Felipe Contreras","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"201708","messageId":"1350939394-21622-1-git-send-email-felipe.contreras@gmail.com","threadId":"31907","inReplyTo":null,"subject":"[PATCH] remote-testgit: properly check for errors","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-22T20:56:34Z","receivedAt":"2012-10-22T20:56:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"'feature done' was missing, which allowed fast-import exit properly, and\ntransport-helper to continue checking for refs and what not when in fact\nthe remote-helper died.\n\nLet's enable that, and make sure the error paths are triggered.\n\nNow transport-helper correctly detects the errors from fast-import,\nunfortunately, not from fast-export because it might finish before\ndetecting a SIGPIPE. This means transport-helper will quit silently and\nthe user will not see any errors, which is bad. Hopefully the helper\nwill print the error before dying anyway, so not all is lost.\n\nSigned-off-by: Felipe Contreras \n---\n git-remote-testgit.py | 8 ++++++++\n t/t5800-remote-helpers.sh | 21 +++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex 5f3ebd2..b8707e6 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -159,6 +159,11 @@ def do_import(repo, args):\n ref = line[7:].strip()\n refs.append(ref)\n \n+ print \"feature done\"\n+\n+ if os.environ.get(\"GIT_REMOTE_TESTGIT_FAILURE\"):\n+ die('Told to fail')\n+\n repo = update_local_repo(repo)\n repo.exporter.export_repo(repo.gitdir, refs)\n \n@@ -172,6 +177,9 @@ def do_export(repo, args):\n if not repo.gitdir:\n die(\"Need gitdir to export\")\n \n+ if os.environ.get(\"GIT_REMOTE_TESTGIT_FAILURE\"):\n+ die('Told to fail')\n+\n update_local_repo(repo)\n changed = repo.importer.do_import(repo.gitdir)\n \ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex e7dc668..446cc8e 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -145,4 +145,25 @@ test_expect_failure 'push new branch with old:new refspec' '\n \tcompare_refs clone HEAD server refs/heads/new-refspec\n '\n \n+test_expect_success 'proper failure checks for fetching' '\n+\t(GIT_REMOTE_TESTGIT_FAILURE=1 &&\n+\texport GIT_REMOTE_TESTGIT_FAILURE &&\n+\tcd localclone &&\n+\ttest_must_fail git fetch 2>&1 | \\\n+\t\tgrep \"Error while running fast-import\"\n+\t)\n+'\n+\n+# We sleep to give fast-export a chance to catch the SIGPIPE\n+test_expect_failure 'proper failure checks for pushing' '\n+\t(GIT_REMOTE_TESTGIT_FAILURE=1 &&\n+\texport GIT_REMOTE_TESTGIT_FAILURE &&\n+\tGIT_REMOTE_TESTGIT_SLEEPY=1 &&\n+\texport GIT_REMOTE_TESTGIT_SLEEPY &&\n+\tcd localclone &&\n+\ttest_must_fail git push --all 2>&1 | \\\n+\t\tgrep \"Error while running fast-export\"\n+\t)\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"201709","messageId":"CAGdFq_gOb3GX1iZshGZahZEbj3RqQSEk8AN_hHXKyD5aHwfxYA@mail.gmail.com","threadId":"31907","inReplyTo":"1350939394-21622-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] remote-testgit: properly check for errors","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2012-10-22T21:01:23Z","receivedAt":"2012-10-22T21:01:23Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Mon, Oct 22, 2012 at 1:56 PM, Felipe Contreras\n wrote:\n> diff --git a/git-remote-testgit.py b/git-remote-testgit.py\n> index 5f3ebd2..b8707e6 100644\n> --- a/git-remote-testgit.py\n> +++ b/git-remote-testgit.py\n> @@ -159,6 +159,11 @@ def do_import(repo, args):\n> ref = line[7:].strip()\n> refs.append(ref)\n>\n> + print \"feature done\"\n\nThere's not enough context for me to see in which part of the code\nthis is, import or export? If you advertise 'feature done', shouldn't\nyou also print 'done' when you finish writing the fast-export stream?\nIf that's already the case feel free to ignore me :)\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"201716","messageId":"CAMP44s2=yxXGzUoxu=6baPv-6oq+UA9g1JdyqSSUYHS2800S2A@mail.gmail.com","threadId":"31907","inReplyTo":"CAGdFq_gOb3GX1iZshGZahZEbj3RqQSEk8AN_hHXKyD5aHwfxYA@mail.gmail.com","subject":"Re: [PATCH] remote-testgit: properly check for errors","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-23T00:57:11Z","receivedAt":"2012-10-23T00:57:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Oct 22, 2012 at 11:01 PM, Sverre Rabbelier wrote:\n> On Mon, Oct 22, 2012 at 1:56 PM, Felipe Contreras\n> wrote:\n>> diff --git a/git-remote-testgit.py b/git-remote-testgit.py\n>> index 5f3ebd2..b8707e6 100644\n>> --- a/git-remote-testgit.py\n>> +++ b/git-remote-testgit.py\n>> @@ -159,6 +159,11 @@ def do_import(repo, args):\n>> ref = line[7:].strip()\n>> refs.append(ref)\n>>\n>> + print \"feature done\"\n>\n> There's not enough context for me to see in which part of the code\n> this is, import or export?\n\nIsn't this enough?\n>> @@ -159,6 +159,11 @@ def do_import(repo, args):\n\nIt's import.\n\n> If you advertise 'feature done', shouldn't\n> you also print 'done' when you finish writing the fast-export stream?\n> If that's already the case feel free to ignore me :)\n\nIt's already there:\nhttp://git.kernel.org/?p=git/git.git;a=blob;f=git-remote-testgit.py#l165\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}