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

[Junio C Hamano <gitster@pobox.com>]

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

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