Re: [PATCH 2/3] bisect: fix internal diff-tree config loading

[Christian Couder <christian.couder@gmail.com>]

On Fri, Feb 22, 2019 at 7:21 AM Jeff King <peff@peff.net> wrote:
>
> When we run our internal diff-tree to show the bisected commit, we call
> init_revisions(), then load config, then setup_revisions(). But that
> order is wrong: we copy the configured defaults into the rev_info struct
> during the init_revisions step, so our config load wasn't actually doing
> anything.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> It does feel a little weird loading config at all here, since it would
> potentially affect other in-process operations.

I like that this patch fixes a bug, but this still triggers some
wondering/comments.

Would it be better or at least less weird to load it at the beginning
of `git bisect`?

Or is the real problem a limitation of the config system, that prevent
us from temporarily loading, and then maybe unloading, some config?

> This is where an
> external process might be cleaner.

It depends on which definition of clean we use. Yeah, having many
global variables and not caring because we launch many external
processes that will "clean" things up when they exit can seem "clean"
for some time. But I think we are past this point now, and I still
wouldn't like us to go back to our previous way of doing things even
here. So thanks for not using an external process.

Thanks for working on this,
Christian.