git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Fix "t0001: test git init when run via an alias"

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 10, 2014, 18:48 UTC
Message-ID
<xmqq4mzsa9ej.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1402220230-799-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 15 quoted lines
> Commit 4ad8332 (t0001: test git init when run via an alias -
> 2010-11-26) noted breakages when running init via alias. The problem
> is for alias to be used, $GIT_DIR must be searched, but 'init' and
> 'clone' are not happy with that. So we start a new process like an
> external command, with clean environment in this case. Env variables
> that are set by command line (e.g. "git --git-dir=.. ") are kept.
>
> This should also fix autocorrecting a command typo to "init" because
> it's the same problem: aliases are read, then "init" is unhappy with
> $GIT_DIR already set up because of that.
>
> Reminded-by: David Turner <dturner@twopensource.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  git.c           | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
This goes far deeper than "Fix t0001", doesn't it?
Show 30 quoted lines
>  t/t0001-init.sh |  4 ++--
>  2 files changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/git.c b/git.c
> index 7780572..d1e49da 100644
> --- a/git.c
> +++ b/git.c
> @@ -484,6 +521,10 @@ static void handle_builtin(int argc, const char **argv)
>  		struct cmd_struct *p = commands+i;
>  		if (strcmp(p->cmd, cmd))
>  			continue;
> +		if (saved_environment && (p->option & NO_SETUP)) {
> +			restore_env();
> +			break;
> +		}
>  		exit(run_builtin(p, argc, argv));
>  	}
>  }
> @@ -539,7 +580,10 @@ static int run_argv(int *argcp, const char ***argv)
>  		 * of overriding "git log" with "git show" by having
>  		 * alias.log = show
>  		 */
> -		if (done_alias || !handle_alias(argcp, argv))
> +		if (done_alias)
> +			break;
> +		save_env();
> +		if (!handle_alias(argcp, argv))
>  			break;
>  		done_alias = 1;
>  	}

So the save-env kicks in only after we tried the builtins and the externals and didn't find any, and before expanding the alias (which has to look at the config, which means we need to do discovery and may contaminate the environment and the globals), and then when we retry with the expanded alias, we restore when we know the command will misbehave if we didn't do so?

That does not sound so bad. Even though I wonder if that "save and then restore" sequence logically belongs around handle_alias(), you would not have sufficient clue to let you cheat by not restoring the environment for commands that you happen to know that they do not care, so that may be a reasonable optimization.

Is it too brittle a solution to force people to mark problematic subcommands with NO_SETUP, though? What kind of change to a subcommand that currently does not have to be marked with NO_SETUP would make it necessary to mark it with NO_SETUP?

Thanks.
Previous: Nguyễn Thái Ngọc DuyNext: Duy Nguyen
Message 8 of 12 in “Git autocorrect bug”
  1. David TurnerJun 5, 2014
  2. Øystein WalleJun 5, 2014
  3. Øystein WalleJun 5, 2014
  4. Duy NguyenJun 5, 2014
  5. David TurnerJun 5, 2014
  6. Duy NguyenJun 6, 2014
  7. Fix "t0001: test git init when run via an alias"Nguyễn Thái Ngọc Duy, Jun 8, 2014
  8. Junio C HamanoJun 10, 2014
  9. Duy NguyenJun 11, 2014
  10. Junio C HamanoJun 12, 2014
  11. Junio C HamanoJun 10, 2014
  12. Junio C HamanoJun 10, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.