git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* core.worktree bug
@ 2009-12-07 10:15 Robin Rosenberg
  2009-12-15 16:30 ` Nguyen Thai Ngoc Duy
  2009-12-27 13:28 ` [PATCH] Fix core.worktree being used when GIT_DIR is not set Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 11+ messages in thread
From: Robin Rosenberg @ 2009-12-07 10:15 UTC (permalink / raw)
  To: git


According to git-config(1) 

       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 can be overridden by the GIT_WORK_TREE environment variable and the
           --work-tree command line option. It can be a absolute path or relative path to the directory specified by --git-dir or
           GIT_DIR. Note: If --git-dir or GIT_DIR are specified but none of --work-tree, GIT_WORK_TREE and core.worktree is
           specified, the current working directory is regarded as the top directory of your working tree.

this setting is not used if GIT_DIR is set. But when I try it out 

$ mkdir r1
$ mkdir r2
$ cd r2
$ git init
Initialized empty Git repository in /home/me/tmp/r2/.git/
$ date >f
$ git add f
$ git commit -m "f"
 f
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 f
$ git status
# On branch master
nothing to commit (working directory clean)

=> Nothing interesting here. It comes here:

$ git config core.worktree $(cd ../r1;pwd)
$ git status
# On branch master
# Changed but not updated:
#   (use "git add/rm <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       deleted:    f
#
no changes added to commit (use "git add" and/or "git commit -a")

=> Seems the config is actually honored even though GIT_DIR is not set.

Bisect tells me 4f38f6b5bafb1f7f85c7b54d0bb0a0e977cd947c broke it. My main point is that I am
implementing this in JGit so I want the same behaviour. Question: Should we try to fix this
in git so it matches the documentation or fix the documentation to match behaviour.

The breakage appeared over a year ago and no one has complained.

-- robin

My bisection script:

#!/bin/bash

test_description="CEILING"

. ./test-lib.sh

test_expect_success \
    "ceiling" \
    "
     rm -rf r1 r2 &&
     mkdir r1 r2 &&
     (cd r2 &&
     git init &&
     date >f &&
     git add f &&
echo ADD &&
     git commit -m f &&
echo COMMIT &&
     git diff --exit-code f && 
echo DIFF &&
     git config core.worktree \$(cd ../r1/..;pwd) &&
echo CONFIG &&
     (unset GIT_DIR;git diff --exit-code -- f) &&
echo DIFF
     )
     "

test_don

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: core.worktree bug
  2009-12-07 10:15 core.worktree bug Robin Rosenberg
@ 2009-12-15 16:30 ` Nguyen Thai Ngoc Duy
  2009-12-27 13:28 ` [PATCH] Fix core.worktree being used when GIT_DIR is not set Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 11+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2009-12-15 16:30 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

On Mon, Dec 07, 2009 at 11:15:47AM +0100, Robin Rosenberg wrote:
> $ git config core.worktree $(cd ../r1;pwd)
> $ git status
> # On branch master
> # Changed but not updated:
> #   (use "git add/rm <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       deleted:    f
> #
> no changes added to commit (use "git add" and/or "git commit -a")
> 
> => Seems the config is actually honored even though GIT_DIR is not set.
> 
> Bisect tells me 4f38f6b5bafb1f7f85c7b54d0bb0a0e977cd947c broke it. My main point is that I am

You should have CCed me.

> implementing this in JGit so I want the same behaviour. Question: Should we try to fix this
> in git so it matches the documentation or fix the documentation to match behaviour.
> 
> The breakage appeared over a year ago and no one has complained.

This is, I think, due to the shared use of git_work_tree_cfg. When
setup_git_directory_gently() comes close to the end, work_tree has
been detected and set. Then check_repository_format_gently() is
called to make sure the repository is valid. Among the checks are
core.worktree check, which overrides the previously-set git_work_tree_cfg.

This might be the fix, or a start of new breakages. I'll need to look
at this again and make a proper patch message with tests if it's
really correct.

diff --git a/setup.c b/setup.c
index f67250b..1385edb 100644
--- a/setup.c
+++ b/setup.c
@@ -280,6 +280,18 @@ const char *read_gitfile_gently(const char *path)
 	return path;
 }
 
