{"thread":{"id":"18040","subject":"[PATCH 1/2] bisect: fix quoting TRIED revs when \"bad\" commit is also \"skip\"ped","startedAt":"2009-02-27T06:31:22Z","lastAt":"2009-02-27T06:55:48Z","messageCount":2,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"106397","messageId":"20090227073122.ca74c387.chriscool@tuxfamily.org","threadId":"18040","inReplyTo":null,"subject":"[PATCH 1/2] bisect: fix quoting TRIED revs when \"bad\" commit is also \"skip\"ped","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-02-27T06:31:22Z","receivedAt":"2009-02-27T06:31:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When the \"bad\" commit was also \"skip\"ped and when more than one\ncommit was skipped, the \"filter_skipped\" function would have\nprinted something like:\n\nbisect_rev=<hash1>|<hash2>\n\n(where <hash1> and <hash2> are hexadecimal sha1 hashes)\n\nand this would have been evaled later as piping \"bisect_rev=<hash1>\"\ninto \"<hash2>\", which would have failed.\n\nSo this patch makes the \"filter_skipped\" function properly quote\nwhat it outputs, so that it will print something like:\n\nbisect_rev='<hash1>|<hash2>'\n\nwhich will be properly evaled later.\n\nA test case is added to the test suite.\n\nAnd while at it, we also initialize the VARS, FOUND and TRIED\nvariables, so that we protect ourselves from environment variables\nthe user may have with these names.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-bisect.sh               |    6 ++++--\n t/t6030-bisect-porcelain.sh |   25 +++++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 85db4ba..a9324b2 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -288,6 +288,8 @@ filter_skipped() {\n \t\treturn\n \tfi\n \n+\tVARS= FOUND= TRIED=\n+\n \t# Let's parse the output of:\n \t# \"git rev-list --bisect-vars --bisect-all ...\"\n \teval \"$_eval\" | while read hash line\n@@ -309,7 +311,7 @@ filter_skipped() {\n \t\t\t# This should happen only if the \"bad\"\n \t\t\t# commit is also a \"skip\" commit.\n \t\t\t,,*,bisect_rev*)\n-\t\t\t\techo \"bisect_rev=$TRIED\"\n+\t\t\t\techo \"bisect_rev='$TRIED'\"\n \t\t\t\tVARS=1\n \t\t\t\t;;\n \n@@ -320,7 +322,7 @@ filter_skipped() {\n \t\t\t\t*$hash*) ;;\n \t\t\t\t*)\n \t\t\t\t\techo \"bisect_rev=$hash\"\n-\t\t\t\t\techo \"bisect_tried=\\\"$TRIED\\\"\"\n+\t\t\t\t\techo \"bisect_tried='$TRIED'\"\n \t\t\t\t\tFOUND=1\n \t\t\t\t\t;;\n \t\t\t\tesac\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex dd7eac8..052a6c9 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -224,6 +224,31 @@ test_expect_success 'bisect skip: cannot tell between 2 commits' '\n \tfi\n '\n \n+# $HASH1 is good, $HASH4 is both skipped and bad, we skip $HASH3\n+# and $HASH2 is good,\n+# so we should not be able to tell the first bad commit\n+# among $HASH3 and $HASH4\n+test_expect_success 'bisect skip: with commit both bad and skipped' '\n+\tgit bisect start &&\n+\tgit bisect skip &&\n+\tgit bisect bad &&\n+\tgit bisect good $HASH1 &&\n+\tgit bisect skip &&\n+\tif git bisect good > my_bisect_log.txt\n+\tthen\n+\t\techo Oops, should have failed.\n+\t\tfalse\n+\telse\n+\t\ttest $? -eq 2 &&\n+\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t\t! grep $HASH1 my_bisect_log.txt &&\n+\t\t! grep $HASH2 my_bisect_log.txt &&\n+\t\tgrep $HASH3 my_bisect_log.txt &&\n+\t\tgrep $HASH4 my_bisect_log.txt &&\n+\t\tgit bisect reset\n+\tfi\n+'\n+\n # We want to automatically find the commit that\n # introduced \"Another\" into hello.\n test_expect_success \\\n-- \n1.6.2.rc1.257.g6c75\n"},{"id":"106401","messageId":"7v8wnsnzbf.fsf@gitster.siamese.dyndns.org","threadId":"18040","inReplyTo":"20090227073122.ca74c387.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/2] bisect: fix quoting TRIED revs when \"bad\" commit is also \"skip\"ped","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-27T06:55:48Z","receivedAt":"2009-02-27T06:55:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 85db4ba..a9324b2 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -288,6 +288,8 @@ filter_skipped() {\n>  \t\treturn\n>  \tfi\n>  \n> +\tVARS= FOUND= TRIED=\n> +\n\nDid you have any particular reason to move these out of the subshell on\nthe downstream side of the pipe as I wrote in my response?  That is where\nthese variables matter.\n\n>  \t# Let's parse the output of:\n>  \t# \"git rev-list --bisect-vars --bisect-all ...\"\n>  \teval \"$_eval\" | while read hash line\n> @@ -309,7 +311,7 @@ filter_skipped() {\n\nAlso you seem to have lost my fix to ignore $line for \"bisect_foo=value\"\nlines where $line is always empty.\n"}]}