git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* GIT_DIR vs. --git-dir
@ 2012-09-24  7:19 Michael J Gruber
  2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
  2012-09-24 14:49 ` Jeff King
  0 siblings, 2 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24  7:19 UTC (permalink / raw)
  To: Git Mailing List

[mjg@localhost ~]$ GIT_DIR=~/.githome git rev-parse --show-toplevel
/home/mjg

[mjg@localhost ~]$ git --git-dir=~/.githome rev-parse --show-toplevel
fatal: Not a git repository: '~/.githome'

Huh? Ok, so most users probably would not try further and blame git, but:

[mjg@localhost ~]$ git --git-dir=/home/mjg/.githome rev-parse
--show-toplevel
/home/mjg

(All this is with core.worktree set to /home/mjg.)

So, while I do understand that we don't expand '~' in any of these cases
and it's only a matter of bash tilde expansion kicking in or not, we
might want to do something about it. (--git-dir=$HOME/.githome gets
expanded, as well, and --git-dir=.githome works from the appropriate cwd
only).

Michael

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  7:19 GIT_DIR vs. --git-dir Michael J Gruber
@ 2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
  2012-09-24  7:57   ` Michael J Gruber
  2012-09-24 14:36   ` Junio C Hamano
  2012-09-24 14:49 ` Jeff King
  1 sibling, 2 replies; 17+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2012-09-24  7:41 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List

On Mon, Sep 24, 2012 at 2:19 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> [mjg@localhost ~]$ GIT_DIR=~/.githome git rev-parse --show-toplevel
> /home/mjg
>
> [mjg@localhost ~]$ git --git-dir=~/.githome rev-parse --show-toplevel
> fatal: Not a git repository: '~/.githome'
>
> Huh?

The message looks pretty clear to me that ~ is not expanded.

> Ok, so most users probably would not try further and blame git, but:
>
> [mjg@localhost ~]$ git --git-dir=/home/mjg/.githome rev-parse
> --show-toplevel
> /home/mjg
>
> (All this is with core.worktree set to /home/mjg.)
>
> So, while I do understand that we don't expand '~' in any of these cases
> and it's only a matter of bash tilde expansion kicking in or not, we
> might want to do something about it. (--git-dir=$HOME/.githome gets
> expanded, as well, and --git-dir=.githome works from the appropriate cwd
> only).

"~" is a shell feature. Know your shell. If we make an exception for
--git-dir, we might have to support --blahblah=~/somewhere. That's a
lot of changes and we might mistakenly over-expand something. Users
running git on cmd.exe may get surprised that "~" is expanded. We
could print an advice "did you mean $HOME/.githome?". That could still
be a lot of changes, but it's no-op so less worries of breaking
something. I prefer doing nothing in this case.
-- 
Duy

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
@ 2012-09-24  7:57   ` Michael J Gruber
  2012-09-24  9:53     ` Nguyen Thai Ngoc Duy
  2012-09-24 14:36   ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24  7:57 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List

Nguyen Thai Ngoc Duy venit, vidit, dixit 24.09.2012 09:41:
> On Mon, Sep 24, 2012 at 2:19 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> [mjg@localhost ~]$ GIT_DIR=~/.githome git rev-parse --show-toplevel
>> /home/mjg
>>
>> [mjg@localhost ~]$ git --git-dir=~/.githome rev-parse --show-toplevel
>> fatal: Not a git repository: '~/.githome'
>>
>> Huh?
> 
> The message looks pretty clear to me that ~ is not expanded.

...to you and me, of course, but the point is whether we can be more
helpful to most users.

