git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Christian Couder <christian.couder@gmail.com>,
	Ramkumar Ramachandra <r@artagnon.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git List <git@vger.kernel.org>, "Miriam R." <mirucam@gmail.com>
Subject: Re: git bisect bad @
Date: Thu, 13 Jan 2022 14:55:53 +0100	[thread overview]
Message-ID: <8bc73981-589e-20e5-247b-2f74e166ae1a@web.de> (raw)
In-Reply-To: <CAP8UFD3tyBhrOQzg9j4qDAT0Tb8TCTK0=J6ORsiLVuMWn+W9wg@mail.gmail.com>

Am 13.01.22 um 13:28 schrieb Christian Couder:
> On Thu, Jan 13, 2022 at 10:32 AM Ramkumar Ramachandra
> <r@artagnon.com> wrote:
>>
>> René Scharfe wrote:
>
>>> Reserving 126 and 127 shouldn't cause too much trouble,
>
> I don't think it's a good idea at this point to reserve the 126 and
> 127 error codes as there might be existing scripts relying on them
> to mean "bad".

Certainly possible -- people get the weirdest ideas.

> Perhaps we could introduce a new command line option, for example
> --bad-is-only-1, to specify that the only error code considered bad
> will be 1. Or perhaps a more general --bad-is=<list of ranges>, to
> be able to specify all the values and ranges that should be
> considered bad.

This would only help someone who mistyped the script name or forgot to
make it executable if they also used that option.  I can't imagine
someone planning their mistakes ahead like that.  And always using this
option would be annoying.

>>> but there's also a way to avoid it: bisect run could checkout a
>>> known-good revision first and abort if the script returns
>>> non-zero for any reason, including its non-existence.
>>
>> I can't say I'm overly enthusiastic about this trade-off. I think
>> most people would check their bisect scripts against the good
>> revision by hand before starting bisect: why introduce one
>> redundant step for users like me who tend to bump their heads,
>> because they're a bit rusty with machines?

It would slow the normal operation a bit, but reduce the time to error
and its impact significantly.

> I also don't like introducing a redundant step, unless a special
> command line option is introduced for it.

My comment regarding --is-bad applies here as well.

OK, here's another idea: We verify using a known-good commit only if
the return code of the first run of the bisect script is 126 or 127.
If we get the same value again then we report the script as broken and
leave the bisect state unchanged.  Otherwise we know it's an old school
script, register the first revision as bad and continue without further
verification steps.

>> Again, I don't know if this is a good idea, but if exit codes from
>> the shell aren't standardized, surely fork() and exec() would have
>> a better spec? So, perhaps remove the little git-bisect.sh and
>> rewrite it in C? I'd be up for this task, if we decide that this is
>> a better way to go.
>
> There has been a lot of effort, especially by Miriam (added in Cc),
> to port git-bisect.sh to C over the years.

The implementation language of git bisect is not immediately relevant
here, but that the shell is used to call the user-supplied bisect run
script is.  If we'd run it directly (without RUN_USING_SHELL) we could
distinguish error code 126/127 from execution errors.  I assume the
option is used to stay compatible with the old shell version of bisect.

René


  reply	other threads:[~2022-01-13 13:56 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-09 19:29 git bisect bad @ Ramkumar Ramachandra
2022-01-09 19:54 ` Junio C Hamano
2022-01-09 20:48   ` Ramkumar Ramachandra
2022-01-10  9:01     ` [PATCH] bisect: report actual bisect_state() argument on error René Scharfe
2022-01-10 10:04       ` Ramkumar Ramachandra
2022-01-10 17:06     ` git bisect bad @ Junio C Hamano
2022-01-10 21:04       ` Ramkumar Ramachandra
2022-01-12  9:04         ` René Scharfe
2022-01-12 17:50           ` Junio C Hamano
2022-01-12 18:34             ` René Scharfe
2022-01-13  5:10               ` René Scharfe
2022-01-13  9:32                 ` Ramkumar Ramachandra
2022-01-13 12:28                   ` Christian Couder
2022-01-13 13:55                     ` René Scharfe [this message]
2022-01-13 15:16                       ` Ramkumar Ramachandra
2022-01-14  7:47                         ` René Scharfe
2022-01-14  8:04                           ` Ramkumar Ramachandra
2022-01-18 12:45                             ` René Scharfe
2022-01-14 18:42                           ` Junio C Hamano
2022-01-13 18:40                       ` Junio C Hamano
2022-01-18 12:45     ` [PATCH v2 1/4] bisect--helper: report actual bisect_state() argument on error René Scharfe
2022-01-18 12:46     ` [PATCH v2 2/4] bisect--helper: release strbuf and strvec on run error René Scharfe
2022-01-18 12:46     ` [PATCH v2 3/4] bisect: document run behavior with exit codes 126 and 127 René Scharfe
2022-01-18 12:46     ` [PATCH v2 4/4] bisect--helper: double-check run command on exit code " René Scharfe
2022-01-19  2:36       ` Junio C Hamano
2022-01-19  7:52         ` René Scharfe
2022-02-04  0:42       ` Junio C Hamano
2022-02-04 17:16         ` René Scharfe
2022-02-04 18:16         ` Ramkumar Ramachandra
2022-02-04 19:32           ` Junio C Hamano
2022-02-04 18:09       ` Ramkumar Ramachandra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8bc73981-589e-20e5-247b-2f74e166ae1a@web.de \
    --to=l.s.r@web.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mirucam@gmail.com \
    --cc=r@artagnon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).