git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins
@ 2011-05-26  3:37 David Aguilar
  2011-05-26  3:37 ` [PATCH v2 2/3] git: Remove handling for GIT_PREFIX David Aguilar
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26  3:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that
aliases can know the directory from which a !alias was called.

Knowing the prefix relative to the root is helpful in other programs
so export it to built-ins as well.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

The original version of this patch did not set GIT_PREFIX
to an empty string when prefix returns NULL.
Now we do.

 setup.c                 |    5 +++++
 t/t1020-subdirectory.sh |   16 ++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/setup.c b/setup.c
index 013ad11..4569974 100644
--- a/setup.c
+++ b/setup.c
@@ -710,6 +710,11 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	const char *prefix;
 
 	prefix = setup_git_directory_gently_1(nongit_ok);
+	if (prefix)
+		setenv("GIT_PREFIX", prefix, 1);
+	else
+		setenv("GIT_PREFIX", "", 1);
+
 	if (startup_info) {
 		startup_info->have_repository = !nongit_ok || !*nongit_ok;
 		startup_info->prefix = prefix;
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index ddc3921..3c74480 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -139,6 +139,22 @@ test_expect_success 'GIT_PREFIX for !alias' '
 	test_cmp expect actual
 '
 
+test_expect_success 'GIT_PREFIX for built-ins' '
+	# Use GIT_EXTERNAL_DIFF to test that the "diff" built-in
+	# receives the GIT_PREFIX variable.
+	printf "dir/" >expect &&
+	printf "#!/bin/sh\n" >diff &&
+	printf "printf \"\$GIT_PREFIX\"" >>diff &&
+	chmod +x diff &&
+	(
+		cd dir &&
+		printf "change" >two &&
+		env GIT_EXTERNAL_DIFF=./diff git diff >../actual
+		git checkout -- two
+	) &&
+	test_cmp expect actual
+'
+
 test_expect_success 'no file/rev ambiguity check inside .git' '
 	git commit -a -m 1 &&
 	(
-- 
1.7.5.2.660.g9f46

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

* [PATCH v2 2/3] git: Remove handling for GIT_PREFIX
  2011-05-26  3:37 [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins David Aguilar
@ 2011-05-26  3:37 ` David Aguilar
  2011-05-26  3:37 ` [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory David Aguilar
  2011-05-26 17:45 ` [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins Junio C Hamano
  2 siblings, 0 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26  3:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

handle_alias() no longer needs to set GIT_PREFIX since it is defined
in setup_git_directory_gently().  Remove the duplicated effort and use
run_command_v_opt() since there is no need to setup the environment.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

This patch is unchanged.

 git.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/git.c b/git.c
index 89721d4..8828c18 100644
--- a/git.c
+++ b/git.c
@@ -183,8 +183,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		if (alias_string[0] == '!') {
 			const char **alias_argv;
 			int argc = *argcp, i;
-			struct strbuf sb = STRBUF_INIT;
-			const char *env[2];
 
 			commit_pager_choice();
 
@@ -195,13 +193,7 @@ static int handle_alias(int *argcp, const char ***argv)
 				alias_argv[i] = (*argv)[i];
 			alias_argv[argc] = NULL;
 
-			strbuf_addstr(&sb, "GIT_PREFIX=");
-			if (subdir)
-				strbuf_addstr(&sb, subdir);
-			env[0] = sb.buf;
-			env[1] = NULL;
-			ret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);
-			strbuf_release(&sb);
+			ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
 			if (ret >= 0)   /* normal exit */
 				exit(ret);
 
-- 
1.7.5.2.660.g9f46

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

* [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory
  2011-05-26  3:37 [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins David Aguilar
  2011-05-26  3:37 ` [PATCH v2 2/3] git: Remove handling for GIT_PREFIX David Aguilar
@ 2011-05-26  3:37 ` David Aguilar
  2011-05-26  5:05   ` David Aguilar
  2011-05-26  5:29   ` [PATCH] " David Aguilar
  2011-05-26 17:45 ` [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins Junio C Hamano
  2 siblings, 2 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26  3:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

When using difftool with vimdiff it can be unexpected that
the current directory changes to the root of the project.
Tell vim to chdir to the value of $GIT_PREFIX to fix this.

Care is taken to quote the variable so that vim expands it.
This avoids problems when directory names contain spaces.

Signed-off-by: David Aguilar <davvid@gmail.com>
Reported-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---

This version of the patch resolves GIT_PREFIX unconditionally.
We did not do so before.

 git-mergetool--lib.sh |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4db9212..2cef8a2 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -89,6 +89,7 @@ run_merge_tool () {
 	merge_tool_path="$(get_merge_tool_path "$1")" || exit
 	base_present="$2"
 	status=0
+	resolve_git_prefix
 
 	case "$1" in
 	araxis)
@@ -188,6 +189,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
@@ -199,6 +201,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
@@ -437,3 +440,12 @@ get_merge_tool () {
 	fi
 	echo "$merge_tool"
 }
+
+resolve_git_prefix() {
+	# If GIT_PREFIX is empty then we cannot use it in tools
+	# that expect to be able to chdir() to its value.
+	if test -z "$GIT_PREFIX"; then
+		GIT_PREFIX=.
+		export GIT_PREFIX
+	fi
+}
-- 
1.7.5.2.660.g9f46

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

* Re: [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory
  2011-05-26  3:37 ` [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory David Aguilar
@ 2011-05-26  5:05   ` David Aguilar
  2011-05-26  5:29   ` [PATCH] " David Aguilar
  1 sibling, 0 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26  5:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

On Wed, May 25, 2011 at 8:37 PM, David Aguilar <davvid@gmail.com> wrote:
> +
> +resolve_git_prefix() {
> +       # If GIT_PREFIX is empty then we cannot use it in tools
> +       # that expect to be able to chdir() to its value.
> +       if test -z "$GIT_PREFIX"; then
> +               GIT_PREFIX=.
> +               export GIT_PREFIX
> +       fi
> +}

Oops, I forgot to replace the function call with : GIT_PREFIX=${GIT_PREFIX:-.}
-- 
    David

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

* [PATCH] git-mergetool--lib: Make vimdiff retain the current directory
  2011-05-26  3:37 ` [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory David Aguilar
  2011-05-26  5:05   ` David Aguilar
@ 2011-05-26  5:29   ` David Aguilar
  2011-05-26  6:21     ` [PATCH v4 3/3] " David Aguilar
  1 sibling, 1 reply; 8+ messages in thread
From: David Aguilar @ 2011-05-26  5:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason, git

When using difftool with vimdiff it can be unexpected that
the current directory changes to the root of the project.
Tell vim to chdir to the value of $GIT_PREFIX to fix this.

Care is taken to quote the variable so that vim expands it.
This avoids problems when directory names contain spaces.

Signed-off-by: David Aguilar <davvid@gmail.com>
Reported-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
Replacement for the last patch: uses ${GIT_PREFIX:-.}

 git-mergetool--lib.sh |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4db9212..43a9a28 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -86,6 +86,10 @@ get_merge_tool_cmd () {
 }
 
 run_merge_tool () {
+	# If GIT_PREFIX is empty then we cannot use it in tools
+	# that expect to be able to chdir() to its value.
+	: GIT_PREFIX=${GIT_PREFIX:-.}
+
 	merge_tool_path="$(get_merge_tool_path "$1")" || exit
 	base_present="$2"
 	status=0
@@ -188,6 +192,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
@@ -199,6 +204,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
-- 
1.7.5.2.660.g9f46c

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

* [PATCH v4 3/3] git-mergetool--lib: Make vimdiff retain the current directory
  2011-05-26  5:29   ` [PATCH] " David Aguilar
@ 2011-05-26  6:21     ` David Aguilar
  0 siblings, 0 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26  6:21 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

When using difftool with vimdiff it can be unexpected that
the current directory changes to the root of the project.
Tell vim to chdir to the value of $GIT_PREFIX to fix this.

Care is taken to quote the variable so that vim expands it.
This avoids problems when directory names contain spaces.

Signed-off-by: David Aguilar <davvid@gmail.com>
Reported-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
How embarassing.  I'm going to bed now.  This one works.

 git-mergetool--lib.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4db9212..91f90ac 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -86,6 +86,11 @@ get_merge_tool_cmd () {
 }
 
 run_merge_tool () {
+	# If GIT_PREFIX is empty then we cannot use it in tools
+	# that expect to be able to chdir() to its value.
+	GIT_PREFIX=${GIT_PREFIX:-.}
+	export GIT_PREFIX
+
 	merge_tool_path="$(get_merge_tool_path "$1")" || exit
 	base_present="$2"
 	status=0
@@ -188,6 +193,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
@@ -199,6 +205,7 @@ run_merge_tool () {
 			check_unchanged
 		else
 			"$merge_tool_path" -R -f -d -c "wincmd l" \
+				-c 'cd $GIT_PREFIX' \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
-- 
1.7.5.2.663.gfcb11.dirty

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

* Re: [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins
  2011-05-26  3:37 [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins David Aguilar
  2011-05-26  3:37 ` [PATCH v2 2/3] git: Remove handling for GIT_PREFIX David Aguilar
  2011-05-26  3:37 ` [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory David Aguilar
@ 2011-05-26 17:45 ` Junio C Hamano
  2011-05-26 21:49   ` David Aguilar
  2 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2011-05-26 17:45 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, git, Frédéric Heitzmann,
	Michael J Gruber, Ævar Arnfjörð Bjarmason

David Aguilar <davvid@gmail.com> writes:

> GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that
> aliases can know the directory from which a !alias was called.
>
> Knowing the prefix relative to the root is helpful in other programs
> so export it to built-ins as well.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>
> The original version of this patch did not set GIT_PREFIX
> to an empty string when prefix returns NULL.
> Now we do.

Thanks.  Should I add "Helped-by: MJG"?

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

* Re: [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins
  2011-05-26 17:45 ` [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins Junio C Hamano
@ 2011-05-26 21:49   ` David Aguilar
  0 siblings, 0 replies; 8+ messages in thread
From: David Aguilar @ 2011-05-26 21:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Frédéric Heitzmann, Michael J Gruber,
	Ævar Arnfjörð Bjarmason

On Thu, May 26, 2011 at 10:45:35AM -0700, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that
> > aliases can know the directory from which a !alias was called.
> >
> > Knowing the prefix relative to the root is helpful in other programs
> > so export it to built-ins as well.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> >
> > The original version of this patch did not set GIT_PREFIX
> > to an empty string when prefix returns NULL.
> > Now we do.
> 
> Thanks.  Should I add "Helped-by: MJG"?

Yes, that'd be great.
-- 
					David

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

end of thread, other threads:[~2011-05-26 21:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-26  3:37 [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins David Aguilar
2011-05-26  3:37 ` [PATCH v2 2/3] git: Remove handling for GIT_PREFIX David Aguilar
2011-05-26  3:37 ` [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory David Aguilar
2011-05-26  5:05   ` David Aguilar
2011-05-26  5:29   ` [PATCH] " David Aguilar
2011-05-26  6:21     ` [PATCH v4 3/3] " David Aguilar
2011-05-26 17:45 ` [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins Junio C Hamano
2011-05-26 21:49   ` David Aguilar

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 NNTP newsgroup(s).