Re: [PATCH] Fix core.worktree being used when GIT_DIR is not set

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 27, 2009, 20:58 UTC
Message-ID: <7viqbsw2vn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1261920513-25189-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 14 quoted lines

> According to config.txt:
>> core.worktree::
>>         Set the path to the working tree.  The value will not be
>>         used in combination with repositories found automatically in
>>         a .git directory (i.e. $GIT_DIR is not set).
>
> This behavior was changed after e90fdc3 (Clean up work-tree handling -
> 2007-08-01) and 9459aa7 (Do check_repository_format() early (re-fix) -
> 2007-12-05). If core.worktree is set, even if git_dir automatically
> found (and git_work_tree_cfg set), git_work_tree_cfg will be reset to
> core.worktree. This makes core.worktree effective even if GIT_DIR is
> not set, in contrast to config.txt.
>
> This patch makes sure it only checks for core.worktree if GIT_DIR is set.

The work-tree area got too complicated over time for a small Panda brain to grasp, so let me think aloud here.

The command line option --git-dir= has the same impact on the semantics as the $GIT_DIR environment variable has. The only difference is that the option has higher precedence over environment. Therefore, I won't talk about the command line options in the following description.

In the beginning, there was GIT_DIR environment variable. We had a very simple semantics:

 - When there is no GIT_DIR environment variable:

   - if "." is a "git directory", i.e. it has the set of git things like
     refs/, objects/ and HEAD, then you are working in a bare repository.

   - if ./.git exists and it is a "git directory", then "." is the top of
     the work tree;

   - otherwise, try the parent directory of "." for the second rule
     repeatedly to find the git directory and the top of the work tree.

 - When there is GIT_DIR environment variable:

   - $GIT_DIR is (and must be) the "git directory" and "." is the top of
     the work tree.

People wanted to have a work tree that is at a location totally unrelated to where the "git directory" and setting $GIT_DIR at runtime was the only way to do so, but that restricted them to work only at the top of the work tree. $GIT_WORK_TREE was invented as a way to say "this is the top of the work tree". So that people can do something like:

    $ GIT_DIR=/srv/git/proj.git GIT_WORK_TREE=/scratch/proj
    $ export GIT_DIR GIT_WORK_TREE
    $ cd $GIT_WORK_TREE/Documentation
    $ edit; git diff; git commit -a; ...

Because the facility was meant to allow separation of "git directory" and its associated work tree, and not meant to allow more than one work trees sharing the same "git directory" (which does not make any sense, as there is only one index in "git directory" that describes the state of the work tree), it was an unnecessary nuisanse that you had to set two environment variables. core.worktree was invented---by recording the location of the work tree in the config file in the "git directory", the above can be made into this:

    $ GIT_DIR=/srv/git/proj.git
    $ cd /scratch/proj/Documentation
    $ edit; git diff; git commit -a; ...

Given these background, I am not sure the "fix" is addressing the right issue. What does it mean to have "core.worktree" in a configuration file, but that configuration file was found in a "git directory" that was found thorough the repository discovery process due to lack of $GIT_DIR? There are only two cases I can see:

 - The user is in the "git directory" itself, which is bare (iow,
   /srv/git/proj.git in the above example).  This is not the case the
   documentation snippet you quoted is about, and I don't think your patch
   changes (nor should change) the behaviour for;

 - The "git directory" is a ".git/" subdirectory of some work tree, and
   the value of core.worktree may or may not match that work tree.  This
   is the case the documentation talks about, and your patch addresses.

For the former case, while I don't see much point, we do seem to support this use case (continuing the example scenario):

    $ unset GIT_DIR GIT_WORK_TREE
    $ cd /srv/git/proj.git
    $ git checkout -b newbranch master

We find that "." is our "git directory", and through its config file, we know core.worktree points at /scratch/proj/, and the checkout updates files over there, not in /srv/git/proj.git/. While it is not obvious why anybody finds this useful to me, I think the behaviour makes _some_ sense, and I don't think your patch breaks it by changing the behaviour for this case [*1*].

The latter, unless core.worktree matches the parent directory of the "git directory" in question, seems to me a misconfiguration and nothing else. Shouldn't it be diagnosed as an error, instead of matching the documentation to the letter?

[Footnote]

*1* I said "makes _some_ sense" for a reason. While operations like switching branches that is inherently whole-tree makes sense, it is totally unclear what operations that work relative to the work tree, i.e. "git add", in such a set-up.