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

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On 12/28/09, Junio C Hamano <gitster@pobox.com> wrote:
> 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.
>
>  ...
>
>  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?

I had not read that part of the documentation until Robin pointed out
and always thought core.worktree was in effect if set. I thought the
author intention was not to let core.worktree get in the way if not
requested, but given that the worktree is moved to somewhere else
already, that does not make sense as it could use parent directory of
the "git directory" as worktree (unless core.worktree matches the
parent directory as you said). Probably best fixing documentation.
-- 
Duy