> 
>> Ok, so most users probably would not try further and blame git, but:
>>
>> [mjg@localhost ~]$ git --git-dir=/home/mjg/.githome rev-parse
>> --show-toplevel
>> /home/mjg
>>
>> (All this is with core.worktree set to /home/mjg.)
>>
>> So, while I do understand that we don't expand '~' in any of these cases
>> and it's only a matter of bash tilde expansion kicking in or not, we
>> might want to do something about it. (--git-dir=$HOME/.githome gets
>> expanded, as well, and --git-dir=.githome works from the appropriate cwd
>> only).
> 
> "~" is a shell feature. Know your shell. If we make an exception for
> --git-dir, we might have to support --blahblah=~/somewhere. That's a
> lot of changes and we might mistakenly over-expand something. Users
> running git on cmd.exe may get surprised that "~" is expanded. We
> could print an advice "did you mean $HOME/.githome?". That could still
> be a lot of changes, but it's no-op so less worries of breaking
> something. I prefer doing nothing in this case.

First of all, we expand '~' in other cases (basically all config vars)
ta) where it's unambiguously a path. And --git-dir really is the
sensible version of core.gitdir (which would make no sense, of course).

Then, "we" refuse to make "~2" a shorthand for "HEAD~2" in the revision
parser because "~2" has a meaning for certain shells in certain
situations (depending on your dirstack) and would need to be quoted, so
we certainly take into account how the shell behaves.

It might be difficult to implement, but I'm sorry I can't follow the
argumentation above at all; it's not based on what we do in other places
and other cases.

Michael

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  7:57   ` Michael J Gruber
@ 2012-09-24  9:53     ` Nguyen Thai Ngoc Duy
  2012-09-24 12:56       ` Michael J Gruber
  0 siblings, 1 reply; 17+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2012-09-24  9:53 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List

On Mon, Sep 24, 2012 at 2:57 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> It might be difficult to implement, but I'm sorry I can't follow the
> argumentation above at all; it's not based on what we do in other places
> and other cases.

My point is, what's so special about --git-dir? what about
--work-tree, or "commit -F path"? It's hard to draw a line there.
Config files are a special case, because git is the only one that
reads the file. "~" expansion depends on shell setting. If users turn
it off, they may be surprised that "~foo" is turned to /home/foo while
they really mean "~foo". Warning is the only sensible thing we could
do.
-- 
Duy

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  9:53     ` Nguyen Thai Ngoc Duy
@ 2012-09-24 12:56       ` Michael J Gruber
  2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
  2012-09-24 13:37         ` GIT_DIR vs. --git-dir Andreas Schwab
  0 siblings, 2 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24 12:56 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List

Nguyen Thai Ngoc Duy venit, vidit, dixit 24.09.2012 11:53:
> On Mon, Sep 24, 2012 at 2:57 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> It might be difficult to implement, but I'm sorry I can't follow the
>> argumentation above at all; it's not based on what we do in other places
>> and other cases.
> 
> My point is, what's so special about --git-dir? what about
> --work-tree, or "commit -F path"? It's hard to draw a line there.

Sure those should follow, especially work-tree.

> Config files are a special case, because git is the only one that
> reads the file. "~" expansion depends on shell setting. If users turn
> it off, they may be surprised that "~foo" is turned to /home/foo while
> they really mean "~foo". Warning is the only sensible thing we could
> do.

But that argument applies to config files in exactly the same way as it
applies to command line arguments. Git is the only one reading them. So
why not leave it up to Git to decide about expansion?

On the other hand: If Git expanding arguments is surprising, why is it
unsurprising if Git expands config values?

You know the implementation, so there are no surprises for you. But once
in a while we can pretend to design Git for those who just use it.

Michael

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

