patchrehabilitate 'git index-pack' inside the object store

Before commit d0b92a3f6e it was possible to run 'git index-pack' directly in the .git/objects/pack/ directory. Restore that ability.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

This tries to make sure that the recent fix to d0b92a3 (index-pack: setup git repository, 2008-08-26) does not introduce regression, and protects the fix from getting broken again.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * And unfortunately the first one fails with the fix and succeeds
   without; the second one succeeds with your fix and fails without.

 t/t5302-pack-index.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

On 10/21/08, Nicolas Pitre <nico@cam.org> wrote:

I am sorry I did not catch this in the first place. While the fix should be fine for "git index-pack". I wonder what can happen for other setup_*_gently()'s consumers. Other commands may be affected too (e.g. running "git apply" or "git bundle" from inside .git). Maybe we should let setup_*_gently() do read-only stuff only and let its consumers to handle cwd. I recall Jeff has plan about worktree setup rework, though could not find the thread. Will think more of it tomorrow.

On Tue, Oct 21, 2008 at 09:57:04PM +0700, Nguyen Thai Ngoc Duy wrote:

If by "plan" you mean "intense desire to see it fixed", then yes, I have a plan. But there isn't a concrete plan yet, and I'm not sure when I'll get to it. I have a vague assumption that we should move in the direction of just setting up as much of the environment as possible (finding git dir, finding work tree, reading config, etc) as soon as possible, but not producing any error messages or dying as a result. Then those commands which want to enforce that some setup has occurred can easily do so.

-Peff

On Tue, 21 Oct 2008, Nguyen Thai Ngoc Duy wrote:

Normally you should not be running such commands inside the .git directory. The index-pack command is a bit special in that regard.

Nicolas

Hi,

On Tue, 21 Oct 2008, Nguyen Thai Ngoc Duy wrote:

I think that makes sense. This would allow setup*gently() to be much cleaner, and only setup_git_directory() itself would do the chdir(worktree).

However, let's think this really through this time, so that that darned worktree stuff is fixed for good.

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.

Ciao, Dscho

On Tue, Oct 21, 2008 at 07:02:48PM +0200, Johannes Schindelin wrote:

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.

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.

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).

-Peff

Hi,

On Tue, 21 Oct 2008, Jeff King wrote:

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.

Right.

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

On 10/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

I played a bit with code, extracted discover_git_directory() from setup_git_directory_gently() then made the latter a wrapper of the former with chdir(). Some more for thoughts from the experiment.

1. Because discover_git_directory() does not do chdir() until later in
setup_git_directory_gently(), setting GIT_DIR to a relative path seems
unsafe (or worse unset at all, in case .git is found in parent
directories). But making GIT_DIR absolute path breaks tests because
some of them expect "git rev-parse --git-dir" to return a relative
path. The approach used in 044bbbc (Make git_dir a path relative to
work_tree in setup_work_tree()) can be reused to performance loss, but
that won't solve the issue.

2. 044bbbc also brings up another issue: code duplication between
setup_git_directory_gently() and setup_work_tree(). The new
setup*gently() can be roughly like this if setup_work_tree() can
calculate prefix too:

const char *setup_git_directory_gently(int *nongit_ok)
{
        int nonworktree_ok;
        /*
         * Let's assume that we are in a git repository.
         * If it turns out later that we are somewhere else, the value will be
         * updated accordingly.
         */
        if (nongit_ok)
                *nongit_ok = 0;

        if (!discover_git_directory()) {
                if (nongit_ok) {
                        *nongit_ok = 1;
                        return NULL;
                }
                die("Not a git repository");
        }

        return setup_work_tree_gently(&nonworktree_ok); // gentle version
}

So I propose to make setup_work_tree() return a prefix, relative to current cwd. The setup procedure then would become:

if (discover_git_directory())
    die("Git repository needed");
prefix = setup_work_tree(); // die() inside if cannot setup worktree

3. Dealing with cwd outside worktree. If cwd is inside a worktree,
prefix will be calculated correctly. If it is outside, the current
behavior (with both GIT_DIR and GIT_WORK_TREE set) is leave prefix as
NULL. I think that is not right. With a wrong prefix, git commands
will not be able to access on-disk files. I would propose either:
  - die() if cwd is outside worktree
  - setup*gently() discovers the situation and gives up, then lets git
commands handle themselves. Some commands, like git-archive, don't
care about on-disk files at all, they could just simply ignore the
prefix and keep going. Others may die() or handle it properly.

Again, this breaks things.

On 10/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

One more thing: "git foo -h" with RUN_SETUP won't run if repository is not found. Maybe just drop RUN_SETUP and let subcommands call setup_git_directory()

On 10/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Also, from [1] may be only set default git_dir in setup_git_env() after setup_git_directory() has been called and before setup_work_tree() is called (if any)? If not, die().

[1] http://article.gmane.org/gmane.comp.version-control.git/98849