Re: [PATCH] Add missing inside_work_tree setting in setup_git_directory_gently

Nguyễn Thái Ngọc Duy [off-list ref] writes:

When both GIT_DIR and GIT_WORK_TREE are set, and
setup_git_directory_gently() changes the current working
directory accordingly, it should also set inside_work_tree = 1.

Without this, work_tree handling code in setup_git_directory()
will be activated. If you stay in root work tree (no prefix),
it does not harm. It does if you work from a subdirectory though.

Please add automated test script for this, thanks.

On Sat, Nov 03, 2007 at 09:33:40PM -0700, Junio C Hamano wrote:

Nguyễn Thái Ngọc Duy [off-list ref] writes:

quoted

When both GIT_DIR and GIT_WORK_TREE are set, and
setup_git_directory_gently() changes the current working
directory accordingly, it should also set inside_work_tree = 1.

Without this, work_tree handling code in setup_git_directory()
will be activated. If you stay in root work tree (no prefix),
it does not harm. It does if you work from a subdirectory though.

Please add automated test script for this, thanks.

Thank you for reminding.  I tried to put a test in
t1501-worktree.sh and found out core.worktree can override
inside_work_tree previously set by setup_git_directory_gently(),
activating the worktree code in setup_git_directory() again.

This made me think setup_git_directory_gently() should use
get_git_work_tree() instead. But then git_work_tree_cfg may not be
initialized when get_git_work_tree() is called (starting from
setup_git_directory(), git_work_tree_cfg is initialized in
check_repository_format_version(), which is called _after_
setup_git_directory_gently()).

The interaction between these variables and functions is really beyond
my knowledge. Johannes, can you have a look at this? In theory the
following test should pass:

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 7ee3820..bdb7720 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh

@@ -103,6 +103,11 @@ test_expect_success 'repo finds its work tree from work tree, too' '
 	 test sub/dir/tracked = "$(git ls-files)")
 '
 
+test_expect_success 'Try a command from subdir in worktree' '
+	(cd repo.git/work/sub &&
+	GIT_DIR=../.. GIT_WORK_TREE=.. git blame dir/tracked)
+'
+
 test_expect_success '_gently() groks relative GIT_DIR & GIT_WORK_TREE' '
 	cd repo.git/work/sub/dir &&
 	GIT_DIR=../../.. GIT_WORK_TREE=../.. GIT_PAGER= \

-- 
Duy

Nguyen Thai Ngoc Duy [off-list ref] writes:

I am wondering what happened to this thread...

Hi,

On Wed, 7 Nov 2007, Junio C Hamano wrote:

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Sat, Nov 03, 2007 at 09:33:40PM -0700, Junio C Hamano wrote:

quoted

Please add automated test script for this, thanks.

Thank you for reminding.  I tried to put a test in
t1501-worktree.sh and found out core.worktree can override
inside_work_tree previously set by setup_git_directory_gently(),
activating the worktree code in setup_git_directory() again.

This made me think setup_git_directory_gently() should use
get_git_work_tree() instead. But then git_work_tree_cfg may not be
initialized when get_git_work_tree() is called (starting from
setup_git_directory(), git_work_tree_cfg is initialized in
check_repository_format_version(), which is called _after_
setup_git_directory_gently()).

The interaction between these variables and functions is really beyond
my knowledge. Johannes, can you have a look at this? In theory the
following test should pass:

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 7ee3820..bdb7720 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh

@@ -103,6 +103,11 @@ test_expect_success 'repo finds its work tree from work tree, too' '
 	 test sub/dir/tracked = "$(git ls-files)")
 '
 
+test_expect_success 'Try a command from subdir in worktree' '
+	(cd repo.git/work/sub &&
+	GIT_DIR=../.. GIT_WORK_TREE=.. git blame dir/tracked)
+'
+
 test_expect_success '_gently() groks relative GIT_DIR & GIT_WORK_TREE' '
 	cd repo.git/work/sub/dir &&
 	GIT_DIR=../../.. GIT_WORK_TREE=../.. GIT_PAGER= \

I am wondering what happened to this thread...

It is still in my inbox, waiting for a time where I can actually 
concentrate.

Ciao,
Dscho

Hi,

On Sun, 4 Nov 2007, Nguyen Thai Ngoc Duy wrote:

This does not really test work_tree, but if blame uses the work_tree 
machinery correctly.

I will send out a patch to builtin-blame.c in a minute.

However, this test case still fails, since blame needs a HEAD revision!  
And in t1501 there is no commit done yet.

Ciao,
Dscho

We check in cmd_blame() if the specified path is there, but we
failed to set up the working tree before that.

While at it, make setup_work_tree() just return if it was run
before.

Signed-off-by: Johannes Schindelin <redacted>
---

	IMO both this patch and the recent patch to call setup_work_tree() 
	are needed.  Only the second call to setup_work_tree() will know 
	if there was no revision specified, and the first one will know if 
	a -- has been seen.

 builtin-blame.c |    1 +
 setup.c         |   10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 55a3c0b..ba80bf8 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -2295,6 +2295,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			else if (i != argc - 1)
 				usage(blame_usage); /* garbage at end */
 
+			setup_work_tree();
 			if (!has_path_in_work_tree(path))
 				die("cannot stat path %s: %s",
 				    path, strerror(errno));

diff --git a/setup.c b/setup.c
index 084d722..1421a2c 100644
--- a/setup.c
+++ b/setup.c

@@ -207,12 +207,18 @@ static const char *set_work_tree(const char *dir)
 }
 
 void setup_work_tree(void) {
-	const char *work_tree = get_git_work_tree();
-	const char *git_dir = get_git_dir();
+	const char *work_tree, *git_dir;
+	static int initialized = 0;
+
+	if (initialized)
+		return;
+	work_tree = get_git_work_tree();
+	git_dir = get_git_dir();
 	if (!is_absolute_path(git_dir))
 		set_git_dir(make_absolute_path(git_dir));
 	if (!work_tree || chdir(work_tree))
 		die("This operation must be run in a work tree");
+	initialized = 1;
 }
 
 /*

-- 
1.5.3.5.1645.g1f4df