* [RFC/PATCH] git: expand user path in --git-dir
  2012-09-24 12:56       ` Michael J Gruber
@ 2012-09-24 12:57         ` Michael J Gruber
  2012-09-24 14:52           ` Jeff King
                             ` (2 more replies)
  2012-09-24 13:37         ` GIT_DIR vs. --git-dir Andreas Schwab
  1 sibling, 3 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24 12:57 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Currently, all paths in the config file are subject to tilde expansion
for user paths while the argument to --git-dir is not expanded, and
neither are paths in the environment such as GIT_DIR. From the user
perspective, though, the two commands

GIT_DIR=~user/foo git command
git --git-dir=~user/foo command

currently behave differently because in the first case the shell would
perform tilde expansion, but not in the second. Also, one may argue that
specifying '--git-dir=' is like specifying a config variable (which
cannot exist for this purpose).

Thus, make arguments to '--git-dir' undergo tilde expansion.
---
So, here's a simple patch implementing tilde expansion for --git-dir. It passes
all tests. It's done doing the expansion on the setting side.

Alternatively, one might do it on the getting side, i.e. when reading GIT_DIR,
so that paths passed directly through the environment undergo tilde expansion
as well. We don't do this for any environment variable yet, so I didn't go
that far.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 git.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 8788b32..35e8011 100644
--- a/git.c
+++ b/git.c
@@ -64,6 +64,22 @@ static void commit_pager_choice(void) {
 	}
 }
 
+static int expand_path_setenv(const char *name, const char *value, int overwrite)
+{
+	int ret;
+	const char *expanded;
+
+	if (!value)
+		return setenv(name, value, overwrite);
+
+	expanded = expand_user_path(value);
+	if (!expanded)
+		die("Failed to expand user dir in: '%s'", value);
+	ret = setenv(name, expanded, overwrite);
+	free((void *)expanded);
+	return ret;
+}
+
 static int handle_options(const char ***argv, int *argc, int *envchanged)
 {
 	const char **orig_argv = *argv;
@@ -117,13 +133,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				fprintf(stderr, "No directory given for --git-dir.\n" );
 				usage(git_usage_string);
 			}
-			setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
+			expand_path_setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
 			if (envchanged)
 				*envchanged = 1;
 			(*argv)++;
 			(*argc)--;
 		} else if (!prefixcmp(cmd, "--git-dir=")) {
-			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
+			expand_path_setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--namespace")) {
-- 
1.7.12.1.580.gb9ed689

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24 12:56       ` Michael J Gruber
  2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
@ 2012-09-24 13:37         ` Andreas Schwab
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Schwab @ 2012-09-24 13:37 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List

Michael J Gruber <git@drmicha.warpmail.net> writes:

> But that argument applies to config files in exactly the same way as it
> applies to command line arguments. Git is the only one reading them. So
> why not leave it up to Git to decide about expansion?

Command line arguments are first processed by the shell, handling
expansions like $HOME.  It is part of the Unix philosophy that expansion
of command lines is left to the shell.

> On the other hand: If Git expanding arguments is surprising, why is it
> unsurprising if Git expands config values?

The config file is not processed by the shell.

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."

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
  2012-09-24  7:57   ` Michael J Gruber
@ 2012-09-24 14:36   ` Junio C Hamano
  2012-09-24 14:51     ` Michael J Gruber
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2012-09-24 14:36 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Michael J Gruber, Git Mailing List

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

> "~" is a shell feature. Know your shell. If we make an exception for
> --git-dir, we might have to support --blahblah=~/somewhere.

Correct but not entirely true.

When we know --git-dir=<path> must name a path, we should be able to
do better.  See OPT_FILENAME in >parse-optios.h>, for inspiration.

MJG's patch later in this thread is conceptually OK but I do not
think it should introduce a "expand and then setenv" helper that
won't be useful unless the variable is GIT_DIR.  That pattern does
not appear that often, and smells like a bad API design taste.

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24  7:19 GIT_DIR vs. --git-dir Michael J Gruber
  2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
