Thanks for your explanation Fredrik! However, I believe your 4. step is not what happens in the current implementation as the write call in wrapper.c dies directly. I see three ways to fix the problem: (1) Make upload-pack wait for a response (with timeout) before it closes the pipe. However, I believe this would not be in line with the general Git philosophy stated in "git.c" (added in 7559a1be): "Many parts of Git have subprograms communicate via pipe, expect the upstream of a pipe to die with SIGPIPE when the downstream of a pipe does not need to read all that is written." (2) Ignore SIGPIPE errors when "fetch-pack" sends a "done" using "sigchain_push(SIGPIPE, SIG_IGN)" / "sigchain_pop(SIGPIPE)". However, then the check_pipe function (added in 756e676c) kicks in and we would need to work around that as well. (3) Add a method "test_must_fail_or_die" to "test-lib-functions.sh". This method accepts exit codes 129<x<192, too. Use the new method in t5516. All this code is fairly new to me and I don't understand all the ramifications. That being said I prefer solution (3) as it is the simplest solution. Thanks, Lars On 25 Oct 2015, at 08:18, Fredrik Medley <fredrik.medley@xxxxxxxxx> wrote: > I think the following happens: > 1. The remote upload-pack finds out "not our ref" > 2. The remote send a response and close the pipe > 3. fetch-pack still tries to write commands to the remote upload-pack > 4. Because the connection has already been closed, writing will fail > with EPIPE which is detected by write_or_die() -> check_pipe() > resulting in die(141) > > The reason for the test to succeed, i.e. git-fetch fails with 128 (or > 1?), in most cases must be because it manages to write everything > before the context switch to the remote upload-pack occurs. > > What is actually the wanted outcome? Should git-fetch try to continue > to see if the already received response is enough to continue as > normal? > > Best regards, > Fredrik > > 2015-10-24 15:08 GMT+02:00 Lars Schneider <larsxschneider@xxxxxxxxx>: >> Hi, >> >> while working on the Git CI integration I noticed that t5516 "75 - deny fetch >> unreachable SHA1, allowtipsha1inwant=true" (introduced in 68ee628) seems to be >> flaky on TravisCI. I get the following output in verbose mode: >> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >> expecting success: >> mk_empty testrepo && >> ( >> cd testrepo && >> git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant && >> git commit --allow-empty -m foo && >> git commit --allow-empty -m bar && >> git commit --allow-empty -m xyz >> ) && >> SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) && >> SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) && >> SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) && >> ( >> cd testrepo && >> git reset --hard $SHA1_2 && >> git cat-file commit $SHA1_1 && >> git cat-file commit $SHA1_3 >> ) && >> mk_empty shallow && >> ( >> cd shallow && >> test_must_fail git fetch ../testrepo/.git $SHA1_3 && >> test_must_fail git fetch ../testrepo/.git $SHA1_1 && >> git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true && >> git fetch ../testrepo/.git $SHA1_1 && >> git cat-file commit $SHA1_1 && >> test_must_fail git cat-file commit $SHA1_2 && >> git fetch ../testrepo/.git $SHA1_2 && >> git cat-file commit $SHA1_2 && >> test_must_fail git fetch ../testrepo/.git $SHA1_3 >> ) >> >> Initialized empty Git repository in /home/travis/build/larsxschneider/git/t/trash directory.t5516-fetch-push/testrepo/.git/ >> [master (root-commit) a6b22ca] foo >> Author: A U Thor <author@xxxxxxxxxxx> >> [master 5e26403] bar >> Author: A U Thor <author@xxxxxxxxxxx> >> [master 64ea4c1] xyz >> Author: A U Thor <author@xxxxxxxxxxx> >> HEAD is now at 5e26403 bar >> tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >> author A U Thor <author@xxxxxxxxxxx> 1112912053 -0700 >> committer C O Mitter <committer@xxxxxxxxxxx> 1112912053 -0700 >> foo >> tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >> parent 5e26403b4485d7a44fd8b65dc3f71e21c0dd6fad >> author A U Thor <author@xxxxxxxxxxx> 1112912053 -0700 >> committer C O Mitter <committer@xxxxxxxxxxx> 1112912053 -0700 >> xyz >> Initialized empty Git repository in /home/travis/build/larsxschneider/git/t/trash directory.t5516-fetch-push/shallow/.git/ >> fatal: git upload-pack: not our ref 64ea4c133d59fa98e86a771eda009872d6ab2886 >> test_must_fail: died by signal: git fetch ../testrepo/.git 64ea4c133d59fa98e86a771eda009872d6ab2886 >> not ok 75 - deny fetch unreachable SHA1, allowtipsha1inwant=true >> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< >> >> "git fetch ../testrepo/.git $SHA1_3" seems to die as follows: >> 1. fetch-pack.c:408 goto done; >> 2. fetch-pack.c:447 done: >> 3. fetch-pack.c:229 static void send_request... (send "0009done\n" --> 9 bytes) >> 4. write_or_die.c:74 write_or_die >> 5. wrapper.c:331 write_in_full >> 6. wrapper.c:281 xwrite >> 7. wrapper.c:287 write --> just dies with exit code 141?! >> (use 4688abf to match the line numbers) >> >> You can find the full logs about the CI run here: >> https://travis-ci.org/larsxschneider/git/jobs/86984110 >> >> I repeatedly executed this test to demonstrate the flakiness: >> failed after 100 attempts: https://travis-ci.org/larsxschneider/git/jobs/87181753 >> failed after 1876 attempts: https://travis-ci.org/larsxschneider/git/jobs/87181754 >> (all executed on bbd2929 from https://github.com/larsxschneider/git) >> >> Unfortunately I was not able to make it fail on my local machine (OS X 10.9) >> nor on my VM (Ubuntu 14.10). Travis CI uses Ubuntu 12.04 LTS Server Edition 64 >> bit with a AUFS file system. >> >> Has anyone an idea what might causes these failures or can give me pointers how >> to investigate it? >> >> Thank you, >> Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html