threads / rfc / 27087

RFC patchhandle_alias: provide GIT_CWD to !alias

Subject: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

## tl;dr

12 messages between Apr 13, 2011 and Apr 27, 2011. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Michael J Gruber· Apr 13, 2011, 13:05 UTC · lore

Provide an environment variable GIT_CWD which contains the subdirectory from which a !alias was called since these cd to the to level directory before they are executed.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
RFC for lack of tests and doc.

I would say this is something realistic and useful which can be done now, no matter how our efforts with !(top) and the like go.

 git.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Show changes to git.c +9 −1
diff --git a/git.c b/git.c
index ef598c3..c8b9189 100644
--- a/git.c
+++ b/git.c
@@ -179,6 +179,8 @@ 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();
 
@@ -189,7 +191,13 @@ static int handle_alias(int *argcp, const char ***argv)
 				alias_argv[i] = (*argv)[i];
 			alias_argv[argc] = NULL;
 
-			ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
+			strbuf_addstr(&sb, "GIT_CWD=");
+			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);
 			if (ret >= 0)   /* normal exit */
 				exit(ret);
 

-- 
1.7.5.rc0.365.g23929a
Nguyen Thai Ngoc Duy· Apr 13, 2011, 13:12 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 10 quoted lines
> -                       ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
> +                       strbuf_addstr(&sb, "GIT_CWD=");
> +                       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);
>                        if (ret >= 0)   /* normal exit */
>                                exit(ret);

subdir can be NULL. I'm not sure if it can be empty string though. May need a check and set it to '.' so "cd $GIT_CWD" does not go back to $HOME.

-- 
Duy
Michael J Gruber· Apr 13, 2011, 13:16 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

Nguyen Thai Ngoc Duy venit, vidit, dixit 13.04.2011 15:12:
Show 14 quoted lines
> On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> -                       ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
>> +                       strbuf_addstr(&sb, "GIT_CWD=");
>> +                       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);
>>                        if (ret >= 0)   /* normal exit */
>>                                exit(ret);
> 
> subdir can be NULL. I'm not sure if it can be empty string though. May
I'm pretty sure that my "if (subdir)" would catch that...
> need a check and set it to '.' so "cd $GIT_CWD" does not go back to
> $HOME.

Well, with the current implementation you get empty or the subdir so that you can easily "test -n $GIT_CWD". If you want to cd around you can do "cd ./$GIT_CWD" unconditionally. I think this is more useful than having a "." there.

Michael
Nguyen Thai Ngoc Duy· Apr 13, 2011, 13:24 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

On Wed, Apr 13, 2011 at 8:16 PM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 17 quoted lines
> Nguyen Thai Ngoc Duy venit, vidit, dixit 13.04.2011 15:12:
>> On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber
>> <git@drmicha.warpmail.net> wrote:
>>> -                       ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
>>> +                       strbuf_addstr(&sb, "GIT_CWD=");
>>> +                       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);
>>>                        if (ret >= 0)   /* normal exit */
>>>                                exit(ret);
>>
>> subdir can be NULL. I'm not sure if it can be empty string though. May
>
> I'm pretty sure that my "if (subdir)" would catch that...
Yes. I skimmed too quick and realized after sending my email.
Show 7 quoted lines
>> need a check and set it to '.' so "cd $GIT_CWD" does not go back to
>> $HOME.
>
> Well, with the current implementation you get empty or the subdir so
> that you can easily "test -n $GIT_CWD". If you want to cd around you can
> do "cd ./$GIT_CWD" unconditionally. I think this is more useful than
> having a "." there.

In that case should the variable have a another name? CWD/PWD is usually absolute path. "cd ./$GIT_CWD" looks odd to me. Or you can getcwd() and set $OLDPWD.

-- 
Duy
Junio C Hamano· Apr 26, 2011, 22:22 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

Michael J Gruber <git@drmicha.warpmail.net> writes:
> Provide an environment variable GIT_CWD which contains the subdirectory
> from which a !alias was called since these cd to the to level directory
> before they are executed.
Isn't this what we usually call "prefix", though?
Michael J Gruber· Apr 27, 2011, 06:47 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

Junio C Hamano venit, vidit, dixit 27.04.2011 00:22:
Show 7 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Provide an environment variable GIT_CWD which contains the subdirectory
>> from which a !alias was called since these cd to the to level directory
>> before they are executed.
> 
> Isn't this what we usually call "prefix", though?

In code it is called "prefix". But I have to ask: So what? Are you suggesting GIT_PREFIX? User expectation is probably more tied to CWD, unless a user knows "rev-parse --show-prefix". I don't care about the name, and GIT_PREFIX may be more systematic.

A more important point is the prefix-less case (i.e. we are in topdir): Should GIT_YOUNAMEIT be empty then? I would say yes (just like "rev-parse --show-prefix"), Duy said no. We need a third vote :)

Also, we may want to scrap this altogether and do !!-aliases instead (without chdir).

Michael
Nguyen Thai Ngoc Duy· Apr 27, 2011, 07:40 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

On Wed, Apr 27, 2011 at 1:47 PM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> A more important point is the prefix-less case (i.e. we are in topdir):
> Should GIT_YOUNAMEIT be empty then? I would say yes (just like
> "rev-parse --show-prefix"), Duy said no. We need a third vote :)

I said no because you named it GIT_CWD. If it's GIT_PREFIX, then empty prefix is normal, no objections.

