Re: [PATCH v6 02/11] replay: extract logic to pick commits

> We're about to add a new git-history(1) command that will reuse some of
> the same infrastructure as git-replay(1). To prepare for this, extract
> the logic to pick a commit into a new "replay.c" file so that it can be
> shared between both commands.
> 
> Rename the function to have a "replay_" prefix to clearly indicate its
> subsystem.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   Makefile         |   1 +
>   builtin/replay.c | 110 ++--------------------------------------------------
>   meson.build      |   1 +
>   replay.c         | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   replay.h         |  23 +++++++++++
>   5 files changed, 143 insertions(+), 107 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1919d35bf3f..01c171b4f03 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1261,6 +1261,7 @@ LIB_OBJS += reftable/tree.o
>   LIB_OBJS += reftable/writer.o
>   LIB_OBJS += remote.o
>   LIB_OBJS += replace-object.o
> +LIB_OBJS += replay.o
>   LIB_OBJS += repo-settings.o
>   LIB_OBJS += repository.o
>   LIB_OBJS += rerere.o
> diff --git a/builtin/replay.c b/builtin/replay.c
> index bb0420dc992..e39824912cd 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -2,7 +2,6 @@
>    * "git replay" builtin command
>    */
>   
> -#define USE_THE_REPOSITORY_VARIABLE
>   #define DISABLE_SIGN_COMPARE_WARNINGS
>   
>   #include "git-compat-util.h"
> @@ -16,6 +15,7 @@
>   #include "object-name.h"
>   #include "parse-options.h"
>   #include "refs.h"
> +#include "replay.h"
>   #include "revision.h"
>   #include "strmap.h"
>   #include <oidset.h>
> @@ -26,13 +26,6 @@ enum ref_action_mode {
>   	REF_ACTION_PRINT
>   };
>   
> -static const char *short_commit_name(struct repository *repo,
> -				     struct commit *commit)
> -{
> -	return repo_find_unique_abbrev(repo, &commit->object.oid,
> -				       DEFAULT_ABBREV);
> -}
> -
>   static struct commit *peel_committish(struct repository *repo, const char *name)
>   {
>   	struct object *obj;
> @@ -45,59 +38,6 @@ static struct commit *peel_committish(struct repository *repo, const char *name)
>   						  OBJ_COMMIT);
>   }
>   
> -static char *get_author(const char *message)
> -{
> -	size_t len;
> -	const char *a;
> -
> -	a = find_commit_header(message, "author", &len);
> -	if (a)
> -		return xmemdupz(a, len);
> -
> -	return NULL;
> -}
> -
> -static struct commit *create_commit(struct repository *repo,
> -				    struct tree *tree,
> -				    struct commit *based_on,
> -				    struct commit *parent)
> -{
> -	struct object_id ret;
> -	struct object *obj = NULL;
> -	struct commit_list *parents = NULL;
> -	char *author;
> -	char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
> -	struct commit_extra_header *extra = NULL;
> -	struct strbuf msg = STRBUF_INIT;
> -	const char *out_enc = get_commit_output_encoding();
> -	const char *message = repo_logmsg_reencode(repo, based_on,
> -						   NULL, out_enc);
> -	const char *orig_message = NULL;
> -	const char *exclude_gpgsig[] = { "gpgsig", NULL };
> -
> -	commit_list_insert(parent, &parents);
> -	extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> -	find_commit_subject(message, &orig_message);
> -	strbuf_addstr(&msg, orig_message);
> -	author = get_author(message);
> -	reset_ident_date();
> -	if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents,
> -				 &ret, author, NULL, sign_commit, extra)) {
> -		error(_("failed to write commit object"));
> -		goto out;
> -	}
> -
> -	obj = parse_object(repo, &ret);
> -
> -out:
> -	repo_unuse_commit_buffer(the_repository, based_on, message);
> -	free_commit_extra_headers(extra);
> -	free_commit_list(parents);
> -	strbuf_release(&msg);
> -	free(author);
> -	return (struct commit *)obj;
> -}
> -
>   struct ref_info {
>   	struct commit *onto;
>   	struct strset positive_refs;
> @@ -246,50 +186,6 @@ static void determine_replay_mode(struct repository *repo,
>   	strset_clear(&rinfo.positive_refs);
>   }
>   
> -static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
> -				    struct commit *commit,
> -				    struct commit *fallback)
> -{
> -	khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid);
> -	if (pos == kh_end(replayed_commits))
> -		return fallback;
> -	return kh_value(replayed_commits, pos);
> -}
> -
> -static struct commit *pick_regular_commit(struct repository *repo,
> -					  struct commit *pickme,
> -					  kh_oid_map_t *replayed_commits,
> -					  struct commit *onto,
> -					  struct merge_options *merge_opt,
> -					  struct merge_result *result)
> -{
> -	struct commit *base, *replayed_base;
> -	struct tree *pickme_tree, *base_tree;
> -
> -	base = pickme->parents->item;
> -	replayed_base = mapped_commit(replayed_commits, base, onto);
> -
> -	result->tree = repo_get_commit_tree(repo, replayed_base);
> -	pickme_tree = repo_get_commit_tree(repo, pickme);
> -	base_tree = repo_get_commit_tree(repo, base);
> -
> -	merge_opt->branch1 = short_commit_name(repo, replayed_base);
> -	merge_opt->branch2 = short_commit_name(repo, pickme);
> -	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
> -
> -	merge_incore_nonrecursive(merge_opt,
> -				  base_tree,
> -				  result->tree,
> -				  pickme_tree,
> -				  result);
> -
> -	free((char*)merge_opt->ancestor);
> -	merge_opt->ancestor = NULL;
> -	if (!result->clean)
> -		return NULL;
> -	return create_commit(repo, result->tree, pickme, replayed_base);
> -}
> -
>   static int handle_ref_update(enum ref_action_mode mode,
>   			     struct ref_transaction *transaction,
>   			     const char *refname,
> @@ -483,8 +379,8 @@ int cmd_replay(int argc,
>   		if (commit->parents->next)
>   			die(_("replaying merge commits is not supported yet!"));
>   
> -		last_commit = pick_regular_commit(repo, commit, replayed_commits,
> -						  onto, &merge_opt, &result);
> +		last_commit = replay_pick_regular_commit(repo, commit, replayed_commits,
> +							 onto, &merge_opt, &result);
>   		if (!last_commit)
>   			break;
>   
> diff --git a/meson.build b/meson.build
> index cee94244759..ae8d4fef059 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -464,6 +464,7 @@ libgit_sources = [
>     'reftable/writer.c',
>     'remote.c',
>     'replace-object.c',
> +  'replay.c',
>     'repo-settings.c',
>     'repository.c',
>     'rerere.c',
> diff --git a/replay.c b/replay.c
> new file mode 100644
> index 00000000000..98be33b8545
> --- /dev/null
> +++ b/replay.c
> @@ -0,0 +1,115 @@
> +#define USE_THE_REPOSITORY_VARIABLE
> +
> +#include "git-compat-util.h"
> +#include "commit.h"
> +#include "environment.h"
> +#include "gettext.h"
> +#include "ident.h"
> +#include "object.h"
> +#include "object-name.h"
> +#include "replay.h"
> +#include "tree.h"
> +
> +static const char *short_commit_name(struct repository *repo,
> +				     struct commit *commit)
> +{
> +	return repo_find_unique_abbrev(repo, &commit->object.oid,
> +				       DEFAULT_ABBREV);
> +}
> +
> +static char *get_author(const char *message)
> +{
> +	size_t len;
> +	const char *a;
> +
> +	a = find_commit_header(message, "author", &len);
> +	if (a)
> +		return xmemdupz(a, len);
> +
> +	return NULL;
> +}
> +
> +struct commit *replay_create_commit(struct repository *repo,
> +				    struct tree *tree,
> +				    struct commit *based_on,
> +				    struct commit *parent)
> +{
> +	struct object_id ret;
> +	struct object *obj = NULL;
> +	struct commit_list *parents = NULL;
> +	char *author;
> +	char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
> +	struct commit_extra_header *extra = NULL;
> +	struct strbuf msg = STRBUF_INIT;
> +	const char *out_enc = get_commit_output_encoding();
> +	const char *message = repo_logmsg_reencode(repo, based_on,
> +						   NULL, out_enc);
> +	const char *orig_message = NULL;
> +	const char *exclude_gpgsig[] = { "gpgsig", NULL };
> +
> +	commit_list_insert(parent, &parents);
> +	extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> +	find_commit_subject(message, &orig_message);
> +	strbuf_addstr(&msg, orig_message);
> +	author = get_author(message);
> +	reset_ident_date();
> +	if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents,
> +				 &ret, author, NULL, sign_commit, extra)) {
> +		error(_("failed to write commit object"));
> +		goto out;
> +	}
> +
> +	obj = parse_object(repo, &ret);
> +
> +out:
> +	repo_unuse_commit_buffer(the_repository, based_on, message);
> +	free_commit_extra_headers(extra);
> +	free_commit_list(parents);
> +	strbuf_release(&msg);
> +	free(author);
> +	return (struct commit *)obj;
> +}
> +
> +static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
> +				    struct commit *commit,
> +				    struct commit *fallback)
> +{
> +	khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid);
> +	if (pos == kh_end(replayed_commits))
> +		return fallback;
> +	return kh_value(replayed_commits, pos);
> +}
> +
> +struct commit *replay_pick_regular_commit(struct repository *repo,
> +					  struct commit *pickme,
> +					  kh_oid_map_t *replayed_commits,
> +					  struct commit *onto,
> +					  struct merge_options *merge_opt,
> +					  struct merge_result *result)
> +{
> +	struct commit *base, *replayed_base;
> +	struct tree *pickme_tree, *base_tree;
> +
> +	base = pickme->parents->item;
> +	replayed_base = mapped_commit(replayed_commits, base, onto);
> +
> +	result->tree = repo_get_commit_tree(repo, replayed_base);
> +	pickme_tree = repo_get_commit_tree(repo, pickme);
> +	base_tree = repo_get_commit_tree(repo, base);
> +
> +	merge_opt->branch1 = short_commit_name(repo, replayed_base);
> +	merge_opt->branch2 = short_commit_name(repo, pickme);
> +	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
> +
> +	merge_incore_nonrecursive(merge_opt,
> +				  base_tree,
> +				  result->tree,
> +				  pickme_tree,
> +				  result);
> +
> +	free((char*)merge_opt->ancestor);
> +	merge_opt->ancestor = NULL;
> +	if (!result->clean)
> +		return NULL;
> +	return replay_create_commit(repo, result->tree, pickme, replayed_base);
> +}
> diff --git a/replay.h b/replay.h
> new file mode 100644
> index 00000000000..d6535ee56c9
> --- /dev/null
> +++ b/replay.h
> @@ -0,0 +1,23 @@
> +#ifndef REPLAY_H
> +#define REPLAY_H
> +
> +#include "khash.h"
> +#include "merge-ort.h"
> +#include "repository.h"
> +
> +struct commit;
> +struct tree;
> +
> +struct commit *replay_create_commit(struct repository *repo,
> +				    struct tree *tree,
> +				    struct commit *based_on,
> +				    struct commit *parent);
> +
> +struct commit *replay_pick_regular_commit(struct repository *repo,
> +					  struct commit *pickme,
> +					  kh_oid_map_t *replayed_commits,
> +					  struct commit *onto,
> +					  struct merge_options *merge_opt,
> +					  struct merge_result *result);
> +
> +#endif
>