On Sun, Apr 11, 2021 at 10:31 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Miriam Rubio <mirucam@xxxxxxxxx> writes:
> > + temporary_stdout_fd = open(git_path_bisect_run(), O_CREAT | O_WRONLY | O_TRUNC, 0666);
> > + saved_stdout = dup(1);
> > + dup2(temporary_stdout_fd, 1);
> > +
> > + res = bisect_state(terms, args.v, args.nr);
> > +
> > + dup2(saved_stdout, 1);
> > + close(saved_stdout);
> > + close(temporary_stdout_fd);
>
> In v2, we just let bisect_state() to write to our standard output,
> which was the reason why the loss of "cat" in the "write to
> BISECT_RUN file and cat it to show to the user" in the scripted
> version in v2 was OK. Now, we are writing out, just like the
> scripted version did, to BISECT_RUN, the user does not see its
> contents.
>
> Does the distinction matter? Christian, what's your call on this?
Sorry for the late answer. I think the content of the BISECT_RUN file
should indeed be shown.
bisect_state() calls bisect_auto_next() which calls bisect_next()
which calls bisect_next_all(), and bisect_next_all() uses printf() to
show things like "XXX is the first bad commit" which should be shown
when using `git bisect run`.
Also when adding an "exit 1 &&" before "git bisect reset" at the end
of the `"git bisect run" simple case` test, with 'next' I get:
-----------------
$ ./t6030-bisect-porcelain.sh -i -v
...
expecting success of 6030.21 '"git bisect run" simple case':
write_script test_script.sh <<-\EOF &&
! grep Another hello >/dev/null
EOF
git bisect start &&
git bisect good $HASH1 &&
git bisect bad $HASH4 &&
git bisect run ./test_script.sh >my_bisect_log.txt &&
grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
exit 1 &&
git bisect reset
Bisecting: 0 revisions left to test after this (roughly 1 step)
[3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for
git> into <hello>.
3de952f2416b6084f557ec417709eac740c6818c is the first bad commit
FATAL: Unexpected exit with code 1
-----------------
and:
-----------------
$ cat trash\ directory.t6030-bisect-porcelain/.git/BISECT_RUN
3de952f2416b6084f557ec417709eac740c6818c is the first bad commit
commit 3de952f2416b6084f557ec417709eac740c6818c
Author: A U Thor <author@xxxxxxxxxxx>
Date: Thu Apr 7 15:15:13 2005 -0700
Add <3: Another new day for git> into <hello>.
hello | 1 +
1 file changed, 1 insertion(+)
-----------------
while with 'seen' I get:
-----------------
$ ./t6030-bisect-porcelain.sh -i -v
...
expecting success of 6030.21 '"git bisect run" simple case':
write_script test_script.sh <<-\EOF &&
! grep Another hello >/dev/null
EOF
git bisect start &&
git bisect good $HASH1 &&
git bisect bad $HASH4 &&
git bisect run ./test_script.sh >my_bisect_log.txt &&
grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
exit 1 &&
git bisect reset
Bisecting: 0 revisions left to test after this (roughly 1 step)
[3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for
git> into <hello>.
error: bisect run failed:'git bisect--helper --bisect-state good'
exited with error code -10
running './test_script.sh'running
'./test_script.sh'3de952f2416b6084f557ec417709eac740c6818c is the
first bad commit
FATAL: Unexpected exit with code 1
-----------------
and:
-----------------
$ cat trash\ directory.t6030-bisect-porcelain/.git/BISECT_RUN
-----------------
So it looks like there might be another issue with what's in 'seen'.
> If it does not matter, then the code and tests are good as-is, but
> if it does, the fact that you posted this round without noticing any
> broken tests means that we have a gap in the test coverage. Can we
> have a new test about this (i.e. at the end of each round in "bisect
> run", the output from bisect_state() is shown to the user)?
Definitely it seems that taking a look at the tests is a good idea.
For example, comparing the verbose (with -v) output of t6030 before
and after each patch (and before and after this patch series) might
help find issues.
