Re: [PATCH v5 12/14] trailer: set author and committer env variables

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

Christian Couder <chriscool@tuxfamily.org> writes:

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  trailer.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/trailer.c b/trailer.c
> index 98187fc..b5de616 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1,5 +1,6 @@
>  #include "cache.h"
>  #include "run-command.h"
> +#include "argv-array.h"
>  /*
>   * Copyright (c) 2013 Christian Couder <chriscool@tuxfamily.org>
>   */
> @@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp, struct strbuf *buf)
>  	return 0;
>  }
>  
> +static void setup_ac_env(struct argv_array *env, const char *ac_name, const char *ac_mail, const char *(*read)(int))
> +{
> +	if (!getenv(ac_name) || !getenv(ac_mail)) {

Whoever is using this tool while replaying an existing commit likely
wants to export these two variables _into_ this command from the
outside, and this function will not interfere with it.

But for other uses, do we need to export these variables?  After
all, this matters _only_ when the command we spawn wants to know
the idents to be used, but they can ask git-var themselves to cover
both cases, whether the environment that called this tool already
had the ident environment variables or it didn't.

So I am not sure what value this step is adding to the series.

> +		struct ident_split ident;
> +		const char *namebuf, *mailbuf;
> +		int namelen, maillen;
> +		const char *ac_info = read(IDENT_NO_DATE);
> +
> +		if (split_ident_line(&ident, ac_info, strlen(ac_info)))
> +			return;
> +
> +		namelen = ident.name_end - ident.name_begin;
> +		namebuf = ident.name_begin;
> +
> +		maillen = ident.mail_end - ident.mail_begin;
> +		mailbuf = ident.mail_begin;
> +
> +		argv_array_pushf(env, "%s=%.*s", ac_name, namelen, namebuf);
> +		argv_array_pushf(env, "%s=%.*s", ac_mail, maillen, mailbuf);
> +	}
> +}
> +
>  static const char *apply_command(const char *command, const char *arg)
>  {
> +	struct argv_array env = ARGV_ARRAY_INIT;
>  	struct strbuf cmd = STRBUF_INIT;
>  	struct strbuf buf = STRBUF_INIT;
>  	struct child_process cp;
>  	const char *argv[] = {NULL, NULL};
>  	const char *result = "";
>  
> +	setup_ac_env(&env, "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", git_author_info);
> +	setup_ac_env(&env, "GIT_COMMITTER_NAME", "GIT_COMMITTER_EMAIL", git_committer_info);
> +



>  	strbuf_addstr(&cmd, command);
>  	if (arg)
>  		strbuf_replace(&cmd, TRAILER_ARG_STRING, arg);
> @@ -448,7 +475,7 @@ static const char *apply_command(const char *command, const char *arg)
>  	argv[0] = cmd.buf;
>  	memset(&cp, 0, sizeof(cp));
>  	cp.argv = argv;
> -	cp.env = local_repo_env;
> +	cp.env = env.argv;
>  	cp.no_stdin = 1;
>  	cp.out = -1;
>  	cp.use_shell = 1;
> @@ -459,6 +486,7 @@ static const char *apply_command(const char *command, const char *arg)
>  		result = strbuf_detach(&buf, NULL);
>  
>  	strbuf_release(&cmd);
> +	argv_array_clear(&env);
>  	return result;
>  }