Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"

[Junio C Hamano <gitster@pobox.com>]

Jonathan Nieder <jrnieder@gmail.com> writes:

> This trips reproducibly for
>
>   git ls-remote https://kernel.googlesource.com/pub/scm/git/git
>
> when run outside of a git repository.
>
> Backtrace:
>
>   #0  setup_git_env () at environment.c:172
>   #1  get_git_dir () at environment.c:214
>   #2  get_helper at transport-helper.c:127
>   #3  get_refs_list (for_push=0) at transport-helper.c:1038
>   #4  transport_get_remote_refs at transport.c:1076
>   #5  cmd_ls_remote at builtin/ls-remote.c:97
>
> transport-helper.c:127 is
>
> 	argv_array_pushf(&helper->env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
> 			 get_git_dir());
>
> That code is pretty clearly wrong.  Should it be made conditional
> on have_git_dir(), like this?

Looks good and I agree with Peff's analysis.  Care to wrap it in a
patch with a log message?

Thanks.

>
> Thanks,
> Jonathan
>
>  transport-helper.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 91aed35..e4fd982 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -124,8 +124,9 @@ static struct child_process *get_helper(struct transport *transport)
>  	helper->git_cmd = 0;
>  	helper->silent_exec_failure = 1;
>  
> -	argv_array_pushf(&helper->env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
> -			 get_git_dir());
> +	if (have_git_dir())
> +		argv_array_pushf(&helper->env_array, "%s=%s",
> +				 GIT_DIR_ENVIRONMENT, get_git_dir());
>  
>  	code = start_command(helper);
>  	if (code < 0 && errno == ENOENT)