+static int check_repository_work_tree(const char *var, const char *value, void *cb)
+{
+	if (strcmp(var, "core.worktree") == 0) {
+		if (!value)
+			return config_error_nonbool(var);
+		free(git_work_tree_cfg);
+		git_work_tree_cfg = xstrdup(value);
+		inside_work_tree = -1;
+	}
+	return 0;
+}
+
 /*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
@@ -317,6 +329,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env) {
 				retval = set_work_tree(gitdirenv);
 				/* config may override worktree */
+				git_config(check_repository_work_tree, NULL);
 				if (check_repository_format_gently(nongit_ok))
 					return NULL;
 				return retval;
@@ -394,6 +407,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			die_errno("Cannot change to '%s/..'", cwd);
 	}
 
+	git_config(check_repository_work_tree, NULL);
 	inside_git_dir = 0;
 	if (!work_tree_env)
 		inside_work_tree = 1;
@@ -471,12 +485,6 @@ int check_repository_format_version(const char *var, const char *value, void *cb
 		is_bare_repository_cfg = git_config_bool(var, value);
 		if (is_bare_repository_cfg == 1)
 			inside_work_tree = -1;
-	} else if (strcmp(var, "core.worktree") == 0) {
-		if (!value)
-			return config_error_nonbool(var);
-		free(git_work_tree_cfg);
-		git_work_tree_cfg = xstrdup(value);
-		inside_work_tree = -1;
 	}
 	return 0;
 }



-- 
Duy

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] Fix core.worktree being used when GIT_DIR is not set
  2009-12-07 10:15 core.worktree bug Robin Rosenberg
  2009-12-15 16:30 ` Nguyen Thai Ngoc Duy
@ 2009-12-27 13:28 ` Nguyễn Thái Ngọc Duy
  2009-12-27 20:58   ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2009-12-27 13:28 UTC (permalink / raw)
  To: git, Junio C Hamano, Johannes Schindelin, Robin Rosenberg
  Cc: Nguyễn Thái Ngọc Duy

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.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 setup.c             |   19 +++++++++++++------
 t/t1501-worktree.sh |    4 ++++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/setup.c b/setup.c
index 2cf0f19..35b7915 100644
--- a/setup.c
+++ b/setup.c
@@ -283,6 +283,18 @@ const char *read_gitfile_gently(const char *path)
 	return path;
 }
 
+static int check_repository_work_tree(const char *var, const char *value, void *cb)
+{
+	if (strcmp(var, "core.worktree") == 0) {
+		if (!value)
+			return config_error_nonbool(var);
+		free(git_work_tree_cfg);
+		git_work_tree_cfg = xstrdup(value);
+		inside_work_tree = -1;
+	}
+	return 0;
+}
+
 /*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
@@ -320,6 +332,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env) {
 				retval = set_work_tree(gitdirenv);
 				/* config may override worktree */
+				git_config(check_repository_work_tree, NULL);
 				if (check_repository_format_gently(nongit_ok))
 					return NULL;
 				return retval;