@ 2012-09-24 14:49 ` Jeff King
  2012-09-24 14:54   ` Michael J Gruber
  2012-09-24 15:42   ` Andreas Schwab
  1 sibling, 2 replies; 17+ messages in thread
From: Jeff King @ 2012-09-24 14:49 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List

On Mon, Sep 24, 2012 at 09:19:27AM +0200, Michael J Gruber wrote:

> [mjg@localhost ~]$ GIT_DIR=~/.githome git rev-parse --show-toplevel
> /home/mjg
> 
> [mjg@localhost ~]$ git --git-dir=~/.githome rev-parse --show-toplevel
> fatal: Not a git repository: '~/.githome'
> 
> Huh? Ok, so most users probably would not try further and blame git, but:
> 
> [mjg@localhost ~]$ git --git-dir=/home/mjg/.githome rev-parse
> --show-toplevel
> /home/mjg
> 
> (All this is with core.worktree set to /home/mjg.)
> 
> So, while I do understand that we don't expand '~' in any of these cases
> and it's only a matter of bash tilde expansion kicking in or not, we
> might want to do something about it. (--git-dir=$HOME/.githome gets
> expanded, as well, and --git-dir=.githome works from the appropriate cwd
> only).

Bash is even weirder than you might think. Try this:

  $ echo ~/foo
  /home/peff/foo

  $ echo arg=~/foo
  arg=/home/peff/foo

  $ echo --arg=~/foo
  --arg=~/foo

That is, it expands on the right-hand side of an "=" (which, from my
reading of the bash manual, means it considers it a word split), but
refuses to expand after an "=" that is part of a long option.

The first one is definitely correct. It would be convenient for your use
case to expand the third one, and it logically follows from the second
one. However, dash does not expand the second one. I'm not sure if this
is a bug in bash, or simply a grey area where the two shells do not
agree.

But it makes me wonder if the world would be better served by a bash
option to always enable tilde expansion after an "=". That would solve
your issue, and it would make the same feature work for every other git
long option, as well as for other programs.

-Peff

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24 14:36   ` Junio C Hamano
@ 2012-09-24 14:51     ` Michael J Gruber
  0 siblings, 0 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24 14:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List

Junio C Hamano venit, vidit, dixit 24.09.2012 16:36:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> 
>> "~" is a shell feature. Know your shell. If we make an exception for
>> --git-dir, we might have to support --blahblah=~/somewhere.
> 
> Correct but not entirely true.
> 
> When we know --git-dir=<path> must name a path, we should be able to
> do better.  See OPT_FILENAME in >parse-optios.h>, for inspiration.
> 
> MJG's patch later in this thread is conceptually OK but I do not
> think it should introduce a "expand and then setenv" helper that
> won't be useful unless the variable is GIT_DIR.  That pattern does
> not appear that often, and smells like a bad API design taste.

I can't quite parse. My little helper can be used for any path
environment variable, not just GIT_DIR. Granted, there aren't that many
in use.

Do you suggest tilde expansion right in fix_filename() (i.e. for all
OPT_FILE options), or some OPT_FILENAME_EXPANDED which may or may not be
used by some config? There's git_config_pathname() already which does
expansion, of course.

Michael

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

* Re: [RFC/PATCH] git: expand user path in --git-dir
  2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
@ 2012-09-24 14:52           ` Jeff King
  2012-09-24 14:57             ` Michael J Gruber
  2012-09-24 17:30           ` Junio C Hamano
  2012-09-25  5:33           ` Jan Engelhardt
  2 siblings, 1 reply; 17+ messages in thread
From: Jeff King @ 2012-09-24 14:52 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Nguyễn Thái Ngọc Duy

On Mon, Sep 24, 2012 at 02:57:20PM +0200, Michael J Gruber wrote:

> Currently, all paths in the config file are subject to tilde expansion
> for user paths while the argument to --git-dir is not expanded, and
> neither are paths in the environment such as GIT_DIR. From the user
> perspective, though, the two commands
> 
> GIT_DIR=~user/foo git command
> git --git-dir=~user/foo command
> 
> currently behave differently because in the first case the shell would
> perform tilde expansion, but not in the second. Also, one may argue that
> specifying '--git-dir=' is like specifying a config variable (which
> cannot exist for this purpose).
> 
> Thus, make arguments to '--git-dir' undergo tilde expansion.
> ---
> So, here's a simple patch implementing tilde expansion for --git-dir. It passes
> all tests. It's done doing the expansion on the setting side.
> 
> Alternatively, one might do it on the getting side, i.e. when reading GIT_DIR,
> so that paths passed directly through the environment undergo tilde expansion
> as well. We don't do this for any environment variable yet, so I didn't go
> that far.

Keep in mind that every layer of expansion you add is a layer of quoting
somebody else must do in order to pass through certain paths.  I will
admit that putting a "~" into a path is relatively uncommon, but this
patch is a regression for anybody who does so with --git-dir (they will
now need to quote the "~" against not just their shell, but against
git). Expanding environment variables like GIT_DIR means that we would
also need to quote things we put into GIT_DIR.

-Peff

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24 14:49 ` Jeff King
@ 2012-09-24 14:54   ` Michael J Gruber
  2012-09-24 15:42   ` Andreas Schwab
  1 sibling, 0 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24 14:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List

