{"thread":{"id":"52613","subject":"[RFC] Proposal: New options for git bisect run to control the interpretation of exit codes","startedAt":"2020-01-11T01:51:44Z","lastAt":"2020-03-28T19:14:54Z","messageCount":4,"participants":["Stephen Oberholtzer","Christian Couder","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"389604","messageId":"CAD_xR9f7jHnCByOaOVJvxdW2c5dPHM8OUDwZhcPL1iTVR3NzmQ@mail.gmail.com","threadId":"52613","inReplyTo":null,"subject":"[RFC] Proposal: New options for git bisect run to control the interpretation of exit codes","fromName":"Stephen Oberholtzer","fromEmail":"stevie@qrpff.net","sentAt":"2020-01-11T01:42:35Z","receivedAt":"2020-01-11T01:51:44Z","isPatch":false,"sender":{"key":"stevie@qrpff.net","avatar":null},"body":"After considering the responses I got for my --invert-status proposal, I\nwent back to the drawing board and considered how it might interact with\nanother feature I was going to propose.  This is the result.\n\nTo avoid repeating myself, I'm going to start with an example of what I\nimagine putting into Documentation/git-bisect.txt, to document this\nfeature.  After the second snip line, you can find my justification for\nthis feature, as well as some preemptive answers to questions I expect\npeople to have.\n\n>8----------------------------------------------------------------------8<\n\n{at top}\n\ngit bisect run [--(old|good|<term-old>)-status=<list>]\n[--(new|bad|<term-new>)-status=<list>]\n[--skip-status=<list>] [--] <cmd>...\n\n{below the paragraph beginning with \"The special exit code 125\")\n\nCustom exit status mappings\n^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\nThe previous paragraphs describe how the exit status of <cmd> is mapped\nto one of four actions:\n\n * `old`/`good`, which defaults to exit status 0.\n\n * `new`/`bad`, which defaults to exit status 1-127 except 125.\n\n * `skip`, which defaults to exit status 125.\n\n * Any status not mapped to one of the first three is treated as a fatal\n   error, causing `git bisect run` to immediately terminate with a\n   nonzero exit status.\n\nFor more precise control over bisect run's interpretation of the command's\nexit status, you can use one or more `--<term>-status=<list>` options:\n\n<term>::\n  One of the following:\n  * `skip` (`--skip-status`)\n  * `old` or the corresponding term specified in the bisect start call\n     (`--old-status` or `--good-status`)\n  * `new` or the corresponding term specified in the bisect start call\n     (`--new-status` or `--bad-status`)\n<list>::\nA comma-separated list of values (e.g. 1) or ranges (e.g. 1-10).\n\n(It should be noted that specifying --old-status is unlikely to be useful\nwithout a corresponding --new-status option.)\n\nThis feature can make a few things much easier:\n\n * If you want to bisect a \"fix\", you can use (as an example)\n\n---------------\n$ git bisect run --old-status=1-127 --new-status=0 my_script arguments\n---------------\n\n * If the test involves some fundamentally-unreliable aspects such as I/O\n  (for example, a network connection could be disrupted, or a file write\n   could fail due to disk space issues), you can specify something like\n   --new-status=99, and then any exit status other than 0 or 99 will be\n   treated as a fatal error:\n\n   -----------------\n   $ git bisect run --new-status=99 sh -c 'setup-test && (run-test || exit 99)'\n   -----------------\n\n   If setup-test exits with a nonzero status code (except 99), then the\n   run will abort with an error, rather than assume that the current commit\n   contains the issue being bisected.\n\n\nThe default status code lists are as follows:\n  --skip-status=125\n  --old-status=0\n  --new-status=1-127\n\nThe priority of each classification is as follows:\n\n * If the status code is on the --skip-status list, the action is \"skip\".\n   Note that this takes precedence over the --old-status and --new-status\n   lists; doing so simplifies the specification of --new-status.\n\n * If the status is on the --old-status list, *and* not on the --new-status\n   list, the action is \"old\" (or \"good\").\n\n * If the status is on the --new-status list, *and* not on the --old-status\n   list, the action is \"new\" (or \"bad\").\n\n * Otherwise, the command is treated as having experienced a fatal error,\n   and run will terminate with a nonzero exit status.\n\nIn the last case, the bisection will be left where it was; you may correct\nthe error and perform another bisect run, or you may finish the bisection\nwith manual good/bad commands.\n\n>8----------------------------------------------------------------------8<\n\nThe motivation\n==============\n\nFirst, an excerpt from the current documentation for 'git bisect':\n\n> 126 and 127 are used by POSIX shells to signal specific error status\n> (127 is for command not found, 126 is for command found but not executable\n> - these details do not matter, as they are normal errors in the script,\n> as far as bisect run is concerned\n\nThis shows a fundamental disconnect between bisect run's view of the\nworld and reality.  I submit that, in reality, status codes 126 and 127 are\noverwhelmingly likely indicators that the script did not work correctly,\nin which case the run should be halted so the user may correct the issue.\nHowever, 126 and 127 are mapped to \"git bisect bad\" -- as in, \"this commit\ndefinitely contains the bug that I am searching for\".\n\n\nLet's consider the consequences of an inappropriate status code:\n\n- 'good':  The bisect will incorrectly select the wrong commit (specifically,\n  a later commit than the one that actually introduced the issue.)\n\n  This will also indirectly result in more trials than would otherwise be\n  necessary to determine the result, because the bisection will have to be\n  restarted at the point where the mistake occurred.  (In the worst possible\n  case, where a mistake occurs on the first step, the bisection will take\n  at least twice as long.)\n\n- 'bad': The bisect will incorrectly select the wrong commit (specifically,\n  an earlier commit than the one that actually introduced the issue.)\n\n  Like 'good', this will also indirectly result in extra trials.\n\n- 'skip': The bisect will unnecessarily test at least one extra commit for\n  each false 'skip' result.  In the worst case, it may not be able to narrow\n  down the issue to a single commit.\n\n- Abort: The bisection stops until the user restarts it.  No extra commits\n  are tested, though if the user isn't paying attention, the wallclock\n  time will take longer than usual.\n\nIn particular, the behavior of a false match on 'good' or 'bad' is *at best*\nextra time needed to do the bisection.  At worst -- if the user is not\nfamiliar with the code in question -- a great deal of confusion and time-\nwasting can result as the user investigates the wrong commit.  As such, it\nis critical that you have no false 'good' or 'bad' results.\n\n\nIf you're using a shell script to run your test, a false 'good' result can\neasily be prevented by putting 'set -e' at the top of the script.\n\nAvoiding a false 'bad' result is far more difficult, especially if the test\nis complex and you're not familiar with shell scripting in general.  (The\nman page for bash, on my machine, is 3725 lines long, and does not lend\nitself well to searching.)  For the task that set me down this path, it\ntook me about six iterations to create a robust script that would return\nthe exit code that git wanted,  and _it still didn't work right in all\ncases_.\n\n\nWhat would have been a great deal simpler is if I could have just picked\nan exit code that none of the other commands in my script would ever\nreturn (such as 99), and told git to treat any code other than 0 or 99 as\na fatal error.  Which is essentially what I ended up doing (or trying to),\nbut unfamiliar with shell scripting as I was, I had several issues.\n\nIf I can make it easy for someone to use bisect run without them having\nto learn any otherwise-unnecessary shell scripting constructs, I would\nconsider that a win.\n\n\nPre-emptive Q&A\n===============\n\nQ: Why allow ranges?  Why not restrict it to single status codes?\nA: It allows for a simpler implementation, because the default 'bad' status\n   list is a already a range (1-124,126-127).  By supporting ranges,\n   we need only one single code path:\n\n   exit_bad=1-124,126-127\n   # override exit_bad if --bad-status is specified\n   # check exit code against exit_bad\n\nQ: Why treat any exit code that's on both the 'old' and 'new' lists as a\n   fatal error?\nA: Because it's obviously a mistake, and we don't know the correct way\n   to interpret it.  By halting the run and returning an error, we can\n   return control to the user, who can fix it by fixing the overlapping\n   lists and re-running the command.\n\nQ: Why not an error when a code is on both 'old' and 'skip', or 'new' and\n   'skip'?\nA: Two reasons:\n   1. As I detailed earlier, a false 'skip' is less problematic than a\n      false 'new'/'bad'.\n   2. By prioritizing 'skip' over 'new', the default behavior for 'new'\n      can be written as '1-127' instead of '1-124,126-127'.\n\n\n-- \n-- Stevie-O\nReal programmers use COPY CON PROGRAM.EXE\n"},{"id":"389838","messageId":"CAP8UFD3dr3XdZSm88qoGDajSoFMx-TJZfxsMGqhFMKA6dEWj4A@mail.gmail.com","threadId":"52613","inReplyTo":"CAD_xR9f7jHnCByOaOVJvxdW2c5dPHM8OUDwZhcPL1iTVR3NzmQ@mail.gmail.com","subject":"Re: [RFC] Proposal: New options for git bisect run to control the interpretation of exit codes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-01-16T00:39:44Z","receivedAt":"2020-01-16T00:40:00Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Jan 11, 2020 at 2:43 AM Stephen Oberholtzer <stevie@qrpff.net> wrote:\n>\n> After considering the responses I got for my --invert-status proposal, I\n> went back to the drawing board and considered how it might interact with\n> another feature I was going to propose.  This is the result.\n>\n> To avoid repeating myself, I'm going to start with an example of what I\n> imagine putting into Documentation/git-bisect.txt, to document this\n> feature.  After the second snip line, you can find my justification for\n> this feature, as well as some preemptive answers to questions I expect\n> people to have.\n\nOk.\n\n> >8----------------------------------------------------------------------8<\n>\n> {at top}\n>\n> git bisect run [--(old|good|<term-old>)-status=<list>]\n> [--(new|bad|<term-new>)-status=<list>]\n> [--skip-status=<list>] [--] <cmd>...\n\nThat seems interesting.\n\nIn many cases I have found that something like the following would do:\n\ngit bisect run sh -c \"make || exit 125; setup_script || exit 255;\nrun_cmd >log 2>&1 || exit 255; (! grep foo log)\"\n\nbut I understand that some people may prefer options like the one you suggest.\n\n> {below the paragraph beginning with \"The special exit code 125\")\n>\n> Custom exit status mappings\n> ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n>\n> The previous paragraphs describe how the exit status of <cmd> is mapped\n> to one of four actions:\n>\n>  * `old`/`good`, which defaults to exit status 0.\n>\n>  * `new`/`bad`, which defaults to exit status 1-127 except 125.\n>\n>  * `skip`, which defaults to exit status 125.\n>\n>  * Any status not mapped to one of the first three is treated as a fatal\n>    error, causing `git bisect run` to immediately terminate with a\n>    nonzero exit status.\n>\n> For more precise control over bisect run's interpretation of the command's\n> exit status, you can use one or more `--<term>-status=<list>` options:\n>\n> <term>::\n>   One of the following:\n>   * `skip` (`--skip-status`)\n>   * `old` or the corresponding term specified in the bisect start call\n>      (`--old-status` or `--good-status`)\n>   * `new` or the corresponding term specified in the bisect start call\n>      (`--new-status` or `--bad-status`)\n> <list>::\n> A comma-separated list of values (e.g. 1) or ranges (e.g. 1-10).\n>\n> (It should be noted that specifying --old-status is unlikely to be useful\n> without a corresponding --new-status option.)\n\nThis is not clear from what has been documented above.\n\n> This feature can make a few things much easier:\n>\n>  * If you want to bisect a \"fix\", you can use (as an example)\n>\n> ---------------\n> $ git bisect run --old-status=1-127 --new-status=0 my_script arguments\n> ---------------\n\nYeah, that seems useful. It is not clear though if 125 then means\n\"old\" or still \"skip\".\n\n>  * If the test involves some fundamentally-unreliable aspects such as I/O\n>   (for example, a network connection could be disrupted, or a file write\n>    could fail due to disk space issues), you can specify something like\n>    --new-status=99, and then any exit status other than 0 or 99 will be\n>    treated as a fatal error:\n>\n>    -----------------\n>    $ git bisect run --new-status=99 sh -c 'setup-test && (run-test || exit 99)'\n>    -----------------\n>\n>    If setup-test exits with a nonzero status code (except 99), then the\n>    run will abort with an error, rather than assume that the current commit\n>    contains the issue being bisected.\n\nYeah but `sh -c 'setup-test || exit 255; run-test'` should work fine\nin this case too.\n\n> The default status code lists are as follows:\n>   --skip-status=125\n>   --old-status=0\n>   --new-status=1-127\n>\n> The priority of each classification is as follows:\n>\n>  * If the status code is on the --skip-status list, the action is \"skip\".\n>    Note that this takes precedence over the --old-status and --new-status\n>    lists; doing so simplifies the specification of --new-status.\n>\n>  * If the status is on the --old-status list, *and* not on the --new-status\n>    list, the action is \"old\" (or \"good\").\n>\n>  * If the status is on the --new-status list, *and* not on the --old-status\n>    list, the action is \"new\" (or \"bad\").\n\nThe above doesn't tell what happens if a status is both on the\n--old-status and on the --new-status lists...\n\n>  * Otherwise, the command is treated as having experienced a fatal error,\n>    and run will terminate with a nonzero exit status.\n\n...so it seems that it is an error only when such a status code is\nactually received by `git bisect run`.\n\nSome people could argue though that `--new-status=0 --old-status=0`\nshould be detected as an error as soon as `git bisect run` is\nlaunched.\n\n> In the last case, the bisection will be left where it was; you may correct\n> the error and perform another bisect run, or you may finish the bisection\n> with manual good/bad commands.\n\nIt would be nice if the above clarification about priority was before\nthe examples you gave.\n\n> >8----------------------------------------------------------------------8<\n>\n> The motivation\n> ==============\n>\n> First, an excerpt from the current documentation for 'git bisect':\n>\n> > 126 and 127 are used by POSIX shells to signal specific error status\n> > (127 is for command not found, 126 is for command found but not executable\n> > - these details do not matter, as they are normal errors in the script,\n> > as far as bisect run is concerned\n>\n> This shows a fundamental disconnect between bisect run's view of the\n> world and reality.  I submit that, in reality, status codes 126 and 127 are\n> overwhelmingly likely indicators that the script did not work correctly,\n> in which case the run should be halted so the user may correct the issue.\n\nWell when bisect run was implemented the main concern was to make it\nabort immediately in case of a signal and to consider common non zero\nstatus code as \"bad\". A signal would result in an status code greater\nthan 128, so it was decided to split the [1-255] range in two more or\nless equal parts, so [1-128] would be \"bad\" and [129-255] would be\n\"abort\". Then \"split\" was added and given special code 125.\n\n> However, 126 and 127 are mapped to \"git bisect bad\" -- as in, \"this commit\n> definitely contains the bug that I am searching for\".\n\nYeah, it might have been better to use [126-255] for \"abort\".\n\nI think though that 126 and 127 should still be rare. And git bisect\nmight be used to debug software written in shell (Git itself had many\ncommands written in shell a long time ago) where 126 or 127 could come\nfrom errors in the software itself not from the testing scripts.\n\n> Let's consider the consequences of an inappropriate status code:\n>\n> - 'good':  The bisect will incorrectly select the wrong commit (specifically,\n>   a later commit than the one that actually introduced the issue.)\n>\n>   This will also indirectly result in more trials than would otherwise be\n>   necessary to determine the result, because the bisection will have to be\n>   restarted at the point where the mistake occurred.  (In the worst possible\n>   case, where a mistake occurs on the first step, the bisection will take\n>   at least twice as long.)\n>\n> - 'bad': The bisect will incorrectly select the wrong commit (specifically,\n>   an earlier commit than the one that actually introduced the issue.)\n>\n>   Like 'good', this will also indirectly result in extra trials.\n>\n> - 'skip': The bisect will unnecessarily test at least one extra commit for\n>   each false 'skip' result.  In the worst case, it may not be able to narrow\n>   down the issue to a single commit.\n>\n> - Abort: The bisection stops until the user restarts it.  No extra commits\n>   are tested, though if the user isn't paying attention, the wallclock\n>   time will take longer than usual.\n>\n> In particular, the behavior of a false match on 'good' or 'bad' is *at best*\n> extra time needed to do the bisection.  At worst -- if the user is not\n> familiar with the code in question -- a great deal of confusion and time-\n> wasting can result as the user investigates the wrong commit.  As such, it\n> is critical that you have no false 'good' or 'bad' results.\n\nI agree with that.\n\n> If you're using a shell script to run your test, a false 'good' result can\n> easily be prevented by putting 'set -e' at the top of the script.\n\nSometimes it is easier to understand a script when it properly checks\nfor errors and produces good error messages than when it uses `set\n-e`.\n\n> Avoiding a false 'bad' result is far more difficult, especially if the test\n> is complex and you're not familiar with shell scripting in general.  (The\n> man page for bash, on my machine, is 3725 lines long, and does not lend\n> itself well to searching.)  For the task that set me down this path, it\n> took me about six iterations to create a robust script that would return\n> the exit code that git wanted,  and _it still didn't work right in all\n> cases_.\n\nI agree that it is not always easy.\n\n> What would have been a great deal simpler is if I could have just picked\n> an exit code that none of the other commands in my script would ever\n> return (such as 99), and told git to treat any code other than 0 or 99 as\n> a fatal error.  Which is essentially what I ended up doing (or trying to),\n> but unfamiliar with shell scripting as I was, I had several issues.\n\nI think there might have been easier ways.\n\nSee above the alternative I suggested to `git bisect run\n--new-status=99 sh -c 'setup-test && (run-test || exit 99)'`.\n\n> If I can make it easy for someone to use bisect run without them having\n> to learn any otherwise-unnecessary shell scripting constructs, I would\n> consider that a win.\n\nI agree that in some cases the options you suggest could seem simpler\nfor some people.\n\n> Pre-emptive Q&A\n> ===============\n\nThanks for this!\n\nBest,\nChristian.\n"},{"id":"389852","messageId":"CAD_xR9cC_MrrqGoHuBsfuVAQ=qVMd0rjXvp22+8ed-D=4TQRbA@mail.gmail.com","threadId":"52613","inReplyTo":"CAP8UFD3dr3XdZSm88qoGDajSoFMx-TJZfxsMGqhFMKA6dEWj4A@mail.gmail.com","subject":"Re: [RFC] Proposal: New options for git bisect run to control the interpretation of exit codes","fromName":"Stephen Oberholtzer","fromEmail":"stevie@qrpff.net","sentAt":"2020-01-16T02:40:54Z","receivedAt":"2020-01-16T02:41:16Z","isPatch":false,"sender":{"key":"stevie@qrpff.net","avatar":null},"body":"On Wed, Jan 15, 2020 at 7:39 PM Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> On Sat, Jan 11, 2020 at 2:43 AM Stephen Oberholtzer <stevie@qrpff.net> wrote:\n> >\n> In many cases I have found that something like the following would do:\n>\n> git bisect run sh -c \"make || exit 125; setup_script || exit 255;\n> run_cmd >log 2>&1 || exit 255; (! grep foo log)\"\n>\n> but I understand that some people may prefer options like the one you suggest.\n\nYeah,  A lot of this stuff can be easily done if you're a skilled\nshell scripter.  But it's easier to do it _wrong_, especially for\nsomething non-trivial.\n\n> >\n> > For more precise control over bisect run's interpretation of the command's\n> > exit status, you can use one or more `--<term>-status=<list>` options:\n> >\n> > <term>::\n> >   One of the following:\n> >   * `skip` (`--skip-status`)\n> >   * `old` or the corresponding term specified in the bisect start call\n> >      (`--old-status` or `--good-status`)\n> >   * `new` or the corresponding term specified in the bisect start call\n> >      (`--new-status` or `--bad-status`)\n> > <list>::\n> > A comma-separated list of values (e.g. 1) or ranges (e.g. 1-10).\n> >\n> > (It should be noted that specifying --old-status is unlikely to be useful\n> > without a corresponding --new-status option.)\n>\n> This is not clear from what has been documented above.\n\nI'm not sure what you're referring to here, especially regarding \"what\nhas been documented above\".\nAll of that stuff is still in my documentation notes.\n\n>\n> > This feature can make a few things much easier:\n> >\n> >  * If you want to bisect a \"fix\", you can use (as an example)\n> >\n> > ---------------\n> > $ git bisect run --old-status=1-127 --new-status=0 my_script arguments\n> > ---------------\n>\n> Yeah, that seems useful. It is not clear though if 125 then means\n> \"old\" or still \"skip\".\n\nWell, it is clear if you read the entire thing.  But as you pointed\nout below, I ought to reorder the text a bit.\n\n>\n> >    -----------------\n> >    $ git bisect run --new-status=99 sh -c 'setup-test && (run-test || exit 99)'\n> >    -----------------\n> >\n>\n> Yeah but `sh -c 'setup-test || exit 255; run-test'` should work fine\n> in this case too.\n\nYep, and it only works because there's a flaw in my example: it\nassumes that all nonzero exit codes from run-test are conclusive\nresults.  But if run-test died from a signal (say, SEGV or INT or an\noom KILL) that overly-simplistic || is going to turn all of them into\n99s.  If we were watching for a SEGV or INT or KILL, that's awesome.\nBut if we were looking for something else, then we've misclassified\nthe result!\n\nThat's one of the major problems with || -- it boils down the\nprocess's exit code to a simple boolean.\n\nI'm going to rewrite my example to say \"if run-test exits with code 99\nif the defect is present\" and just do setup-test && run--test.\n\n>\n> The above doesn't tell what happens if a status is both on the\n> --old-status and on the --new-status lists...\n\nNo, the below does.\n\n>\n> >  * Otherwise, the command is treated as having experienced a fatal error,\n> >    and run will terminate with a nonzero exit status.\n>\n> ...so it seems that it is an error only when such a status code is\n> actually received by `git bisect run`.\n>\n> Some people could argue though that `--new-status=0 --old-status=0`\n> should be detected as an error as soon as `git bisect run` is\n> launched.\n\nThere are a few reasons for not raising an error immediately:\n- Such a check would not be free.  While the example you gave is\nsimple enough, things can quickly get complicated.  A generalized\nversion would have to check every single status from 0 to 255\n  (that said, I can see some value in proactively checking 0 and 1\nbefore starting the run, just as a sanity check)\n-  If there _is_ an ambiguous exit code, it doesn't actually matter\nunless it's actually encountered\n- If the user makes such a mistake, the bisection doesn't go off the\nrails; it just halts.  A simple 'git bisect good' or 'git bisect bad',\nfollowed by another call to \"run\" with corrected options, will address\nthe issue.\n\n>\n> It would be nice if the above clarification about priority was before\n> the examples you gave.\n\nYeah, I'll reorder things.\n\n>\n> Well when bisect run was implemented the main concern was to make it\n> abort immediately in case of a signal and to consider common non zero\n> status code as \"bad\". A signal would result in an status code greater\n> than 128, so it was decided to split the [1-255] range in two more or\n> less equal parts, so [1-128] would be \"bad\" and [129-255] would be\n> \"abort\". Then \"split\" was added and given special code 125.\n\nOh, yes, I understand the reasoning behind the current behavior; that\ndoesn't change the fact that trying to work around it can be awkward.\n\n>\n> > However, 126 and 127 are mapped to \"git bisect bad\" -- as in, \"this commit\n> > definitely contains the bug that I am searching for\".\n>\n> Yeah, it might have been better to use [126-255] for \"abort\".\n>\n> I think though that 126 and 127 should still be rare. And git bisect\n> might be used to debug software written in shell (Git itself had many\n> commands written in shell a long time ago) where 126 or 127 could come\n> from errors in the software itself not from the testing scripts.\n\n\"rare\" doesn't mean \"impossible\" -- and in fact, the 'command not\nfound' exit code was tripped on one of my early runs when an error in\nmy test script left it in the wrong directory.\n\nBut you make an excellent point that seems to support my proposal: If\nyou're bisecting a shell script issue, you probably want to treat\n126-127 as signifying \"bad\" commits, but if you're not, you probably\nwant 126-127 to abort the run because it means your test itself\nmalfunctioned.\n\nThe little project I was working on that set me down this path took\n15-20 minutes per iteration, and was bisecting through over 64K\ncommits; I believe it ended up testing about 20 revisions (due to\nmerges).  A single false positive/negative, especially early on, could\nhave resulted in hours wasted.\n\n> > If you're using a shell script to run your test, a false 'good' result can\n> > easily be prevented by putting 'set -e' at the top of the script.\n>\n> Sometimes it is easier to understand a script when it properly checks\n> for errors and produces good error messages than when it uses `set\n> -e`.\n\nPerhaps, but not necessarily -- and _definitely_ not if you're not\nexperienced with shell scripting.\nShell scripting feels incredibly fragile; every bit you add is a\nchance to make a mistake.\n\nIn contrast, \"set -exuo pipefail\" makes life really easy: You can see\neverything the script does, and you can be confident that as soon as\nsomething goes wrong, your script will stop right away.\n\n> > What would have been a great deal simpler is if I could have just picked\n> > an exit code that none of the other commands in my script would ever\n> > return (such as 99), and told git to treat any code other than 0 or 99 as\n> > a fatal error.  Which is essentially what I ended up doing (or trying to),\n> > but unfamiliar with shell scripting as I was, I had several issues.\n>\n> I think there might have been easier ways.\n>\n> See above the alternative I suggested to `git bisect run\n> --new-status=99 sh -c 'setup-test && (run-test || exit 99)'`.\n\nThere's a bootstrapping issue, here: If someone's unfamiliar with\nshell scripting, how would they know how to construct such a thing?\nIf you're going to bet, say, five hours of your life on your\none-liner, how can you be confident that it will work reliably?\n\n> Thanks for this!\n\nThank you for your feedback!\n\nI've got a first draft of the proof-of-concept patch written already;\nI'll add a proactive sanity check for exit codes 0 and 1, since that's\ncheap and should cover over 99% of cases, do some cleanups to the\ndocumentation and examples, and then add some test cases.\n\n>\n> Best,\n> Christian.\n\n\n\n-- \n-- Stevie-O\nReal programmers use COPY CON PROGRAM.EXE\n"},{"id":"394249","messageId":"24c58c80-c274-6a64-051c-90329432b85d@iee.email","threadId":"52613","inReplyTo":"CAD_xR9cC_MrrqGoHuBsfuVAQ=qVMd0rjXvp22+8ed-D=4TQRbA@mail.gmail.com","subject":"Re: [RFC] Proposal: New options for git bisect run to control the interpretation of exit codes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-03-28T19:14:51Z","receivedAt":"2020-03-28T19:14:54Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Sorry this is late.\nOn 16/01/2020 02:40, Stephen Oberholtzer wrote:\n>> The above doesn't tell what happens if a status is both on the\n>> --old-status and on the --new-status lists...\n> No, the below does.\n>\n>>>  * Otherwise, the command is treated as having experienced a fatal error,\n>>>    and run will terminate with a nonzero exit status.\n>> ...so it seems that it is an error only when such a status code is\n>> actually received by `git bisect run`.\n>>\n>> Some people could argue though that `--new-status=0 --old-status=0`\n>> should be detected as an error as soon as `git bisect run` is\n>> launched.\n> There are a few reasons for not raising an error immediately:\n> - Such a check would not be free.  While the example you gave is\n> simple enough, things can quickly get complicated.  A generalized\n> version would have to check every single status from 0 to 255\n>   (that said, I can see some value in proactively checking 0 and 1\n> before starting the run, just as a sanity check)\n> -  If there _is_ an ambiguous exit code, it doesn't actually matter\n> unless it's actually encountered\n> - If the user makes such a mistake, the bisection doesn't go off the\n> rails; it just halts.  \n\nWill such a mistake be _reported_ (suitable message) to the user? This\nwould at least short circuit misunderstandings as to the reason for the\nhalting of the bisect.\n> A simple 'git bisect good' or 'git bisect bad',\n> followed by another call to \"run\" with corrected options, will address\n> the issue.\n>\nPhilip\n"}]}