@@ -474,12 +487,6 @@ int check_repository_format_version(const char *var, const char *value, void *cb
 		is_bare_repository_cfg = git_config_bool(var, value);
 		if (is_bare_repository_cfg == 1)
 			inside_work_tree = -1;
-	} else if (strcmp(var, "core.worktree") == 0) {
-		if (!value)
-			return config_error_nonbool(var);
-		free(git_work_tree_cfg);
-		git_work_tree_cfg = xstrdup(value);
-		inside_work_tree = -1;
 	}
 	return 0;
 }
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 74e6443..9756f35 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -30,6 +30,10 @@ test_rev_parse() {
 
 EMPTY_TREE=$(git write-tree)
 mkdir -p work/sub/dir || exit 1
+
+git config core.worktree work
+test_rev_parse 'core.worktree without GIT_DIR' false false true ''
+
 mv .git repo.git || exit 1
 
 say "core.worktree = relative path"
-- 
1.6.5.2.216.g9c1ec

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix core.worktree being used when GIT_DIR is not set
  2009-12-27 13:28 ` [PATCH] Fix core.worktree being used when GIT_DIR is not set Nguyễn Thái Ngọc Duy
@ 2009-12-27 20:58   ` Junio C Hamano
  2009-12-28  0:08     ` Robin Rosenberg
  2009-12-28  5:41     ` Nguyen Thai Ngoc Duy
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2009-12-27 20:58 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Johannes Schindelin, Robin Rosenberg

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix core.worktree being used when GIT_DIR is not set
  2009-12-27 20:58   ` Junio C Hamano
@ 2009-12-28  0:08     ` Robin Rosenberg
  2009-12-28  5:41     ` Nguyen Thai Ngoc Duy
  1 sibling, 0 replies; 11+ messages in thread
From: Robin Rosenberg @ 2009-12-28  0:08 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Johannes Schindelin,
	Robin Rosenberg

söndagen den 27 december 2009 21.58.20 skrev  Junio C Hamano:
> 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.

...

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

I'm inclined towards fixing the docs. Overriding a config setting in non-
intuitive and nobody has complained.

-- robin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix core.worktree being used when GIT_DIR is not set
  2009-12-27 20:58   ` Junio C Hamano
  2009-12-28  0:08     ` Robin Rosenberg
@ 2009-12-28  5:41     ` Nguyen Thai Ngoc Duy
  2009-12-28  5:55       ` [PATCH] Documentation: always respect core.worktree if set Nguyễn Thái Ngọc Duy
  1 sibling, 1 reply; 11+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2009-12-28  5:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Robin Rosenberg

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Documentation: always respect core.worktree if set
  2009-12-28  5:41     ` Nguyen Thai Ngoc Duy
@ 2009-12-28  5:55       ` Nguyễn Thái Ngọc Duy
  2009-12-28  9:16         ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2009-12-28  5:55 UTC (permalink / raw)
  To: git, Junio C Hamano, Johannes Schindelin, Robin Rosenberg
  Cc: Nguyễn Thái Ngọc Duy

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

According to the current documentation, if GIT_DIR is not set,
core.worktree is not respected, and the parent directory of the "git
directory" may be used as worktree. This case, unless core.worktree
matches the parent directory of the "git directory" in question, seems
a misconfiguration and nothing else. So remove this part of the
documentation.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Junio may find the description highly familiar :) only slightly modified
 at the end

 Documentation/config.txt |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7d10a21..4b3d568 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -297,9 +297,7 @@ false), while all other repositories are assumed to be bare (bare
 = true).
 
 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).
+	Set the path to the working tree.
 	This can be overridden by the GIT_WORK_TREE environment
 	variable and the '--work-tree' command line option. It can be
 	a absolute path or relative path to the directory specified by
-- 
1.6.5.2.216.g9c1ec

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation: always respect core.worktree if set
  2009-12-28  5:55       ` [PATCH] Documentation: always respect core.worktree if set Nguyễn Thái Ngọc Duy
@ 2009-12-28  9:16         ` Junio C Hamano
  2009-12-29  7:48           ` Nguyễn Thái Ngọc Duy
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2009-12-28  9:16 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Johannes Schindelin, Robin Rosenberg

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

> In the beginning, there was GIT_DIR environment variable, with a very
> simple semantics:
> ...

(Omitted an explanation of how GIT_DIR and core.worktree work together).

> According to the current documentation, if GIT_DIR is not set,
> core.worktree is not respected, and the parent directory of the "git
> directory" may be used as worktree. This case, unless core.worktree
> matches the parent directory of the "git directory" in question, seems
> a misconfiguration and nothing else. So remove this part of the
> documentation.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Junio may find the description highly familiar :) only slightly modified
>  at the end

I found it familiar, but I didn't see how that earlier part leads to the
conclusion in, or even gives a necessary background information for, the
last paragraph.  Perhaps you omitted something in the middle that was more
relevant?  If so, I would suggest omiting the earlier part, whose purpose
was to prepare readers for that middle part, as well.

And I don't think the claim your last paragraph makes is consistent with
what the patch does at all.

Removing the "it will be ignored" from the documentation is a good change
only in the sense that it makes the document closer to reality, but "it is
a misconfiguration" is not a good justification of the change.

A documentation-only patch that can be justitifed by "a misconfiguration"
claim would probably read like this:

	The value of core.worktree in a ".git/config" is honored even when
	it differs from the directory that has the ".git" directory as its
	subdirectory.  This is likely to be a misconfiguration, so warn
	users about it.  Also, drop the part of the documentation that
	incorrectly claimed that we ignore such a misconfigured value.

	---

	core.worktree::
                Set the path to the root of the work tree.  Note that this
        	is honored even when set in a configuration file in a
        	".git" subdirectory of a directory, and its value differs
        	from the latter directory (e.g. "/path/to/.git/config" has
        	core.worktree set to "/different/path"), which is most
        	likely a misconfiguration.  Running git commands in
        	"/path/to" directory will still use "/different/path" as
        	the root of the work tree and can cause great confusion to
        	the users.