Jeff King venit, vidit, dixit 24.09.2012 16:49:
> On Mon, Sep 24, 2012 at 09:19:27AM +0200, Michael J Gruber wrote:
> 
>> [mjg@localhost ~]$ GIT_DIR=~/.githome git rev-parse --show-toplevel
>> /home/mjg
>>
>> [mjg@localhost ~]$ git --git-dir=~/.githome rev-parse --show-toplevel
>> fatal: Not a git repository: '~/.githome'
>>
>> Huh? Ok, so most users probably would not try further and blame git, but:
>>
>> [mjg@localhost ~]$ git --git-dir=/home/mjg/.githome rev-parse
>> --show-toplevel
>> /home/mjg
>>
>> (All this is with core.worktree set to /home/mjg.)
>>
>> So, while I do understand that we don't expand '~' in any of these cases
>> and it's only a matter of bash tilde expansion kicking in or not, we
>> might want to do something about it. (--git-dir=$HOME/.githome gets
>> expanded, as well, and --git-dir=.githome works from the appropriate cwd
>> only).
> 
> Bash is even weirder than you might think. Try this:
> 
>   $ echo ~/foo
>   /home/peff/foo
> 
>   $ echo arg=~/foo
>   arg=/home/peff/foo
> 
>   $ echo --arg=~/foo
>   --arg=~/foo
> 
> That is, it expands on the right-hand side of an "=" (which, from my
> reading of the bash manual, means it considers it a word split), but
> refuses to expand after an "=" that is part of a long option.
> 
> The first one is definitely correct. It would be convenient for your use
> case to expand the third one, and it logically follows from the second
> one. However, dash does not expand the second one. I'm not sure if this
> is a bug in bash, or simply a grey area where the two shells do not
> agree.
> 
> But it makes me wonder if the world would be better served by a bash
> option to always enable tilde expansion after an "=". That would solve
> your issue, and it would make the same feature work for every other git
> long option, as well as for other programs.

Yes, in some sense this is working around a bash "feature". I don't
really care too much personally.

Note that we also have "git -c "config.var=value" which, again "works"
because we expand config vars.

So, technically there are no surprises if you know the shell's expansion
rules. But still...

Michael

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

* Re: [RFC/PATCH] git: expand user path in --git-dir
  2012-09-24 14:52           ` Jeff King
@ 2012-09-24 14:57             ` Michael J Gruber
  0 siblings, 0 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-24 14:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Nguyễn Thái Ngọc Duy

Jeff King venit, vidit, dixit 24.09.2012 16:52:
> On Mon, Sep 24, 2012 at 02:57:20PM +0200, Michael J Gruber wrote:
> 
>> Currently, all paths in the config file are subject to tilde expansion
>> for user paths while the argument to --git-dir is not expanded, and
>> neither are paths in the environment such as GIT_DIR. From the user
>> perspective, though, the two commands
>>
>> GIT_DIR=~user/foo git command
>> git --git-dir=~user/foo command
>>
>> currently behave differently because in the first case the shell would
>> perform tilde expansion, but not in the second. Also, one may argue that
>> specifying '--git-dir=' is like specifying a config variable (which
>> cannot exist for this purpose).
>>
>> Thus, make arguments to '--git-dir' undergo tilde expansion.
>> ---
>> So, here's a simple patch implementing tilde expansion for --git-dir. It passes
>> all tests. It's done doing the expansion on the setting side.
>>
>> Alternatively, one might do it on the getting side, i.e. when reading GIT_DIR,
>> so that paths passed directly through the environment undergo tilde expansion
>> as well. We don't do this for any environment variable yet, so I didn't go
>> that far.
> 
> Keep in mind that every layer of expansion you add is a layer of quoting
> somebody else must do in order to pass through certain paths.  I will
> admit that putting a "~" into a path is relatively uncommon, but this
> patch is a regression for anybody who does so with --git-dir (they will
> now need to quote the "~" against not just their shell, but against
> git). Expanding environment variables like GIT_DIR means that we would
> also need to quote things we put into GIT_DIR.

