{"thread":{"id":"17389","subject":"[PATCH] Windows: Fix intermittent failures of t7701","startedAt":"2009-01-27T13:09:03Z","lastAt":"2009-01-28T12:43:04Z","messageCount":7,"participants":["Johannes Sixt","Johannes Schindelin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"102115","messageId":"497F076F.8060509@viscovery.net","threadId":"17389","inReplyTo":null,"subject":"[PATCH] Windows: Fix intermittent failures of t7701","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-01-27T13:09:03Z","receivedAt":"2009-01-27T13:09:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt <j6t@kdbg.org>\n\nThe last test case checks whether unpacked objects receive the time stamp\nof the pack file. Due to different implementations of stat(2) by MSYS and\nour version in compat/mingw.c, the test fails in about half of the test\nruns.\n\nNote the following facts:\n\n- The test uses perl's -M operator to compare the time stamps. Since we\n  depend on MSYS perl, the result of this operator is based on MSYS's\n  implementation of the stat(2) call.\n\n- NTFS on Windows records fractional seconds.\n\n- The MSYS implementation of stat(2) *rounds* fractional seconds to full\n  seconds instead of truncating them. This becomes obvious by comparing the\n  modification times reported by 'ls --full-time $f' and 'stat $f' for\n  various files $f.\n\n- Our implementation of stat(2) in compat/mingw.c *truncates* to full\n  seconds.\n\nThe consequence of this is that\n\n- add_packed_git() picks up truncated whole second modification times\n  from the pack file time stamp, which is then used for the loose objects,\n  while the pack file retains its time stamp in fractional seconds;\n\n- but the test case compares the pack file's rounded modification times\n  to the loose objects' truncated modification times.\n\nAnd half of the time the rounded modification time is not the same as its\ntruncated modification time.\n\nThe fix is that we replace perl by 'test-chmtime -v +0', which prints the\ntruncated whole-second mtime without modifying it.\n\nWe want to catch failures of test-chmtime; but since it appears in a pipe,\nwe cannot access its exit code. Therefore, we at least make sure that it\nprints time stamps of all files that are passed on its command line.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n t/t7701-repack-unpack-unreachable.sh |   15 +++++++++------\n 1 files changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex 63a8225..e6e9f99 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -50,12 +50,15 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '\n\n compare_mtimes ()\n {\n-\tperl -e 'my $reference = shift;\n-\t\t foreach my $file (@ARGV) {\n-\t\t\texit(1) unless(-f $file && -M $file == -M $reference);\n-\t\t }\n-\t\t exit(0);\n-\t\t' -- \"$@\"\n+\ttest-chmtime -v +0 \"$@\" |\n+\t{\n+\t\tread ref files &&\n+\t\twhile read t name; do\n+\t\t\ttest $ref = $t || break\n+\t\t\tfiles=\"$files $name\"\n+\t\tdone &&\n+\t\ttest \"$files\" = \"$*\"\n+\t}\n }\n\n test_expect_success '-A without -d option leaves unreachable objects packed' '\n-- \n1.6.1.1.1203.g5882\n"},{"id":"102133","messageId":"alpine.DEB.1.00.0901271740320.3586@pacific.mpi-cbg.de","threadId":"17389","inReplyTo":"497F076F.8060509@viscovery.net","subject":"Re: [PATCH] Windows: Fix intermittent failures of t7701","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-27T16:42:03Z","receivedAt":"2009-01-27T16:42:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jan 2009, Johannes Sixt wrote:\n\n> We want to catch failures of test-chmtime; but since it appears in a \n> pipe, we cannot access its exit code. Therefore, we at least make sure \n> that it prints time stamps of all files that are passed on its command \n> line.\n\nI use this trick in my valgrind series:\n\n\t($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&\n\ttest 0 = \"$(cat exit.code)\"\n\nCiao,\nDscho\n"},{"id":"102245","messageId":"20090128042814.GA18216@coredump.intra.peff.net","threadId":"17389","inReplyTo":"alpine.DEB.1.00.0901271740320.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Windows: Fix intermittent failures of t7701","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-28T04:28:14Z","receivedAt":"2009-01-28T04:28:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 27, 2009 at 05:42:03PM +0100, Johannes Schindelin wrote:\n\n> > We want to catch failures of test-chmtime; but since it appears in a \n> > pipe, we cannot access its exit code. Therefore, we at least make sure \n> > that it prints time stamps of all files that are passed on its command \n> > line.\n> \n> I use this trick in my valgrind series:\n> \n> \t($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&\n> \ttest 0 = \"$(cat exit.code)\"\n\nOh, that's far too readable. How about:\n\n  exec 3>&1\n  status=$( ( ($PROGRAM ; echo $? >&4) | $OTHER_PROGRAM >&3) 4>&1 )\n  exec 3>&-\n\nBut seriously, I think if we are talking about tests, then\n\n  $PROGRAM >output &&\n  $OTHER_PROGRAM <output\n\nis very clear to read, and as a bonus makes \"output\" accessible for\nviewing when the test breaks. The downside is that it isn't very\nefficient for a large output, but most of our test output is small (and\nwe don't care that much about efficiency).\n\nJust my shell bikeshedding 2 cents. Feel free to ignore.\n\n-Peff\n"},{"id":"102262","messageId":"498008BA.2000201@viscovery.net","threadId":"17389","inReplyTo":"alpine.DEB.1.00.0901271740320.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Windows: Fix intermittent failures of t7701","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-01-28T07:26:50Z","receivedAt":"2009-01-28T07:26:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> I use this trick in my valgrind series:\n> \n> \t($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&\n> \ttest 0 = \"$(cat exit.code)\"\n\nAh, using a file as temporary storage? Why not simply\n\n\t$PROGRAM > data &&\n\t$OTHER_PROGRAM < data\n\nHm? ;)\n\n-- Hannes\n"},{"id":"102281","messageId":"7v3af3vnti.fsf@gitster.siamese.dyndns.org","threadId":"17389","inReplyTo":"498008BA.2000201@viscovery.net","subject":"Re: [PATCH] Windows: Fix intermittent failures of t7701","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T08:26:17Z","receivedAt":"2009-01-28T08:26:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Johannes Schindelin schrieb:\n>> I use this trick in my valgrind series:\n>> \n>> \t($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&\n>> \ttest 0 = \"$(cat exit.code)\"\n>\n> Ah, using a file as temporary storage? Why not simply\n>\n> \t$PROGRAM > data &&\n> \t$OTHER_PROGRAM < data\n>\n> Hm? ;)\n\nBecause too much cleverness often blinds clever minds.\n\nCare to reroll with the simpler, dumber but clearer version?\n"},{"id":"102290","messageId":"49802ADA.6030401@viscovery.net","threadId":"17389","inReplyTo":"7v3af3vnti.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] Windows: Fix intermittent failures of t7701","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-01-28T09:52:26Z","receivedAt":"2009-01-28T09:52:26Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt <j6t@kdbg.org>\n\nThe last test case checks whether unpacked objects receive the time stamp\nof the pack file. Due to different implementations of stat(2) by MSYS and\nour version in compat/mingw.c, the test fails in about half of the test\nruns.\n\nNote the following facts:\n\n- The test uses perl's -M operator to compare the time stamps. Since we\n  depend on MSYS perl, the result of this operator is based on MSYS's\n  implementation of the stat(2) call.\n\n- NTFS on Windows records fractional seconds.\n\n- The MSYS implementation of stat(2) *rounds* fractional seconds to full\n  seconds instead of truncating them. This becomes obvious by comparing the\n  modification times reported by 'ls --full-time $f' and 'stat $f' for\n  various files $f.\n\n- Our implementation of stat(2) in compat/mingw.c *truncates* to full\n  seconds.\n\nThe consequence of this is that\n\n- add_packed_git() picks up a truncated whole second modification time\n  from the pack file time stamp, which is then used for the loose objects,\n  while the pack file retains its time stamp in fractional seconds;\n\n- but the test case compared the pack file's rounded modification times\n  to the loose objects' truncated modification times.\n\nAnd half of the time the rounded modification time is not the same as its\ntruncated modification time.\n\nThe fix is that we replace perl by 'test-chmtime -v +0', which prints the\ntruncated whole-second mtime without modifying it.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\nJunio C Hamano schrieb:\n> Care to reroll with the simpler, dumber but clearer version?\n\nOK. This time I use an intermediate file and dquotes around $t.\n\n-- Hannes\n\n t/t7701-repack-unpack-unreachable.sh |   14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex 63a8225..5babdf2 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -50,12 +50,10 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '\n\n compare_mtimes ()\n {\n-\tperl -e 'my $reference = shift;\n-\t\t foreach my $file (@ARGV) {\n-\t\t\texit(1) unless(-f $file && -M $file == -M $reference);\n-\t\t }\n-\t\t exit(0);\n-\t\t' -- \"$@\"\n+\tread tref rest &&\n+\twhile read t rest; do\n+\t\ttest \"$tref\" = \"$t\" || break\n+\tdone\n }\n\n test_expect_success '-A without -d option leaves unreachable objects packed' '\n@@ -87,7 +85,9 @@ test_expect_success 'unpacked objects receive timestamp of pack file' '\n \ttmppack=\".git/objects/pack/tmp_pack\" &&\n \tln \"$packfile\" \"$tmppack\" &&\n \tgit repack -A -l -d &&\n-\tcompare_mtimes \"$tmppack\" \"$fsha1path\" \"$csha1path\" \"$tsha1path\"\n+\ttest-chmtime -v +0 \"$tmppack\" \"$fsha1path\" \"$csha1path\" \"$tsha1path\" \\\n+\t\t> mtimes &&\n+\tcompare_mtimes < mtimes\n '\n\n test_done\n-- \n1.6.1.1.1203.g5882\n"},{"id":"102304","messageId":"alpine.DEB.1.00.0901281341330.3586@pacific.mpi-cbg.de","threadId":"17389","inReplyTo":"20090128042814.GA18216@coredump.intra.peff.net","subject":"Re: [PATCH] Windows: Fix intermittent failures of t7701","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-28T12:43:04Z","receivedAt":"2009-01-28T12:43:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Jan 2009, Jeff King wrote:\n\n> On Tue, Jan 27, 2009 at 05:42:03PM +0100, Johannes Schindelin wrote:\n> \n> > > We want to catch failures of test-chmtime; but since it appears in a \n> > > pipe, we cannot access its exit code. Therefore, we at least make sure \n> > > that it prints time stamps of all files that are passed on its command \n> > > line.\n> > \n> > I use this trick in my valgrind series:\n> > \n> > \t($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&\n> > \ttest 0 = \"$(cat exit.code)\"\n> \n> Oh, that's far too readable. How about:\n> \n>   exec 3>&1\n>   status=$( ( ($PROGRAM ; echo $? >&4) | $OTHER_PROGRAM >&3) 4>&1 )\n>   exec 3>&-\n> \n> But seriously, I think if we are talking about tests, then\n> \n>   $PROGRAM >output &&\n>   $OTHER_PROGRAM <output\n> \n> is very clear to read, and as a bonus makes \"output\" accessible for\n> viewing when the test breaks.\n\nThe real problem is that in my case, OTHER_PROGRAM=tee.\n\nBut you're right, Hannes was talking about tests, where it might \neven make sense to have a record of what was passed between the \ntwo programs.\n\nCiao,\nDscho\n"}]}