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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Oct 22, 2012, 19:35 UTC
Message-ID: <CAMP44s2HRHjoohFOdLT1F6cbGhknmaT3Z5tFfp1gpokoGMxCdQ@mail.gmail.com>
In-Reply-To: <CAMP44s27oPMu8Goded-Tcw9_fmgRux3yiNvu0FBPUxHOdp1Zgw@mail.gmail.com>

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

> On Mon, Oct 22, 2012 at 4:31 PM, Felipe Contreras

Show 9 quoted lines

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

Must be a timing issue:

sh -c 'echo hello' | sh -c 'exit 1' -> no signal sh -c 'echo hello' | /usr/bin/false -> SIGPIPE

I can trigger it by adding an extra delay:

This works:

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

This doesn't:

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

This does:

test_expect_success 'proper failure checks for pushing 3' '
	export GIT_REMOTE_TESTGIT_FAILURE=1 &&
	export GIT_REMOTE_TESTGIT_SLEEPY=1 &&
	(cd localclone && ! git push --all) 2> errors &&
	grep -q "Told to fail" errors
'

So, depending on your luck, transport-helper might or might display an error, it will exit at the right place nonetheless, because of:

	if (strbuf_getline(buffer, helper, '\n') == EOF) {
		if (debug)
			fprintf(stderr, "Debug: Remote helper quit.\n");
		exit(128);
	}

Not ideal, but I guess it's not a big deal.

Cheers.

-- 
Felipe Contreras