Re: [PATCH v2 2/5] rebase -i: support --committer-date-is-author-date

[Alban Gruin <alban.gruin@gmail.com>]

Hi Phillip,

Thank you for continuing this work.

Le 29/04/2020 à 12:25, Phillip Wood a écrit :
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> As part of the on-going effort to retire the apply rebase backend teach
> the merge backend how to handle --committer-date-is-author-date.
> 
> Original-patch-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  Documentation/git-rebase.txt           | 10 +++--
>  builtin/rebase.c                       | 16 ++++---
>  sequencer.c                            | 60 +++++++++++++++++++++++++-
>  sequencer.h                            |  1 +
>  t/t3422-rebase-incompatible-options.sh |  1 -
>  t/t3436-rebase-more-options.sh         | 56 ++++++++++++++++++++++++
>  6 files changed, 133 insertions(+), 11 deletions(-)
> 
> -%<-
>
> diff --git a/sequencer.c b/sequencer.c
> index 6fd2674632..3bb80d7414 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -149,6 +149,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete")
>   * command-line.
>   */
>  static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
> +static GIT_PATH_FUNC(rebase_path_cdate_is_adate, "rebase-merge/cdate_is_adate")
>  static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
>  static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
>  static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet")
> @@ -872,6 +873,22 @@ static char *get_author(const char *message)
>  	return NULL;
>  }
>  
> +static const char *author_date_from_env_array(const struct argv_array *env)
> +{
> +	int i;
> +	const char *date;
> +
> +	for (i = 0; i < env->argc; i++)
> +		if (skip_prefix(env->argv[i],
> +				"GIT_AUTHOR_DATE=", &date))
> +			return date;
> +	/*
> +	 * If GIT_AUTHOR_DATE is missing we should have already errored out when
> +	 * reading the script
> +	 */
> +	BUG("GIT_AUTHOR_DATE missing from author script");
> +}
> +

Since `GIT_AUTHOR_DATE' is set in read_env_script(), why not set
`GIT_COMMITTER_DATE' in this function too, instead of looping in the env
array just after setting it to get the value back?

>  static const char staged_changes_advice[] =
>  N_("you have staged changes in your working tree\n"
>  "If these changes are meant to be squashed into the previous commit, run:\n"
> @@ -938,6 +955,10 @@ static int run_git_commit(struct repository *r,
>  			     gpg_opt, gpg_opt);
>  	}
>  
> +	if (opts->committer_date_is_author_date)
> +		argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
> +				 author_date_from_env_array(&cmd.env_array));
> +
>  	argv_array_push(&cmd.args, "commit");
>  
>  	if (!(flags & VERIFY_MSG))
> @@ -1321,7 +1342,6 @@ static int try_to_commit(struct repository *r,
>  
>  	if (parse_head(r, &current_head))
>  		return -1;
> -

Nit: don't remove this empty line?

>  	if (flags & AMEND_MSG) {
>  		const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
>  		const char *out_enc = get_commit_output_encoding();

Cheers,
Alban