-- 
Duy
Michael J Gruber· Apr 27, 2011, 08:10 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias

Nguyen Thai Ngoc Duy venit, vidit, dixit 27.04.2011 09:40:
Show 8 quoted lines
> On Wed, Apr 27, 2011 at 1:47 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> A more important point is the prefix-less case (i.e. we are in topdir):
>> Should GIT_YOUNAMEIT be empty then? I would say yes (just like
>> "rev-parse --show-prefix"), Duy said no. We need a third vote :)
> 
> I said no because you named it GIT_CWD. If it's GIT_PREFIX, then empty
> prefix is normal, no objections.
We have a winner then! Patch upcoming.
Michael
Michael J Gruber· Apr 27, 2011, 08:36 UTC · re: Michael J Gruber · lore

[PATCH 1/2] t1020: test !alias in subdirectory

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t1020-subdirectory.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Show changes to t/t1020-subdirectory.sh +11 −0
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index 1fd187c..66e40d3 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -118,6 +118,17 @@ test_expect_success 'alias expansion' '
 		git ss
 	)
 '
+
+test_expect_success '!alias expansion' '
+	pwd >expect &&
+	(
+		git config alias.test !pwd &&
+		cd dir &&
+		git test >../actual
+	) &&
+	test_cmp expect actual
+'
+
 test_expect_success 'no file/rev ambiguity check inside .git' '
 	git commit -a -m 1 &&
 	(

-- 
1.7.5.270.gafca7
Michael J Gruber· Apr 27, 2011, 08:36 UTC · re: Michael J Gruber · lore

[PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias

Provide an environment variable GIT_PREFIX which contains the subdirectory from which a !alias was called (i.e. 'git rev-parse --show-prefix') since these cd to the to level directory before they are executed.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/config.txt |    2 ++
 git.c                    |   10 +++++++++-
 t/t1020-subdirectory.sh  |   10 ++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)
Show changes to 3 files +21 −1

Documentation/config.txt, git.c, t/t1020-subdirectory.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 750c86d..66281dd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -589,6 +589,8 @@ it will be treated as a shell command.  For example, defining
 "gitk --all --not ORIG_HEAD".  Note that shell commands will be
 executed from the top-level directory of a repository, which may
 not necessarily be the current directory.
+'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'
+from the original current directory. See linkgit:git-rev-parse[1].
 
 am.keepcr::
 	If true, git-am will call git-mailsplit for patches in mbox format
diff --git a/git.c b/git.c
index ef598c3..ed89951 100644
--- a/git.c
+++ b/git.c
@@ -179,6 +179,8 @@ 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();
 
@@ -189,7 +191,13 @@ static int handle_alias(int *argcp, const char ***argv)
 				alias_argv[i] = (*argv)[i];
 			alias_argv[argc] = NULL;
 
-			ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
+			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);
 			if (ret >= 0)   /* normal exit */
 				exit(ret);
 
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index 66e40d3..ddc3921 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -129,6 +129,16 @@ test_expect_success '!alias expansion' '
 	test_cmp expect actual
 '
 
+test_expect_success 'GIT_PREFIX for !alias' '
+	printf "dir/" >expect &&
+	(
+		git config alias.test "!sh -c \"printf \$GIT_PREFIX\"" &&
+		cd dir &&
+		git test >../actual
+	) &&
+	test_cmp expect actual
+'
+
 test_expect_success 'no file/rev ambiguity check inside .git' '
 	git commit -a -m 1 &&
 	(

-- 
1.7.5.270.gafca7
Nguyen Thai Ngoc Duy· Apr 27, 2011, 09:06 UTC · re: Michael J Gruber · lore

Re: [PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias

2011/4/27 Michael J Gruber <git@drmicha.warpmail.net>:
Show 6 quoted lines
> @@ -589,6 +589,8 @@ it will be treated as a shell command.  For example, defining
>  "gitk --all --not ORIG_HEAD".  Note that shell commands will be
>  executed from the top-level directory of a repository, which may
>  not necessarily be the current directory.
> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'
> +from the original current directory. See linkgit:git-rev-parse[1].

It made me wonder "but then why should not we just do GIT_PREFIX=`git rev-parse --show-prefix` in the alias?" then realized by the time we do that, it's already too late. Maybe

> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'
> +before current directory is changed to top-level directory. See linkgit:git-rev-parse[1].
?
-- 
Duy
Michael J Gruber· Apr 27, 2011, 09:36 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias

Nguyen Thai Ngoc Duy venit, vidit, dixit 27.04.2011 11:06:
Show 16 quoted lines
> 2011/4/27 Michael J Gruber <git@drmicha.warpmail.net>:
>> @@ -589,6 +589,8 @@ it will be treated as a shell command.  For example, defining
>>  "gitk --all --not ORIG_HEAD".  Note that shell commands will be
>>  executed from the top-level directory of a repository, which may
>>  not necessarily be the current directory.
>> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'
>> +from the original current directory. See linkgit:git-rev-parse[1].
> 
> It made me wonder "but then why should not we just do GIT_PREFIX=`git
> rev-parse --show-prefix` in the alias?" then realized by the time we
> do that, it's already too late. Maybe
> 
>> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'
>> +before current directory is changed to top-level directory. See linkgit:git-rev-parse[1].
> 
> ?

Well, the text says already that the alias is not run from the "original current directory", so it's two ways of saying the same thing.

Michael

← back to recent threads