new plan for cleaning up the worktree mess, was Re: [PATCH] rehabilitate 'git index-pack' inside the object store

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 21, 2008, 17:59 UTC
Message-ID: <alpine.DEB.1.00.0810211955250.22125@pacific.mpi-cbg.de.mpi-cbg.de>
In-Reply-To: <20081021174303.GA25827@coredump.intra.peff.net>

Hi,

On Tue, 21 Oct 2008, Jeff King wrote:

Show 17 quoted lines

> On Tue, Oct 21, 2008 at 07:02:48PM +0200, Johannes Schindelin wrote:
> 
> > So I propose this change in semantics:
> > 
> > - setup_git_directory_gently(): rename to discover_git_directory(), 
> >   and avoid any chdir() at all.
> > - setup_git_directory(): keep the semantics that it chdir()s to the
> >   worktree, or to the git directory for bare repositories.
> > 
> > Using _gently() even for RUN_SETUP builtins should solve the long 
> > standing pager problem, too.
> 
> I'm not sure there aren't hidden problems lurking in that strategy 
> (every time I look at this area of code, something unexpected prevents 
> what I think should Just Work from Just Working), but I think that is a 
> promising direction to go for clearing up some of the long-standing 
> issues.

Same here. I grew a pretty strong opinion about the whole worktree thing, but maybe that is only because it was done trying to change as little as possible.

> I think you will need to do something for a few commands which use 
> _gently() but then, if we _are_ in a git repo, assume we are chdir'd 
> properly.

Right.

> We may also be able to just call setup_git_directory_gently() as the 
> first thing in the wrapper, which should help us more consistently find 
> config before the 'exec' stage (see 4e10738a for a discussion of some of 
> the issues).

Yep, that's what I referred to as "pager" problems. Thanks for the pointer! For other people's convenience I quote it here :-)

-- snip --
    Allow per-command pager config

    There is great debate over whether some commands should set
    up a pager automatically. This patch allows individuals to
    set their own pager preferences for each command, overriding
    the default. For example, to disable the pager for git
    status:

      git config pager.status false

    If "--pager" or "--no-pager" is specified on the command
    line, it takes precedence over the config option.

    There are two caveats:

      - you can turn on the pager for plumbing commands.
        Combined with "core.pager = always", this will probably
        break a lot of things. Don't do it.

      - This only works for builtin commands. The reason is
        somewhat complex:

        Calling git_config before we do setup_git_directory
        has bad side effects, because it wants to know where
        the git_dir is to find ".git/config". Unfortunately,
        we cannot call setup_git_directory indiscriminately,
        because some builtins (like "init") break if we do.

        For builtins, this is OK, since we can just wait until
        after we call setup_git_directory. But for aliases, we
        don't know until we expand (recursively) which command
        we're doing. This should not be a huge problem for
        aliases, which can simply use "--pager" or "--no-pager"
        in the alias as appropriate.

        For external commands, however, we don't know we even
        have an external command until we exec it, and by then
        it is too late to check the config.

        An alternative approach would be to have a config mode
        where we don't bother looking at .git/config, but only
        at the user and system config files. This would make the
        behavior consistent across builtins, aliases, and
        external commands, at the cost of not allowing per-repo
        pager config for at all.
-- snap --

Ciao, Dscho