git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Miriam R." <mirucam@gmail.com>
Cc: git <git@vger.kernel.org>
Subject: Re: [PATCH v7 00/13] Finish converting git bisect to C part 2
Date: Wed, 23 Sep 2020 23:23:49 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2009232316570.5061@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2009231647370.5061@tvgsbejvaqbjf.bet>

Hi Miriam,

On Wed, 23 Sep 2020, Johannes Schindelin wrote:

> On Wed, 23 Sep 2020, Miriam R. wrote:
>
> > Applying some of your suggestions related to removing some 'eval' in
> > git-bisect shell script, a bug has appeared. It seems it is related to
> > a previous code merged before my internship.
>
> Now you got me curious: what bug did you see?

I found your fork and ran the test, and this is the first symptom:

-- snip --
[...]
++ git bisect skip
Bisecting: 1 revision left to test after this (roughly 1 step)
[32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into <hello>.
++ git bisect good
++ grep '3082e11d3a0f2edca194c8ce1eb802256e38e75e is the first bad commit' my_bisect_log.txt
3082e11d3a0f2edca194c8ce1eb802256e38e75e is the first bad commit
++ git bisect log
++ git bisect reset
Previous HEAD position was 32a594a Add <4: Ciao for now> into <hello>.
Switched to branch 'other'
ok 22 - bisect skip: add line and then a new test

expecting success of 6030.23 'bisect skip and bisect replay':
        git bisect replay log_to_replay.txt > my_bisect_log.txt &&
        grep "$HASH5 is the first bad commit" my_bisect_log.txt &&
        git bisect reset

++ git bisect replay log_to_replay.txt
error: update_ref failed for ref 'refs/bisect/bad': cannot update ref 'refs/bisect/bad': trying to write ref 'refs/bisect/bad' with nonexistent object 10006d020000000068986d020000000054f65f00
error: last command exited with $?=1
not ok 23 - bisect skip and bisect replay
#
#               git bisect replay log_to_replay.txt > my_bisect_log.txt &&
#               grep "$HASH5 is the first bad commit" my_bisect_log.txt &&
#               git bisect reset
-- snap --

So I dug a little bit further (and applied Christian's patch in the
meantime), and it turns out that the `eval` has nothing to do with what I
originally thought it would be required for: I thought that it wanted to
prevent `exit` calls from actually exiting the script.

Instead, those `eval` calls are required because the arguments are
provided in quoted form. For example, during the execution of t6030.68,
the `eval` would expand the call

	eval "git bisect--helper --bisect-start $rev $tail"

to

	git bisect--helper --bisect-start '--term-old' 'term2' '--term-new' 'term1'

Therefore, the `eval` really needs to stay in place (also the other `eval`
I had originally suggested to remove, for the same reason).

I would still recommend appending `|| exit`, even if it just so happens
that we will eventually abort when the `bisect--helper` command failed
anyway, because the next command will then fail, and abort. But it's
cleaner to abort already when this invocation failed rather than relying
on that side effect.

Ciao,
Dscho


  reply	other threads:[~2020-09-23 21:24 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-31 10:50 Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 01/13] bisect--helper: BUG() in cmd_*() on invalid subcommand Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 02/13] bisect--helper: use '-res' in 'cmd_bisect__helper' return Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 03/13] bisect--helper: introduce new `write_in_file()` function Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 04/13] bisect--helper: reimplement `bisect_autostart` shell function in C Miriam Rubio
2020-09-03  5:50   ` Johannes Schindelin
2020-08-31 10:50 ` [PATCH v7 05/13] bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()' Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 06/13] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C Miriam Rubio
2020-09-03  9:48   ` Johannes Schindelin
2020-08-31 10:50 ` [PATCH v7 07/13] bisect--helper: finish porting `bisect_start()` to C Miriam Rubio
2020-09-03  9:56   ` Johannes Schindelin
2020-08-31 10:50 ` [PATCH v7 08/13] bisect--helper: retire `--bisect-clean-state` subcommand Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 09/13] bisect--helper: retire `--next-all` subcommand Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 10/13] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C Miriam Rubio
2020-09-03 12:03   ` Johannes Schindelin
2020-08-31 10:50 ` [PATCH v7 11/13] bisect--helper: retire `--check-expected-revs` subcommand Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 12/13] bisect--helper: retire `--write-terms` subcommand Miriam Rubio
2020-08-31 10:50 ` [PATCH v7 13/13] bisect--helper: retire `--bisect-autostart` subcommand Miriam Rubio
2020-09-03 12:04 ` [PATCH v7 00/13] Finish converting git bisect to C part 2 Johannes Schindelin
2020-09-23  7:26   ` Miriam R.
2020-09-23 14:48     ` Johannes Schindelin
2020-09-23 21:23       ` Johannes Schindelin [this message]
2020-09-24  5:33         ` Christian Couder
2020-09-24  7:56           ` Johannes Schindelin
2020-09-24 10:46             ` Christian Couder
2020-09-24 12:53               ` Miriam R.
2020-09-24 12:56         ` Miriam R.

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=nycvar.QRO.7.76.6.2009232316570.5061@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=mirucam@gmail.com \
    --subject='Re: [PATCH v7 00/13] Finish converting git bisect to C part 2' \
    /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

Code repositories for project(s) associated with this 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).