Re: [PATCH v2 3/3] replay: allow to specify a ref with option --ref

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

Toon Claes <toon@iotcl.com> writes:

> When option '--onto' is passed to git-replay(1), the command will update
> refs from the <revision-range> passed to the command. When using option
> '--advance' or '--revert', the argument of that option is a ref that
> will be updated.
>
> To enable users to specify which ref to update, add option '--ref'. When
> using option '--ref', the refs described above are left untouched and
> instead the argument of this option is updated instead.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  Documentation/git-replay.adoc | 21 +++++++++++++-
>  builtin/replay.c              |  8 +++++-
>  replay.c                      | 33 +++++++++++++++++-----
>  replay.h                      |  7 +++++
>  t/t3650-replay-basics.sh      | 66 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 126 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
> index 7e749a0477..5952ecb50d 100644
> --- a/Documentation/git-replay.adoc
> +++ b/Documentation/git-replay.adoc
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  (EXPERIMENTAL!) 'git replay' ([--contained] --onto=<newbase> | --advance=<branch> | --revert=<branch>)
> -			     [--ref-action=<mode>] <revision-range>
> +			     [--ref=<branch>] [--ref-action=<mode>] <revision-range>
>  
>  DESCRIPTION
>  -----------
> @@ -66,6 +66,15 @@ incompatible with `--contained` (which is a modifier for `--onto` only).
>  	Update all branches that point at commits in
>  	<revision-range>. Requires `--onto`.
>  
> +--ref=<branch>::

As this thing takes a full refname (e.g., "--ref=refsheads/mybranch"
in the example in hunk ll.197,+16), we probably want

    --ref=<ref>::

instead.

In the modern documentation style, this should be `--ref=<ref>`::
but let's consistently use traditional style and leave the clean-up
until the dust settles and when the command becomes more quiescent.

> @@ -188,6 +197,16 @@ NOTE: For reverting an entire merge request as a single commit (rather than
>  commit-by-commit), consider using `git merge-tree --merge-base $TIP HEAD $BASE`
>  which can avoid unnecessary merge conflicts.
>  
> +To replay onto a specific commit while updating a different reference:
> +
> +------------
> +$ git replay --onto=112233 --ref=refs/heads/mybranch aabbcc..ddeeff
> +------------
> +
> +This replays the range `aabbcc..ddeeff` onto commit `112233` and updates
> +`refs/heads/mybranch` to point at the result. This can be useful when you want
> +to use bare commit IDs instead of branch names.
> +
>  GIT
>  ---
>  Part of the linkgit:git[1] suite
> diff --git a/builtin/replay.c b/builtin/replay.c
> index a5f81b67d4..876026549e 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -84,7 +84,7 @@ int cmd_replay(int argc,
>  	const char *const replay_usage[] = {
>  		N_("(EXPERIMENTAL!) git replay "
>  		   "([--contained] --onto=<newbase> | --advance=<branch> | --revert=<branch>)\n"
> -		   "[--ref-action=<mode>] <revision-range>"),
> +		   "[--ref=<branch>] [--ref-action=<mode>] <revision-range>"),

Ditto.

>  		NULL
>  	};
>  	struct option replay_options[] = {
> @@ -102,6 +102,10 @@ int cmd_replay(int argc,
>  			     N_("branch"),
>  			     N_("revert commits onto given branch"),
>  			     PARSE_OPT_NONEG),
> +		OPT_STRING_F(0, "ref", &opts.ref,
> +			     N_("branch"),
> +			     N_("reference to update with result"),
> +			     PARSE_OPT_NONEG),

Ditto.

> diff --git a/replay.c b/replay.c
> index 199066f6b3..63cec56d48 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -348,6 +348,8 @@ int replay_revisions(struct rev_info *revs,
>  	bool detached_head;
>  	char *advance;
>  	char *revert;
> +	const char *ref;
> +	struct object_id old_oid;
>  	enum replay_mode mode = REPLAY_MODE_PICK;
>  	int ret;
>  
> @@ -358,6 +360,27 @@ int replay_revisions(struct rev_info *revs,
>  	set_up_replay_mode(revs->repo, &revs->cmdline, opts->onto,
>  			   &detached_head, &advance, &revert, &onto, &update_refs);
>  
> +	if (opts->ref) {
> +		struct object_id oid;
> +
> +		if (update_refs && strset_get_size(update_refs) > 1) {
> +			ret = error(_("'--ref' cannot be used with multiple revision ranges"));
> +			goto out;
> +		}
> +		if (check_refname_format(opts->ref, 0) || !starts_with(opts->ref, "refs/")) {

Can we do something about this overly long line?

> +			ret = error(_("'%s' is not a valid refname"), opts->ref);
> +			goto out;
> +		}
> +		ref = opts->ref;
> +		if (!refs_read_ref(get_main_ref_store(revs->repo), opts->ref, &oid))
> +			oidcpy(&old_oid, &oid);
> +		else
> +			oidclr(&old_oid, revs->repo->hash_algo);
> +	} else {
> +		ref = advance ? advance : revert;
> +		oidcpy(&old_oid, &onto->object.oid);
> +	}
> +
>  	/* FIXME: Should allow replaying commits with the first as a root commit */
>  
>  	if (prepare_revision_walk(revs) < 0) {
> @@ -393,7 +416,7 @@ int replay_revisions(struct rev_info *revs,
>  		kh_value(replayed_commits, pos) = last_commit;
>  
>  		/* Update any necessary branches */
> -		if (advance || revert)
> +		if (ref)
>  			continue;

Nice.

> @@ -427,13 +450,9 @@ int replay_revisions(struct rev_info *revs,
>  		goto out;
>  	}
>  
> -	/* In --advance or --revert mode, update the target ref */
> -	if (advance || revert) {
> -		const char *ref = advance ? advance : revert;
> -		replay_result_queue_update(out, ref,
> -					   &onto->object.oid,
> +	if (ref)
> +		replay_result_queue_update(out, ref, &old_oid,
>  					   &last_commit->object.oid);

Nice, too.