Re: [PATCH] bisect: allow to run from subdirectories

From: Jeff King <peff@peff.net>
Date: Jun 29, 2021, 02:00 UTC
Message-ID: <YNp+oYKd5SSyxMk9@coredump.intra.peff.net>
In-Reply-To: <xmqqtulh31nm.fsf@gitster.g>

On Mon, Jun 28, 2021 at 06:22:37PM -0700, Junio C Hamano wrote:

Show 28 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >> This does not depend on "do we have T as a directory?" being the
> >> bisection criteria.  The important thing is that the current
> >> directory would appear and disappear as the bisection process makes
> >> you jump around in the history.
> >
> > I think that is a good explanation. But I remain somewhat unconvinced
> > that it is that big a problem in practice.
> 
> It's just the difference in attitude, I would think.  Things like
> "rebase" take a more liberal attitude and most of the time things
> work out OK because removal of a directory is a rare event and
> replacement of a directory with a non-directory is even rarer, but
> when things break there is no provision to help users to know how it
> broke by diagnosing why the revision cannot be checked out, or why
> the directory D the user's shell session is sitting in is now
> orphaned and different from the directory D the user thinks he is in
> because it was removed (while the user's process is in there) and
> then recreated under the same name, or any of the tricky things.
> 
> The ideal endgame would be to allow operating from subdirectory
> *AND* have provisions for helping users when things go wrong because
> the starting subdirectory goes away.  "bisect" works under the more
> conservative philosophy (start strict and forbid operation that we
> know we didn't spend any effort to avoid taking the user into
> dangerous waters---we can allow it later once we make it safer but
> not until then).

Yes, I agree with this second paragraph. Just trying to create a constructive path forward, I think I'd be comfortable with a patch series that:

  - confirmed that bisect's behavior when checkout fails produces a
    reasonable error message that the user can act on (either from
    checkout itself, or perhaps extra advice from bisect when the
    checkout fails)

  - detected the case when the prefix we started from goes away as part
    of the checkout, and turned that into an error (rather than
    orphaning the user's cwd and leading to confusing results). This
    _might_ even be something that regular "git checkout" would benefit
    from, too. And I think should not be too expensive to implement (at
    least not after an admittedly moderate amount of thinking on it).

  - only then turn on SUBDIRECTORY_OK.

-Peff