...as far as "~" is concerned, yes. We don't do a full expansion like
the shell does (or doesn't).

Probably "--git-dir" is hidden well enough to be for the initiated only.
As for the other path options, we can always blame bash. It's got its
name for a reason ;)

Michael

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

* Re: GIT_DIR vs. --git-dir
  2012-09-24 14:49 ` Jeff King
  2012-09-24 14:54   ` Michael J Gruber
@ 2012-09-24 15:42   ` Andreas Schwab
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Schwab @ 2012-09-24 15:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael J Gruber, Git Mailing List

Jeff King <peff@peff.net> writes:

> Bash is even weirder than you might think. Try this:
>
>   $ echo ~/foo
>   /home/peff/foo
>
>   $ echo arg=~/foo
>   arg=/home/peff/foo
>
>   $ echo --arg=~/foo
>   --arg=~/foo

Bash expands all arguments that look like variable assignments.  That
lets you write "export arg=~/foo" even though a POSIX shell wouldn't
tilde expand it.

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."

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

* Re: [RFC/PATCH] git: expand user path in --git-dir
  2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
  2012-09-24 14:52           ` Jeff King
@ 2012-09-24 17:30           ` Junio C Hamano
  2012-09-25  5:33           ` Jan Engelhardt
  2 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2012-09-24 17:30 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Nguyễn Thái Ngọc Duy

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Currently, all paths in the config file are subject to tilde expansion
> for user paths while the argument to --git-dir is not expanded, and
> neither are paths in the environment such as GIT_DIR. From the user
> perspective, though, the two commands
>
> GIT_DIR=~user/foo git command
> git --git-dir=~user/foo command
>
> currently behave differently because in the first case the shell would
> perform tilde expansion, but not in the second. Also, one may argue that
> specifying '--git-dir=' is like specifying a config variable (which
> cannot exist for this purpose).
>
> Thus, make arguments to '--git-dir' undergo tilde expansion.
> ---
> So, here's a simple patch implementing tilde expansion for --git-dir. It passes
> all tests. It's done doing the expansion on the setting side.

This looks sensible, as long as there is no potential caller within
this file that wants user-path expansion and that does _not_ want to
export the result to an environment.

The helper will be usable as-is for --work-dir, which does want to
export the result to an environment.  We would want --exec-path=
handle its parameter the same way, but that one has its own setenv()
elsewhere, so "expand-path-setenv()" helper would not help it very
much.  The caller of git_set_argv_exec_path() needs to do the
expanding (and freeing after it makes the call) itself.

