From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Peter Baumann <waste.manager@gmx.de>,
Adam Borowski <kilobyte@angband.pl>,
git@vger.kernel.org
Subject: Re: git-bisect working only from toplevel dir
Date: Tue, 29 Nov 2011 07:06:12 -0500 [thread overview]
Message-ID: <20111129120612.GA30456@sigill.intra.peff.net> (raw)
In-Reply-To: <7vd3chage9.fsf@alter.siamese.dyndns.org>
On Thu, Nov 24, 2011 at 03:50:22AM -0800, Junio C Hamano wrote:
> > On Wed, Nov 23, 2011 at 12:45:18PM -0800, Junio C Hamano wrote:
> > ...
> >> Also didn't we make bisect workable in a bare repository recently? So the
> >> start-up sequence has to be something more elaborate like...
> >> ...
> >> and then inside bisect_next() you would check if $prefix exists, and go
> >> there to run bisect--helper (or fail to go there and say "cannot test").
> >
> > But is the "cannot test" aka exit(127) the best we can do in this case?
>
> Yeah, thinking about it a bit more, it may probably be better to make it a
> failure. The user explicitly asked "be in _this_ directory and run make;
> it should succeed for the bisection test to pass". If the bisection test
> criterion the user was interested in was a successful build of the whole
> project (not the subpart of the current directory), the user would have
> gone up to the top-level and "bisect run make" there.
There are more possibilities than that. For example, imagine a project
with two sibling directories, one a library and one a command that is
built on the library. The library has a bug that we want to bisect, but
the command is the only mechanism we have to test the bug. The command's
Makefile points to the library directory (e.g., using recursive
make[1]). It would be natural for the user to do something like:
cd cmd
make && ./test-cmd
: hmph, it's broken
git bisect start
git bisect bad
: I think v1.1 was OK
git checkout v1.1
make && ./test-cmd
: Yep, let's run.
git bisect good
git bisect run 'make && ./test-cmd'
If, somewhere in the middle, the current directory doesn't exist, then
our test harness does not exist. And we can't say good or bad, but only
"don't know". Not knowing all of the details of what the user's command
does, that seems to me to be the only safe option.
The worst case is that the bisection takes longer to run and says "I
don't know where the bug is, but it's in this range", and the user has
to go back and run it again with a smarter test. But if we return "no,
the test failed" then we are likely going to just produce nonsensical
results, as our search is hitting on two different errors, and the "bug"
will appear to come and go.
It might be tempting to say that this case can't come up. After all, at
the branch tip the bug is there, and in v1.1 it isn't. What is the
chance that the test harness goes away in the middle? In a linear
history, not hight. But if you have history like this:
D--*--*--*
/ \
*--*--A--B--C---*--E
where:
- A introduces the "cmd" directory
- B is v1.1 (known good)
- C is the location of the actual bug
- D is on a side branch, but does _not_ have "cmd"
- E is our current tip (known bad)
then we will have to search down the side branch towards D to look for
the bug.
If this seems contrived, well, it is. In 99% of cases, the directory
_won't_ go away, and none of this will matter. And of course you can
have this exact same problem even without the directory issue. If your
test command is "make && ./test-harness", and the side branch doesn't
have the harness, then it's going to erroneously report the presence of
the bug. But that's your fault for writing a crappy test command that
wasn't careful about verifying the pre-conditions.
So maybe it doesn't matter; there are a lot of ways to shoot yourself in
the foot with a bisection. I just think git should set a good example
and default to being conservative with its claims.
-Peff
prev parent reply other threads:[~2011-11-29 12:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-23 14:50 git-bisect working only from toplevel dir Adam Borowski
2011-11-23 19:09 ` Junio C Hamano
2011-11-23 19:23 ` Jeff King
2011-11-23 20:09 ` Adam Borowski
2011-11-23 21:45 ` Jeff King
2011-11-23 20:26 ` Peter Baumann
2011-11-23 21:36 ` Jeff King
2011-11-23 20:45 ` Junio C Hamano
2011-11-24 7:06 ` Peter Baumann
2011-11-24 11:50 ` Junio C Hamano
2011-11-29 12:06 ` Jeff King [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111129120612.GA30456@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kilobyte@angband.pl \
--cc=waste.manager@gmx.de \
/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.
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 NNTP newsgroup(s).