Re: [PATCH 2/3] sequencer: use run_commit_hook()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Phillip,

On Thu, 10 Oct 2019, Phillip Wood via GitGitGadget wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> This simplifies the implementation of run_prepare_commit_msg_hook() and
> will be used in the next commit.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/commit.c | 22 ----------------------
>  commit.h         |  3 ---
>  sequencer.c      | 45 ++++++++++++++++++++++++++++++++++-----------
>  sequencer.h      |  2 ++
>  4 files changed, 36 insertions(+), 36 deletions(-)

Hmm. I would have thought that `commit.c` would be a more logical home
for that function (and that the declaration could remain in `commit.h`)?

The general thrust is good, of course: `commit.h` is supposedly a
`libgit.a` header, but `builtin/commit.o` is _not_ included in
`libgit.a`...

Thanks,
Dscho

>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 1921401117..d898a57f5d 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1443,28 +1443,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
>  	return git_status_config(k, v, s);
>  }
>
> -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
> -{
> -	struct argv_array hook_env = ARGV_ARRAY_INIT;
> -	va_list args;
> -	int ret;
> -
> -	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
> -
> -	/*
> -	 * Let the hook know that no editor will be launched.
> -	 */
> -	if (!editor_is_used)
> -		argv_array_push(&hook_env, "GIT_EDITOR=:");
> -
> -	va_start(args, name);
> -	ret = run_hook_ve(hook_env.argv,name, args);
> -	va_end(args);
> -	argv_array_clear(&hook_env);
> -
> -	return ret;
> -}
> -
>  int cmd_commit(int argc, const char **argv, const char *prefix)
>  {
>  	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
> diff --git a/commit.h b/commit.h
> index f5295ca7f3..37684a35f0 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -389,7 +389,4 @@ void verify_merge_signature(struct commit *commit, int verbose);
>  int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
>  int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused);
>
> -LAST_ARG_MUST_BE_NULL
> -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
> -
>  #endif /* COMMIT_H */
> diff --git a/sequencer.c b/sequencer.c
> index 2adcf5a639..3ce578c40b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1123,28 +1123,51 @@ void commit_post_rewrite(struct repository *r,
>  	run_rewrite_hook(&old_head->object.oid, new_head);
>  }
>
> +int run_commit_hook(int editor_is_used, const char *index_file,
> +		    const char *name, ...)
> +{
> +	struct argv_array hook_env = ARGV_ARRAY_INIT;
> +	va_list args;
> +	int ret;
> +
> +	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
> +
> +	/*
> +	 * Let the hook know that no editor will be launched.
> +	 */
> +	if (!editor_is_used)
> +		argv_array_push(&hook_env, "GIT_EDITOR=:");
> +
> +	va_start(args, name);
> +	ret = run_hook_ve(hook_env.argv,name, args);
> +	va_end(args);
> +	argv_array_clear(&hook_env);
> +
> +	return ret;
> +}
> +
>  static int run_prepare_commit_msg_hook(struct repository *r,
>  				       struct strbuf *msg,
>  				       const char *commit)
>  {
>  	struct argv_array hook_env = ARGV_ARRAY_INIT;
> -	int ret;
> -	const char *name;
> +	int ret = 0;
> +	const char *name, *arg1 = NULL, *arg2 = NULL;
>
>  	name = git_path_commit_editmsg();
>  	if (write_message(msg->buf, msg->len, name, 0))
>  		return -1;
>
> -	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", r->index_file);
> -	argv_array_push(&hook_env, "GIT_EDITOR=:");
> -	if (commit)
> -		ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name,
> -				  "commit", commit, NULL);
> -	else
> -		ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name,
> -				  "message", NULL);
> -	if (ret)
> +	if (commit) {
> +		arg1 = "commit";
> +		arg2 = commit;
> +	} else {
> +		arg1 = "message";
> +	}
> +	if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name,
> +			    arg1, arg2, NULL))
>  		ret = error(_("'prepare-commit-msg' hook failed"));
> +
>  	argv_array_clear(&hook_env);
>
>  	return ret;
> diff --git a/sequencer.h b/sequencer.h
> index ac66892d71..b0419d6ddb 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -201,4 +201,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
>  void sequencer_post_commit_cleanup(struct repository *r);
>  int sequencer_get_last_command(struct repository* r,
>  			       enum replay_action *action);
> +LAST_ARG_MUST_BE_NULL
> +int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
>  #endif /* SEQUENCER_H */
> --
> gitgitgadget
>
>