[PATCH 1/2] t1501: avoid bashisms

[PATCH 1/2] t1501: avoid bashisms

[Nguyễn Thái Ngọc Duy]

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

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..488160e 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -322,7 +322,10 @@ test_expect_success 'git grep' '
 test_expect_success 'git commit' '
 	(
 		cd repo.git &&
-		GIT_DIR=. GIT_WORK_TREE=work git commit -a -m done
+		GIT_DIR=. &&
+		GIT_WORK_TREE=work &&
+		export GIT_DIR GIT_WORK_TREE &&
+		git commit -a -m done
 	)
 '
 
-- 
1.7.3.4.878.g439c7

[PATCH 2/2] setup_work_tree: adjust relative $GIT_WORK_TREE after moving cwd

[Nguyễn Thái Ngọc Duy]

When setup_work_tree() is called, it moves cwd to $GIT_WORK_TREE and
makes internal copy of $GIT_WORK_TREE absolute. The environt variable,
if set by user, remains unchanged. If the variable is relative, it is
no longer correct because its base dir has changed.

Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequence git processes handle it.

Reported-by: Michel Briand <michelbriand@free.fr>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .gitignore          |    1 +
 Makefile            |    1 +
 setup.c             |    8 ++++++++
 t/t1501-worktree.sh |   11 +++++++++++
 test-subprocess.c   |   18 ++++++++++++++++++
 5 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100644 test-subprocess.c

diff --git a/.gitignore b/.gitignore
index 87b833c..3dd6ef7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
 /test-sha1
 /test-sigchain
 /test-string-pool
+/test-subprocess
 /test-svn-fe
 /test-treap
 /common-cmds.h
diff --git a/Makefile b/Makefile
index 57d9c65..bdf86a3 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-string-pool
+TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-treap
 TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/setup.c b/setup.c
index 91887a4..3833569 100644
--- a/setup.c
+++ b/setup.c
@@ -239,6 +239,14 @@ void setup_work_tree(void)
 		git_dir = make_absolute_path(git_dir);
 	if (!work_tree || chdir(work_tree))
 		die("This operation must be run in a work tree");
