Re: [PATCH] setup: do not change to work tree prematurely

[Jonathan Nieder <jrnieder@gmail.com>]

Hi Clemens,

Clemens Buchacher wrote:

> $ cd .git
> $ git --work-tree=/tmp/git symbolic-ref HEAD
> fatal: ref HEAD is not a symbolic ref

Without your patch applied, I get

 $ cd .git
 $ git --work-tree=/tmp/git symbolic-ref HEAD
 refs/heads/hello

I should have done the following instead:

 $ worktree=$(pwd)
 $ cd .git
 $ git --work-tree="$worktree" symbolic-ref HEAD
 fatal: ref HEAD is not a symbolic ref

> +char *get_relative_path(char *cwd, const char *dir)

Looks reasonable.  Should be const char *cwd.

> diff --git a/setup.c b/setup.c
> index 5716d90..67b5122 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -524,14 +524,18 @@ const char *setup_git_directory(void)
> 	const char *retval = setup_git_directory_gently(NULL);
>  
>  	/* If the work tree is not the default one, recompute prefix */
>  	if (inside_work_tree < 0) {

If I understand correctly, you made two changes here:

 - interpret GIT_WORK_TREE and friends relative to .git instead of
   the original cwd (by not calling chdir(retval) before
   get_git_work_tree())

 - make setup_git_directory stay in the last directory searched for a
   .git directory instead of chdir-ing into the toplevel of the work tree.

Are these safe changes to make?

Jonathan

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 9df3012..93bf92c 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -106,6 +106,15 @@ test_expect_success 'repo finds its work tree from work tree, too' '
 	 test sub/dir/tracked = "$(git ls-files)")
 '
 
+test -d work || cp -R repo.git/work .
+test_expect_success 'repo finds its .git dir with separate worktree' '
+	(unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
+	 worktree=$(pwd) &&
+	 cd repo.git &&
+	 git symbolic-ref HEAD &&
+	 git --work-tree="$worktree" symbolic-ref HEAD)
+'
+
 test_expect_success '_gently() groks relative GIT_DIR & GIT_WORK_TREE' '
 	(cd repo.git/work/sub/dir &&
 	GIT_DIR=../../.. GIT_WORK_TREE=../.. GIT_PAGER= \
--