Re: [PATCH v8 2/5] am: stop exporting GIT_COMMITTER_DATE

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> The implementation of --committer-date-is-author-date exports
> GIT_COMMITTER_DATE to override the default committer date but does not
> reset GIT_COMMITTER_DATE in the environment after creating the commit
> so it is set in the environment of any hooks that get run. We're about
> to add the same functionality to the sequencer and do not want to have
> GIT_COMMITTER_DATE set when running hooks or exec commands so lets
> update commit_tree_extended() to take an explicit committer so we
> override the default date without setting GIT_COMMITTER_DATE in the
> environment.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/am.c     | 28 +++++++++++++++++++++++-----
>  builtin/commit.c |  4 ++--
>  commit.c         | 11 +++++++----
>  commit.h         |  7 +++----
>  ident.c          | 24 ++++++++++++++----------
>  sequencer.c      |  4 ++--
>  6 files changed, 51 insertions(+), 27 deletions(-)

Nice.

Obviously this would affect the environment while am is running, and
the change is observable by post-applypatch hook.  I am not sure if
this change-in-behaviour would negatively affect people's hooks, but
given the large end-user population we have, somebody somewhere will
get hit.

> diff --git a/ident.c b/ident.c
> index e666ee4e59..7cbf223350 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -361,11 +361,15 @@ N_("\n"
>  const char *fmt_ident(const char *name, const char *email,
>  		      enum want_ident whose_ident, const char *date_str, int flag)
>  {
> -	static struct strbuf ident = STRBUF_INIT;
> +	static int index;
> +	static struct strbuf ident_pool[2] = { STRBUF_INIT, STRBUF_INIT };
>  	int strict = (flag & IDENT_STRICT);
>  	int want_date = !(flag & IDENT_NO_DATE);
>  	int want_name = !(flag & IDENT_NO_NAME);
>  
> +	struct strbuf *ident = &ident_pool[index];
> +	index = (index + 1) % ARRAY_SIZE(ident_pool);

2-element rotating buffer because we happen to care at most two
idents at the same time, author's and committer's?

How many callers of fmt_ident() do we have these days?  I wonder if
we can introduce a new API that lets/forces the caller to prepare a
strbuf and migrate the current callers of this function to it, of if
it is too large a churn for the purpose of this series.

Thanks.