+
+	/*
+	 * Make sure subsequent git processes find correct worktree
+	 * if $GIT_WORK_TREE is set relative
+	 */
+	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
+		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
 	set_git_dir(make_relative_path(git_dir, work_tree));
 	initialized = 1;
 }
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 488160e..16c953b 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -343,4 +343,15 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
 	git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
 '
 
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
+	(
+	GIT_DIR=repo.git
+	GIT_WORK_TREE=repo.git/work
+	export GIT_DIR GIT_WORK_TREE
+	test-subprocess rev-parse --show-toplevel >actual &&
+	echo "`pwd`/repo.git/work" >expected &&
+	test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/test-subprocess.c b/test-subprocess.c
new file mode 100644
index 0000000..55ad719
--- /dev/null
+++ b/test-subprocess.c
@@ -0,0 +1,18 @@
+#include "cache.h"
+#include "run-command.h"
+
+int main(int argc, char **argv)
+{
+	const char *prefix;
+	struct child_process cp;
+	int nogit;
+
+	prefix = setup_git_directory_gently(&nogit);
+	if (nogit)
+		die("No git repo found");
+	setup_work_tree();
+	memset(&cp, 0, sizeof(cp));
+	cp.git_cmd = 1;
+	cp.argv = (const char **)argv+1;
+	return run_command(&cp);
+}
-- 
1.7.3.4.878.g439c7

Re: [PATCH 1/2] t1501: avoid bashisms

[Andreas Schwab]

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

> @@ -322,7 +322,10 @@ test_expect_success 'git grep' '
>  test_expect_success 'git commit' '
>  	(
>  		cd repo.git &&
> -		GIT_DIR=. GIT_WORK_TREE=work git commit -a -m done

In which way is that not portable?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/2] t1501: avoid bashisms

[Nguyen Thai Ngoc Duy]

2010/12/25 Andreas Schwab <schwab@linux-m68k.org>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> @@ -322,7 +322,10 @@ test_expect_success 'git grep' '
>>  test_expect_success 'git commit' '
>>       (
>>               cd repo.git &&
>> -             GIT_DIR=. GIT_WORK_TREE=work git commit -a -m done
>
> In which way is that not portable?

I admit that I rarely leave bash, so I'll quote Johannes answer [1]

-- 8< --
Sure, it is (bashisms). This:

   GIT_DIR="$TRASH_DIRECTORY/2/.git" test_repo 2/sub

does not work the same way in all shells when test_repo is a shell
function. You have to export GIT_DIR explicitly before the function call.
(But since in this case, test_repo invokes its own subshell anyway, you
better do it in the function.)
-- 8< --

[1] http://article.gmane.org/gmane.comp.version-control.git/162207
-- 
Duy

Re: [PATCH 1/2] t1501: avoid bashisms

[Nguyen Thai Ngoc Duy]

On Sat, Dec 25, 2010 at 9:29 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> 2010/12/25 Andreas Schwab <schwab@linux-m68k.org>:
>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>>
>>> @@ -322,7 +322,10 @@ test_expect_success 'git grep' '
>>>  test_expect_success 'git commit' '
>>>       (
>>>               cd repo.git &&
>>> -             GIT_DIR=. GIT_WORK_TREE=work git commit -a -m done
>>
>> In which way is that not portable?
>
> I admit that I rarely leave bash, so I'll quote Johannes answer [1]
>
> -- 8< --
> Sure, it is (bashisms). This:
>
>   GIT_DIR="$TRASH_DIRECTORY/2/.git" test_repo 2/sub
>
> does not work the same way in all shells when test_repo is a shell
> function. You have to export GIT_DIR explicitly before the function call.

Hmm.. I misread it. That's only for shell _functions_. [1] says
variable assignments for simple commands are actually exported.

Junio, please don't pick up this patch.

[1] http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html
-- 
Duy

[PATCH] setup_work_tree: adjust relative $GIT_WORK_TREE after moving cwd

[Nguyễn Thái Ngọc Duy]

When setup_work_tree() is called, it moves cwd to $GIT_WORK_TREE and
makes internal copy of $GIT_WORK_TREE absolute. The environt variable,
if set by user, remains unchanged. If the variable is relative, it is
no longer correct because its base dir has changed.

Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequent git processes handle it.

Reported-by: Michel Briand <michelbriand@free.fr>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This one is better, on top of master because the next one is on top
 of nd/setup.

 And I forgot to tell why I did not put the fault in real life [1]
 into tests: I don't like git-merge spawning another process just for
 resetting worktree. Sooner or later it should be replaced to use
 unpack_trees() directly. When that happens, the test would become
 invalid.

 [1] http://thread.gmane.org/gmane.comp.version-control.git/164066/focus=164171

 .gitignore          |    1 +
 Makefile            |    1 +
 setup.c             |    8 ++++++++
 t/t1501-worktree.sh |   11 +++++++++++
 test-subprocess.c   |   21 +++++++++++++++++++++
 5 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100644 test-subprocess.c

diff --git a/.gitignore b/.gitignore
index 87b833c..3dd6ef7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
 /test-sha1
 /test-sigchain
 /test-string-pool
+/test-subprocess
 /test-svn-fe
 /test-treap
 /common-cmds.h
diff --git a/Makefile b/Makefile
index 57d9c65..bdf86a3 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-string-pool
+TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-treap
 TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/setup.c b/setup.c
index 91887a4..3833569 100644
--- a/setup.c
+++ b/setup.c
@@ -239,6 +239,14 @@ void setup_work_tree(void)
 		git_dir = make_absolute_path(git_dir);
 	if (!work_tree || chdir(work_tree))
 		die("This operation must be run in a work tree");
+
+	/*
+	 * Make sure subsequent git processes find correct worktree
+	 * if $GIT_WORK_TREE is set relative
+	 */
+	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
+		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
 	set_git_dir(make_relative_path(git_dir, work_tree));
 	initialized = 1;
 }
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..1f3b50d 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -340,4 +340,15 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
 	git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
 '
 
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
+	(
+	GIT_DIR=repo.git &&
+	GIT_WORK_TREE=repo.git/work &&
+	export GIT_DIR GIT_WORK_TREE &&
+	test-subprocess --setup-work-tree rev-parse --show-toplevel >actual &&
+	echo "`pwd`/repo.git/work" >expected &&
+	test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/test-subprocess.c b/test-subprocess.c
new file mode 100644
index 0000000..667d3e5
--- /dev/null
+++ b/test-subprocess.c
@@ -0,0 +1,21 @@
+#include "cache.h"
+#include "run-command.h"
+
+int main(int argc, char **argv)
+{
+	const char *prefix;
+	struct child_process cp;
+	int nogit = 0;
+
+	prefix = setup_git_directory_gently(&nogit);
+	if (nogit)
+		die("No git repo found");
+	if (!strcmp(argv[1], "--setup-work-tree")) {
+		setup_work_tree();
+		argv++;
+	}
+	memset(&cp, 0, sizeof(cp));
+	cp.git_cmd = 1;
+	cp.argv = (const char **)argv+1;
+	return run_command(&cp);
+}
-- 
1.7.3.4.878.g439c7

[PATCH] setup_explicit_git_dir: adjust relative $GIT_WORK_TREE after moving cwd

[Nguyễn Thái Ngọc Duy]

setup_explicit_git_dir() can move cwd. If $GIT_WORK_TREE is relative
to original cwd, then the subsequent git processes will take wrong
worktree.

Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequent git processes handle it.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This one is on top of nd/setup. If nd/setup does not make it in the
 next release, I can still make similar patch for master, but I don't
 think this happens in real life, therefore not really urgent to fix.

 There are only few cases where git spawns more git processes.
 For those that do that, setup_work_tree() is likely called by git.c
 already, which is what the first patch is for.

 setup.c             |    7 +++++++
 t/t1501-worktree.sh |   12 ++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/setup.c b/setup.c
index 3d73269..10b8f16 100644
--- a/setup.c
+++ b/setup.c
@@ -392,6 +392,13 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
 		set_git_dir(make_absolute_path(gitdirenv));
 		if (chdir(worktree))
 			die_errno("Could not chdir to '%s'", worktree);
+		/*
+		 * Make sure subsequent git processes find correct worktree
+		 * if $GIT_WORK_TREE is set relative
+		 */
+		if (work_tree_env)
+			setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
 		cwd[len++] = '/';
 		cwd[len] = '\0';
 		free(gitfile);
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 1f3b50d..fa35c3e 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -351,4 +351,16 @@ test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
 	)
 '
 
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses (2)' '
+	(
+	cd repo.git/work/sub &&
+	GIT_DIR=../.. &&
+	GIT_WORK_TREE=.. &&
+	export GIT_DIR GIT_WORK_TREE &&
+	test-subprocess rev-parse --show-toplevel >actual &&
+	echo "$TRASH_DIRECTORY/repo.git/work" >expected &&
+	test_cmp expected actual
+	)
+'
+
 test_done
-- 
1.7.3.4.878.g439c7

Re: [PATCH 1/2] t1501: avoid bashisms

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Junio, please don't pick up this patch.

Won't.  And [2/2] needs to be updated with something like this?

 t/t1501-worktree.sh |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 1429a65..8cab065 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -341,14 +341,10 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
 '
 
 test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
-	(
-	GIT_DIR=repo.git
-	GIT_WORK_TREE=repo.git/work
-	export GIT_DIR GIT_WORK_TREE
+	GIT_DIR=repo.git GIT_WORK_TREE=repo.git/work \
 	test-subprocess rev-parse --show-toplevel >actual &&
-	echo "`pwd`/repo.git/work" >expected &&
+	echo "$(pwd)/repo.git/work" >expected &&
 	test_cmp expected actual
-	)
 '
 
 test_done

[PATCH] setup_work_tree: adjust relative $GIT_WORK_TREE after moving cwd

[Nguyễn Thái Ngọc Duy]

When setup_work_tree() is called, it moves cwd to $GIT_WORK_TREE and
makes internal copy of $GIT_WORK_TREE absolute. The environt variable,
if set by user, remains unchanged. If the variable is relative, it is
no longer correct because its base dir has changed.

Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequent git processes handle it.

Reported-by: Michel Briand <michelbriand@free.fr>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Third round. Remove subshell and `pwd` in t1501.31

 .gitignore          |    1 +
 Makefile            |    1 +
 setup.c             |    8 ++++++++
 t/t1501-worktree.sh |    7 +++++++
 test-subprocess.c   |   21 +++++++++++++++++++++
 5 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100644 test-subprocess.c

diff --git a/.gitignore b/.gitignore
index 87b833c..3dd6ef7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
 /test-sha1
 /test-sigchain
 /test-string-pool
+/test-subprocess
 /test-svn-fe
 /test-treap
 /common-cmds.h
diff --git a/Makefile b/Makefile
index 57d9c65..bdf86a3 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-string-pool
+TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-treap
 TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/setup.c b/setup.c
index 91887a4..3833569 100644
--- a/setup.c
+++ b/setup.c
@@ -239,6 +239,14 @@ void setup_work_tree(void)
 		git_dir = make_absolute_path(git_dir);
 	if (!work_tree || chdir(work_tree))
 		die("This operation must be run in a work tree");
+
+	/*
+	 * Make sure subsequent git processes find correct worktree
+	 * if $GIT_WORK_TREE is set relative
+	 */
+	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
+		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
 	set_git_dir(make_relative_path(git_dir, work_tree));
 	initialized = 1;
 }
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..f072a8e 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -340,4 +340,11 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
 	git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
 '
 
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
+	GIT_DIR=repo.git GIT_WORK_TREE=repo.git/work \
+	test-subprocess --setup-work-tree rev-parse --show-toplevel >actual &&
+	echo "$TRASH_DIRECTORY/repo.git/work" >expected &&
+	test_cmp expected actual
+'
+
 test_done
diff --git a/test-subprocess.c b/test-subprocess.c
new file mode 100644
index 0000000..667d3e5
--- /dev/null
+++ b/test-subprocess.c
@@ -0,0 +1,21 @@
+#include "cache.h"
+#include "run-command.h"
+
+int main(int argc, char **argv)
+{
+	const char *prefix;
+	struct child_process cp;
+	int nogit = 0;
+
+	prefix = setup_git_directory_gently(&nogit);
+	if (nogit)
+		die("No git repo found");
+	if (!strcmp(argv[1], "--setup-work-tree")) {
+		setup_work_tree();
+		argv++;
+	}
+	memset(&cp, 0, sizeof(cp));
+	cp.git_cmd = 1;
+	cp.argv = (const char **)argv+1;
+	return run_command(&cp);
+}
-- 
1.7.3.4.878.g439c7