> Alternatively, one might do it on the getting side, i.e. when reading GIT_DIR,
> so that paths passed directly through the environment undergo tilde expansion
> as well. We don't do this for any environment variable yet, so I didn't go
> that far.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  git.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/git.c b/git.c
> index 8788b32..35e8011 100644
> --- a/git.c
> +++ b/git.c
> @@ -64,6 +64,22 @@ static void commit_pager_choice(void) {
>  	}
>  }
>  
> +static int expand_path_setenv(const char *name, const char *value, int overwrite)
> +{
> +	int ret;
> +	const char *expanded;
> +
> +	if (!value)
> +		return setenv(name, value, overwrite);
> +
> +	expanded = expand_user_path(value);
> +	if (!expanded)
> +		die("Failed to expand user dir in: '%s'", value);

This should say where the 'value' came from (e.g. "--git-dir=" on
the command line).

> +	ret = setenv(name, expanded, overwrite);
> +	free((void *)expanded);
> +	return ret;
> +}
> +
>  static int handle_options(const char ***argv, int *argc, int *envchanged)
>  {
>  	const char **orig_argv = *argv;
> @@ -117,13 +133,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  				fprintf(stderr, "No directory given for --git-dir.\n" );
>  				usage(git_usage_string);
>  			}
> -			setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
> +			expand_path_setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
>  			if (envchanged)
>  				*envchanged = 1;
>  			(*argv)++;
>  			(*argc)--;
>  		} else if (!prefixcmp(cmd, "--git-dir=")) {
> -			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
> +			expand_path_setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
>  			if (envchanged)
>  				*envchanged = 1;
>  		} else if (!strcmp(cmd, "--namespace")) {

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

* Re: [RFC/PATCH] git: expand user path in --git-dir
  2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
  2012-09-24 14:52           ` Jeff King
  2012-09-24 17:30           ` Junio C Hamano
@ 2012-09-25  5:33           ` Jan Engelhardt
  2012-09-25  7:27             ` Michael J Gruber
  2 siblings, 1 reply; 17+ messages in thread
From: Jan Engelhardt @ 2012-09-25  5:33 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Nguyễn Thái Ngọc Duy


On Monday 2012-09-24 14:57, Michael J Gruber wrote:

>Currently, all paths in the config file are subject to tilde expansion
>for user paths while the argument to --git-dir is not expanded, and
>neither are paths in the environment such as GIT_DIR. From the user
>perspective, though, the two commands
>
>GIT_DIR=~user/foo git command
>git --git-dir=~user/foo command
>
>currently behave differently because in the first case the shell would
>perform tilde expansion, but not in the second.

If git uses a standardized option logic (getopt-like) which accepts
both '=' and (new argument) for long options, you could easily do

	git --git-dir ~user/foo command

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

* Re: [RFC/PATCH] git: expand user path in --git-dir
  2012-09-25  5:33           ` Jan Engelhardt
@ 2012-09-25  7:27             ` Michael J Gruber
  0 siblings, 0 replies; 17+ messages in thread
From: Michael J Gruber @ 2012-09-25  7:27 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git, Nguyễn Thái Ngọc Duy

Jan Engelhardt venit, vidit, dixit 25.09.2012 07:33:
> 
> On Monday 2012-09-24 14:57, Michael J Gruber wrote:
> 
>> Currently, all paths in the config file are subject to tilde expansion
>> for user paths while the argument to --git-dir is not expanded, and
>> neither are paths in the environment such as GIT_DIR. From the user
>> perspective, though, the two commands
>>
>> GIT_DIR=~user/foo git command
>> git --git-dir=~user/foo command
>>
>> currently behave differently because in the first case the shell would
>> perform tilde expansion, but not in the second.
> 
> If git uses a standardized option logic (getopt-like) which accepts
> both '=' and (new argument) for long options, you could easily do
> 
> 	git --git-dir ~user/foo command

Of course, but wouldn't it be even more confusing if tilde expansion "is
done" for "--git-dir ~user/foo" but not "--git-dir=~user/foo"?

That confusion is all bash's "fault" since "is done" == "is done by
bash" (or not), but still.

Michael

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

end of thread, other threads:[~2012-09-25  7:27 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-24  7:19 GIT_DIR vs. --git-dir Michael J Gruber
2012-09-24  7:41 ` Nguyen Thai Ngoc Duy
2012-09-24  7:57   ` Michael J Gruber
2012-09-24  9:53     ` Nguyen Thai Ngoc Duy
2012-09-24 12:56       ` Michael J Gruber
2012-09-24 12:57         ` [RFC/PATCH] git: expand user path in --git-dir Michael J Gruber
2012-09-24 14:52           ` Jeff King
2012-09-24 14:57             ` Michael J Gruber
2012-09-24 17:30           ` Junio C Hamano
2012-09-25  5:33           ` Jan Engelhardt
2012-09-25  7:27             ` Michael J Gruber
2012-09-24 13:37         ` GIT_DIR vs. --git-dir Andreas Schwab
2012-09-24 14:36   ` Junio C Hamano
2012-09-24 14:51     ` Michael J Gruber
2012-09-24 14:49 ` Jeff King
2012-09-24 14:54   ` Michael J Gruber
2012-09-24 15:42   ` Andreas Schwab

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).