{"thread":{"id":"3865","subject":"Solaris test t5500 race condition","startedAt":"2006-04-14T03:17:59Z","lastAt":"2006-04-14T16:41:59Z","messageCount":5,"participants":["Peter Eriksen","Jason Riedy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"18633","messageId":"20060414031759.GA9524@bohr.gbar.dtu.dk","threadId":"3865","inReplyTo":null,"subject":"Solaris test t5500 race condition","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-04-14T03:17:59Z","receivedAt":"2006-04-14T03:17:59Z","isPatch":false,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"Hello,\n\nI've found a race in t5500-fetch-pack.sh.  The problem is the way the\nnumber of unpacked objects are counted:\n\n    pack_count=$(grep Unpacking log.txt|tr -dc \"0-9\")\n\nIt just concatenates all the digits on the line with \"Unpacking\" in it. \nThis is the output I get on Solaris:\n\n    Generating pack...\n    Done counting 3 objects.\n    Deltifying 3 objects.\n      33% (1/3) done^M  66% (2/3) done^M 100% (3/3) done\n    Total 3Unpacking , written 33 objects          <------------\n     (delta 0), reused 0 (delta 0)\n    11fa2f0cb58ed7f02dbd5ac75ed82a53fae62a7b refs/heads/A\n\nThe marked line is written as a joyful duet between these\ntwo functions:\n\n    unpack-objects.c:   fprintf(stderr, \"Unpacking %d objects\\n\",\n                                nr_objects);\n\n    pack-objects.c:     fprintf(stderr, \"Total %d, written %d \n                                (delta %d), reused %d (delta %d)\\n\",\n\nI can't think of a good solution right now.\n\nRegards,\n\nPeter\n"},{"id":"18635","messageId":"24787.1144991024@lotus.CS.Berkeley.EDU","threadId":"3865","inReplyTo":"20060414031759.GA9524@bohr.gbar.dtu.dk","subject":"Re: Solaris test t5500 race condition","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2006-04-14T05:03:44Z","receivedAt":"2006-04-14T05:03:44Z","isPatch":false,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And \"Peter Eriksen\" writes:\n - I've found a race in t5500-fetch-pack.sh.\n\nCrap.  I ran into this on AIX a while ago; I was hoping no\nother systems would see it.  There are no guarantees that \nthe two processes' outputs will be mutually line buffered.\nLuckily, it's just a cosmetic problem, but it does cause \nthat test case to fail.\n\nI know how to fix it (imho), but have no time to implement\nit.  There needs to be a separate communication stage after \nnegotiating the objects and before dumping the pack.  During\nthat stage, upload-pack would just send progress notices to \nthe caller.  Only the caller would communicate to the terminal.\nSome other ideas are in\n  http://marc.theaimsgroup.com/?l=git&m=114357528512063&w=2\n\nJason\n"},{"id":"18636","messageId":"7vhd4wvhyq.fsf@assigned-by-dhcp.cox.net","threadId":"3865","inReplyTo":"20060414031759.GA9524@bohr.gbar.dtu.dk","subject":"Re: Solaris test t5500 race condition","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-04-14T05:34:05Z","receivedAt":"2006-04-14T05:34:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Peter Eriksen\" <s022018@student.dtu.dk> writes:\n\n>     Generating pack...\n>     Done counting 3 objects.\n>     Deltifying 3 objects.\n>       33% (1/3) done^M  66% (2/3) done^M 100% (3/3) done\n>     Total 3Unpacking , written 33 objects          <------------\n>      (delta 0), reused 0 (delta 0)\n>     11fa2f0cb58ed7f02dbd5ac75ed82a53fae62a7b refs/heads/A\n\nHmph.  Not good.  Before the writer managed to flush the report\nthe reader has already decoded the header and reports the number\nof objects it is going to unpack.\n\nUnfortunately the Solaris box I have access to is perhaps\nsufficiently slow that this is not an issue X-<.\n\nI think test based on the eye-candy is fragile anyway.  We would\nwant to probably _count_ before and after to see if the command\ndid what we expected.\n\nThere is a subtle difficulty doing so, however.  The test is\ntrying to see if fetch-pack vs upload-pack negotiations result\nin minimal transfer, but if it is not, unpack side would just\nhappily say \"I received this one, oh, I already have it\".\n\nWe could do \"fetch-pack -k\" to keep the result packed, count the\nnumber of objects in the resulting pack.\n\nHow about doing something like this instead?\n\n-- >8 --\n[PATCH] t5500: test fix\n\nRelying on eye-candy progress bar was fragile to begin with.\nRun fetch-pack with -k option, and count the objects that are in\nthe pack that were transferred from the other end.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n t/t5500-fetch-pack.sh |   33 ++++++++++++++-------------------\n 1 files changed, 14 insertions(+), 19 deletions(-)\n\n7f732c632ff7a1adc2309257becdc0c1fe76b514\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex e15e14f..92f12d9 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -12,11 +12,6 @@ # Test fetch-pack/upload-pack pair.\n \n # Some convenience functions\n \n-function show_count () {\n-\tcommit_count=$(($commit_count+1))\n-\tprintf \"      %d\\r\" $commit_count\n-}\n-\n function add () {\n \tlocal name=$1\n \tlocal text=\"$@\"\n@@ -55,13 +50,6 @@ function test_expect_object_count () {\n \t\t\"test $count = $output\"\n }\n \n-function test_repack () {\n-\tlocal rep=$1\n-\n-\ttest_expect_success \"repack && prune-packed in $rep\" \\\n-\t\t'(git-repack && git-prune-packed)2>>log.txt'\n-}\n-\n function pull_to_client () {\n \tlocal number=$1\n \tlocal heads=$2\n@@ -70,13 +58,23 @@ function pull_to_client () {\n \n \tcd client\n \ttest_expect_success \"$number pull\" \\\n-\t\t\"git-fetch-pack -v .. $heads > log.txt 2>&1\"\n+\t\t\"git-fetch-pack -k -v .. $heads\"\n \tcase \"$heads\" in *A*) echo $ATIP > .git/refs/heads/A;; esac\n \tcase \"$heads\" in *B*) echo $BTIP > .git/refs/heads/B;; esac\n \tgit-symbolic-ref HEAD refs/heads/${heads:0:1}\n+\n \ttest_expect_success \"fsck\" 'git-fsck-objects --full > fsck.txt 2>&1'\n-\ttest_expect_object_count \"after $number pull\" $count\n-\tpack_count=$(grep Unpacking log.txt|tr -dc \"0-9\")\n+\n+\ttest_expect_success 'check downloaded results' \\\n+\t'mv .git/objects/pack/pack-* . &&\n+\t p=`ls -1 pack-*.pack` &&\n+\t git-unpack-objects <$p &&\n+\t git-fsck-objects --full'\n+\n+\ttest_expect_success \"new object count after $number pull\" \\\n+\t'idx=`echo pack-*.idx` &&\n+\t pack_count=`git-show-index <$idx | wc -l` &&\n+\t test $pack_count = $count'\n \ttest -z \"$pack_count\" && pack_count=0\n \tif [ -z \"$no_strict_count_check\" ]; then\n \t\ttest_expect_success \"minimal count\" \"test $count = $pack_count\"\n@@ -84,6 +82,7 @@ function pull_to_client () {\n \t\ttest $count != $pack_count && \\\n \t\t\techo \"WARNING: $pack_count objects transmitted, only $count of which were needed\"\n \tfi\n+\trm -f pack-*\n \tcd ..\n }\n \n@@ -117,8 +116,6 @@ git-symbolic-ref HEAD refs/heads/B\n \n pull_to_client 1st \"B A\" $((11*3))\n \n-(cd client; test_repack client)\n-\n add A11 $A10\n \n prev=1; cur=2; while [ $cur -le 65 ]; do\n@@ -129,8 +126,6 @@ done\n \n pull_to_client 2nd \"B\" $((64*3))\n \n-(cd client; test_repack client)\n-\n pull_to_client 3rd \"A\" $((1*3)) # old fails\n \n test_done\n-- \n1.3.0.rc3.g9306\n"},{"id":"18639","messageId":"20060414115317.GA5191@bohr.gbar.dtu.dk","threadId":"3865","inReplyTo":"7vhd4wvhyq.fsf@assigned-by-dhcp.cox.net","subject":"Re: Solaris test t5500 race condition","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-04-14T11:53:17Z","receivedAt":"2006-04-14T11:53:17Z","isPatch":false,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"On Thu, Apr 13, 2006 at 10:34:05PM -0700, Junio C Hamano wrote:\n> \"Peter Eriksen\" <s022018@student.dtu.dk> writes:\n> \n> >     Generating pack...\n> >     Done counting 3 objects.\n> >     Deltifying 3 objects.\n> >       33% (1/3) done^M  66% (2/3) done^M 100% (3/3) done\n> >     Total 3Unpacking , written 33 objects          <------------\n> >      (delta 0), reused 0 (delta 0)\n> >     11fa2f0cb58ed7f02dbd5ac75ed82a53fae62a7b refs/heads/A\n> \n> Hmph.  Not good.  Before the writer managed to flush the report\n> the reader has already decoded the header and reports the number\n> of objects it is going to unpack.\n...\n> -- >8 --\n> [PATCH] t5500: test fix\n\nWith the patch it doesn't complain anymore.  There are many other \nproblems with the tests on Solaris though.\n\nPeter\n"},{"id":"18643","messageId":"9647.1145032919@lotus.CS.Berkeley.EDU","threadId":"3865","inReplyTo":"20060414115317.GA5191@bohr.gbar.dtu.dk","subject":"Re: Solaris test t5500 race condition","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2006-04-14T16:41:59Z","receivedAt":"2006-04-14T16:41:59Z","isPatch":false,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And \"Peter Eriksen\" writes:\n - > -- >8 --\n - > [PATCH] t5500: test fix\n - \n - With the patch it doesn't complain anymore.  There are many other \n - problems with the tests on Solaris though.\n\nI just ran next branch's tests on 5.8 with no problems.  Could \nyou be a bit more specific?\n\nJason\n"}]}