Re: [RFC/PATCH] git: expand user path in --git-dir

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 24, 2012, 17:30 UTC
Message-ID: <7vvcf3z5tu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <fdeaa78c0a520ce80e6d2cb09978dab348104eaf.1348491039.git.git@drmicha.warpmail.net>

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

Show 17 quoted lines

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

Show 29 quoted lines

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

Show 24 quoted lines

> +	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")) {