threads / patch / 27461

v2, 3 partssetup: Provide GIT_PREFIX to built-ins

Subject: [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins

## tl;dr

8 messages between May 26, 2011 and May 26, 2011. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

David Aguilar· May 26, 2011, 03:37 UTC · lore

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(-)
Show changes to 2 files +21 −0

setup.c, t/t1020-subdirectory.sh

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
David Aguilar· May 26, 2011, 03:37 UTC · re: David Aguilar · lore

[PATCH v2 2/3] git: Remove handling for GIT_PREFIX

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(-)
Show changes to git.c +1 −9
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
David Aguilar· May 26, 2011, 03:37 UTC · re: David Aguilar · lore

[PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory

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(-)
Show changes to git-mergetool--lib.sh +12 −0
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
David Aguilar· May 26, 2011, 05:05 UTC · re: David Aguilar · lore

Re: [PATCH v2 3/3] git-mergetool--lib: Make vimdiff retain the current directory

On Wed, May 25, 2011 at 8:37 PM, David Aguilar <davvid@gmail.com> wrote:
Show 9 quoted lines
> +
> +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
David Aguilar· May 26, 2011, 05:29 UTC · re: David Aguilar · lore

[PATCH] git-mergetool--lib: Make vimdiff retain the current directory

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(-)
Show changes to git-mergetool--lib.sh +6 −0
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
David Aguilar· May 26, 2011, 06:21 UTC · re: David Aguilar · lore

[PATCH v4 3/3] git-mergetool--lib: Make vimdiff retain the current directory

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(-)
Show changes to git-mergetool--lib.sh +7 −0
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
Junio C Hamano· May 26, 2011, 17:45 UTC · re: David Aguilar · lore

Re: [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins

David Aguilar <davvid@gmail.com> writes:
Show 12 quoted lines
> 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"?
David Aguilar· May 26, 2011, 21:49 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/3] setup: Provide GIT_PREFIX to built-ins

On Thu, May 26, 2011 at 10:45:35AM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> 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

← back to recent threads