Your earlier patch took a different approach to the misconfiguration.  "It
is an error, so we silently _ignore_".  It is a valid thing to say, but
the "silently" part is not friendly to the user and we would probably want
to diagnose and warn.  That is what I originally meant.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Documentation: always respect core.worktree if set
  2009-12-28  9:16         ` Junio C Hamano
@ 2009-12-29  7:48           ` Nguyễn Thái Ngọc Duy
  2009-12-29 16:58             ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2009-12-29  7:48 UTC (permalink / raw)
  To: git, Junio C Hamano, Johannes Schindelin, Robin Rosenberg
  Cc: Nguyễn Thái Ngọc Duy

The value of core.worktree in a ".git/config" is honored even when it
differs from the directory that has the ".git" directory as its
subdirectory.  This is likely to be a misconfiguration, so warn users
about it.  Also, drop the part of the documentation that incorrectly
claimed that we ignore such a misconfigured value.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7d10a21..2eb9758 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -297,17 +297,24 @@ false), while all other repositories are assumed to be bare (bare
 = true).
 
 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).
+	Set the path to the root of the work tree.
 	This can be overridden by the GIT_WORK_TREE environment
 	variable and the '--work-tree' command line option. It can be
-	a absolute path or relative path to the directory specified by
-	--git-dir or GIT_DIR.
-	Note: If --git-dir or GIT_DIR are specified but none of
+	an absolute path or a relative path to the .git directory,
+	either specified by --git-dir or GIT_DIR, or automatically
+	discovered.
+	If --git-dir or GIT_DIR are specified but none of
 	--work-tree, GIT_WORK_TREE and core.worktree is specified,
-	the current working directory is regarded as the top directory
-	of your working tree.
+	the current working directory is regarded as the root of the
+	work tree.
++
+Note that this variable is honored even when set in a configuration
+file in a ".git" subdirectory of a directory, and its value differs
+from the latter directory (e.g. "/path/to/.git/config" has
+core.worktree set to "/different/path"), which is most likely a
+misconfiguration.  Running git commands in "/path/to" directory will
+still use "/different/path" as the root of the work tree and can cause
+great confusion to the users.
 
 core.logAllRefUpdates::
 	Enable the reflog. Updates to a ref <ref> is logged to the file
-- 
1.6.5.2.216.g9c1ec

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation: always respect core.worktree if set
  2009-12-29  7:48           ` Nguyễn Thái Ngọc Duy
@ 2009-12-29 16:58             ` Junio C Hamano
  2009-12-29 17:05               ` Robin Rosenberg
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2009-12-29 16:58 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Johannes Schindelin, Robin Rosenberg

Thanks; I'll take this "match documentation to reality with caveats" patch
for now, but I personally think we should revisit the issue someday.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation: always respect core.worktree if set
  2009-12-29 16:58             ` Junio C Hamano
@ 2009-12-29 17:05               ` Robin Rosenberg
  0 siblings, 0 replies; 11+ messages in thread
From: Robin Rosenberg @ 2009-12-29 17:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Johannes Schindelin,
	Robin Rosenberg

tisdagen den 29 december 2009 17.58.17 skrev  Junio C Hamano:
> Thanks; I'll take this "match documentation to reality with caveats" patch
> for now, but I personally think we should revisit the issue someday.
> 

I'm happy with this.

-- robin

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-12-29 17:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-07 10:15 core.worktree bug Robin Rosenberg
2009-12-15 16:30 ` Nguyen Thai Ngoc Duy
2009-12-27 13:28 ` [PATCH] Fix core.worktree being used when GIT_DIR is not set Nguyễn Thái Ngọc Duy
2009-12-27 20:58   ` Junio C Hamano
2009-12-28  0:08     ` Robin Rosenberg
2009-12-28  5:41     ` Nguyen Thai Ngoc Duy
2009-12-28  5:55       ` [PATCH] Documentation: always respect core.worktree if set Nguyễn Thái Ngọc Duy
2009-12-28  9:16         ` Junio C Hamano
2009-12-29  7:48           ` Nguyễn Thái Ngọc Duy
2009-12-29 16:58             ` Junio C Hamano
2009-12-29 17:05               ` Robin Rosenberg

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).