Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Toon Claes <toon@iotcl.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Patrick Steinhardt <ps@pks.im>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>, John Cai <johncai86@gmail.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	Calvin Wan <calvinwan@google.com>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v3 13/15] replay: add --advance or 'cherry-pick' mode
Date: Thu, 22 Jun 2023 12:05:03 +0200	[thread overview]
Message-ID: <87h6qzst8u.fsf@iotcl.com> (raw)
In-Reply-To: <20230602102533.876905-14-christian.couder@gmail.com>


Christian Couder <christian.couder@gmail.com> writes:

> +	/*
> +	 * When the user specifies e.g.
> +	 *   git replay origin/main..mybranch
> +	 *   git replay ^origin/next mybranch1 mybranch2

When I'm trying these, I'm getting the error:
    error: option --onto or --advance is mandatory

In what situation can I omit both --onto and --advance?

> +static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
> +				  const char *onto_name,
> +				  const char **advance_name,
> +				  struct commit **onto,

Would it make sense to call this target?

> +				  struct strset **update_refs)
> +{
> +	struct ref_info rinfo;
> +
> +	get_ref_information(cmd_info, &rinfo);
> +	if (!rinfo.positive_refexprs)
> +		die(_("need some commits to replay"));
> +	if (onto_name && *advance_name)
> +		die(_("--onto and --advance are incompatible"));

Do we actually need to disallow this? I mean from git-replay's point of
view, there's no technical limitation why can cannot support both modes
at once. The update-ref commands in the output will update both target
and source branches, but it's not up to us whether that's desired.

> +	else if (onto_name) {

No need to 'else' here IMHO.

> +		*onto = peel_committish(onto_name);
> +		if (rinfo.positive_refexprs <
> +		    strset_get_size(&rinfo.positive_refs))
> +			die(_("all positive revisions given must be references"));

I tested this locally with the following command:

$ git replay --onto main OID..OID

This command didn't give any errors, neither did it return any
update-ref lines. I would have expected to hit this die().

> +	} else if (*advance_name) {
> +		struct object_id oid;
> +		char *fullname = NULL;
> +
> +		*onto = peel_committish(*advance_name);
> +		if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),
> +			     &oid, &fullname, 0) == 1) {
> +			*advance_name = fullname;
> +		} else {
> +			die(_("argument to --advance must be a reference"));
> +		}
> +		if (rinfo.positive_refexprs > 1)
> +			die(_("cannot advance target with multiple source branches because ordering would be ill-defined"));

The sources aren't always branches, so I suggest something like:

+			die(_("cannot advance target with multiple sources because ordering would be ill-defined"));

> +	determine_replay_mode(&revs.cmdline, onto_name, &advance_name,
> +			      &onto, &update_refs);
> +
> +	if (!onto) /* FIXME: Should handle replaying down to root commit */
> +		die("Replaying down to root commit is not supported yet!");

When I was testing locally I tried the following:

$ git replay --onto main feature

I was expecting this command to find the common ancestor automatically,
but instead I got this error. I'm fine if for now the command does not
determine the common ancestor yet, but I think we should provide a
better error for this scenario.

> +test_expect_success 'using replay on bare repo to perform basic cherry-pick' '
> +	git -C bare replay --advance main topic1..topic2 >result-bare &&
> +	test_cmp expect result-bare
> +'
> +
>  test_done

Shall we add a test case when providing both --onto and --advance? And
one that omits both?

  reply	other threads:[~2023-06-22 10:09 UTC|newest]

Thread overview: 208+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-07  7:24 [PATCH 00/14] Introduce new `git replay` command Christian Couder
2023-04-07  7:24 ` [PATCH 01/14] replay: introduce new builtin Christian Couder
2023-04-07  7:24 ` [PATCH 02/14] replay: start using parse_options API Christian Couder
2023-04-07  7:24 ` [PATCH 03/14] replay: die() instead of failing assert() Christian Couder
2023-04-07  7:24 ` [PATCH 04/14] replay: introduce pick_regular_commit() Christian Couder
2023-04-07  7:24 ` [PATCH 05/14] replay: don't simplify history Christian Couder
2023-04-07  7:24 ` [PATCH 06/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-04-07  7:24 ` [PATCH 07/14] replay: remove progress and info output Christian Couder
2023-04-07  7:24 ` [PATCH 08/14] replay: remove HEAD related sanity check Christian Couder
2023-04-07  7:24 ` [PATCH 09/14] replay: very coarse worktree updating Christian Couder
2023-04-07  7:24 ` [PATCH 10/14] replay: make it a minimal server side command Christian Couder
2023-04-07  7:24 ` [PATCH 11/14] replay: use standard revision ranges Christian Couder
2023-04-14 14:09   ` Derrick Stolee
2023-04-14 14:23     ` Derrick Stolee
2023-04-15 19:07       ` Elijah Newren
2023-04-16  5:28         ` Elijah Newren
2023-04-17 14:05         ` Derrick Stolee
2023-04-18  5:54           ` Elijah Newren
2023-04-18 13:10             ` Derrick Stolee
2023-04-20  4:53               ` Elijah Newren
2023-04-20 13:44                 ` Derrick Stolee
2023-04-23  1:18                   ` Elijah Newren
2023-04-24 15:23                     ` Derrick Stolee
2023-04-30  6:45                       ` Elijah Newren
2023-09-03 15:47                         ` Johannes Schindelin
2023-09-07  8:39                           ` Christian Couder
2023-09-07 10:22                             ` Johannes Schindelin
2023-04-17 15:45         ` Junio C Hamano
2023-04-18  5:58           ` Elijah Newren
2023-04-18  4:58       ` Elijah Newren
2023-04-15 18:30     ` Elijah Newren
2023-04-07  7:24 ` [PATCH 12/14] replay: introduce guess_new_base() Christian Couder
2023-04-07  7:24 ` [PATCH 13/14] replay: add different modes Christian Couder
2023-04-07  7:24 ` [PATCH 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-04-14 10:12 ` [PATCH 00/14] Introduce new `git replay` command Phillip Wood
2023-04-15 17:18   ` Elijah Newren
2023-04-14 17:39 ` Felipe Contreras
2023-04-15  6:44 ` Elijah Newren
2023-05-09 17:53 ` [PATCH v2 00/15] " Christian Couder
2023-05-09 17:53   ` [PATCH v2 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-05-09 17:53   ` [PATCH v2 02/15] replay: introduce new builtin Christian Couder
2023-05-09 17:53   ` [PATCH v2 03/15] replay: start using parse_options API Christian Couder
2023-05-09 17:53   ` [PATCH v2 04/15] replay: die() instead of failing assert() Christian Couder
2023-05-09 17:53   ` [PATCH v2 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-05-09 17:53   ` [PATCH v2 06/15] replay: don't simplify history Christian Couder
2023-05-09 17:53   ` [PATCH v2 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-05-09 17:53   ` [PATCH v2 08/15] replay: remove progress and info output Christian Couder
2023-05-09 17:53   ` [PATCH v2 09/15] replay: remove HEAD related sanity check Christian Couder
2023-05-09 17:53   ` [PATCH v2 10/15] replay: make it a minimal server side command Christian Couder
2023-05-09 17:53   ` [PATCH v2 11/15] replay: use standard revision ranges Christian Couder
2023-05-09 17:53   ` [PATCH v2 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-05-16  4:25     ` Elijah Newren
2023-05-09 17:53   ` [PATCH v2 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-05-09 17:53   ` [PATCH v2 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-05-16  4:26     ` Elijah Newren
2023-05-09 17:53   ` [PATCH v2 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-05-16  4:26     ` Elijah Newren
2023-05-09 22:28   ` [PATCH v2 00/15] Introduce new `git replay` command Junio C Hamano
2023-05-10  7:33     ` Christian Couder
2023-05-16  4:42   ` Elijah Newren
2023-06-02 10:25   ` [PATCH v3 " Christian Couder
2023-06-02 10:25     ` [PATCH v3 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-06-02 10:25     ` [PATCH v3 02/15] replay: introduce new builtin Christian Couder
2023-06-02 10:25     ` [PATCH v3 03/15] replay: start using parse_options API Christian Couder
2023-06-02 10:25     ` [PATCH v3 04/15] replay: die() instead of failing assert() Christian Couder
2023-06-02 10:25     ` [PATCH v3 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-06-02 10:25     ` [PATCH v3 06/15] replay: don't simplify history Christian Couder
2023-06-02 10:25     ` [PATCH v3 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-06-02 10:25     ` [PATCH v3 08/15] replay: remove progress and info output Christian Couder
2023-06-02 10:25     ` [PATCH v3 09/15] replay: remove HEAD related sanity check Christian Couder
2023-06-02 10:25     ` [PATCH v3 10/15] replay: make it a minimal server side command Christian Couder
2023-06-22 10:01       ` Toon Claes
2023-09-07  8:32         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 11/15] replay: use standard revision ranges Christian Couder
2023-06-22 10:03       ` Toon Claes
2023-09-07  8:32         ` Christian Couder
2023-09-07 21:02           ` Dragan Simic
2023-10-10 12:44             ` Christian Couder
2023-10-10 14:02               ` Dragan Simic
2023-06-02 10:25     ` [PATCH v3 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-07-25 21:16       ` Junio C Hamano
2023-09-07  8:33         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-06-22 10:05       ` Toon Claes [this message]
2023-09-07  8:35         ` Christian Couder
2023-07-25 21:41       ` Junio C Hamano
2023-09-07  8:35         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-06-22 10:10       ` Toon Claes
2023-09-07  8:37         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-06-03  1:42     ` [PATCH v3 00/15] Introduce new `git replay` command Junio C Hamano
2023-06-05  7:11       ` Christian Couder
2023-09-07  9:25     ` [PATCH v4 " Christian Couder
2023-09-07  9:25       ` [PATCH v4 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-09-07  9:25       ` [PATCH v4 02/15] replay: introduce new builtin Christian Couder
2023-09-07 10:23         ` Johannes Schindelin
2023-10-10 12:42           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 03/15] replay: start using parse_options API Christian Couder
2023-09-07  9:25       ` [PATCH v4 04/15] replay: die() instead of failing assert() Christian Couder
2023-09-07  9:25       ` [PATCH v4 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-09-07  9:25       ` [PATCH v4 06/15] replay: don't simplify history Christian Couder
2023-09-07 10:23         ` Johannes Schindelin
2023-10-10 12:43           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-09-07  9:25       ` [PATCH v4 08/15] replay: remove progress and info output Christian Couder
2023-09-07  9:25       ` [PATCH v4 09/15] replay: remove HEAD related sanity check Christian Couder
2023-09-07  9:25       ` [PATCH v4 10/15] replay: make it a minimal server side command Christian Couder
2023-09-07  9:25       ` [PATCH v4 11/15] replay: use standard revision ranges Christian Couder
2023-09-07 10:24         ` Johannes Schindelin
2023-10-10 12:49           ` Christian Couder
2023-09-08 22:55         ` Linus Arver
2023-09-10  3:20           ` Linus Arver
2023-10-10 12:48             ` Christian Couder
2023-10-10 12:48           ` Christian Couder
2023-10-19 19:26             ` Linus Arver
2023-09-07  9:25       ` [PATCH v4 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-09-07 10:24         ` Johannes Schindelin
2023-10-10 12:49           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-09-07  9:25       ` [PATCH v4 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-09-07  9:25       ` [PATCH v4 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-09-07 10:25       ` [PATCH v4 00/15] Introduce new `git replay` command Johannes Schindelin
2023-10-10 12:50         ` Christian Couder
2023-10-10 12:38       ` [PATCH v5 00/14] " Christian Couder
2023-10-10 12:38         ` [PATCH v5 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-10-10 12:38         ` [PATCH v5 02/14] replay: introduce new builtin Christian Couder
2023-10-10 12:38         ` [PATCH v5 03/14] replay: start using parse_options API Christian Couder
2023-10-10 12:38         ` [PATCH v5 04/14] replay: die() instead of failing assert() Christian Couder
2023-10-10 12:38         ` [PATCH v5 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-10-10 12:38         ` [PATCH v5 06/14] replay: change rev walking options Christian Couder
2023-10-10 12:38         ` [PATCH v5 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-10-10 12:38         ` [PATCH v5 08/14] replay: remove progress and info output Christian Couder
2023-10-10 12:38         ` [PATCH v5 09/14] replay: remove HEAD related sanity check Christian Couder
2023-10-10 12:38         ` [PATCH v5 10/14] replay: make it a minimal server side command Christian Couder
2023-10-10 12:38         ` [PATCH v5 11/14] replay: use standard revision ranges Christian Couder
2023-10-19 19:49           ` Linus Arver
2023-10-10 12:38         ` [PATCH v5 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-10-10 12:38         ` [PATCH v5 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-10-10 12:38         ` [PATCH v5 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-10-26 13:44         ` [PATCH v5 00/14] Introduce new `git replay` command Johannes Schindelin
2023-10-29  6:01           ` Elijah Newren
2023-11-02 14:59             ` Christian Couder
2023-11-08 12:25               ` Johannes Schindelin
2023-11-02 15:06           ` Christian Couder
2023-11-08 12:25             ` Johannes Schindelin
2023-10-29  6:00         ` Elijah Newren
2023-10-29 14:14           ` Johannes Schindelin
2023-10-30 17:18             ` Elijah Newren
2023-11-02 14:44               ` Christian Couder
2023-11-02 14:48           ` Christian Couder
2023-11-02 13:51         ` [PATCH v6 " Christian Couder
2023-11-02 13:51           ` [PATCH v6 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-02 13:51           ` [PATCH v6 02/14] replay: introduce new builtin Christian Couder
2023-11-02 13:51           ` [PATCH v6 03/14] replay: start using parse_options API Christian Couder
2023-11-02 13:51           ` [PATCH v6 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-02 13:51           ` [PATCH v6 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-02 13:51           ` [PATCH v6 06/14] replay: change rev walking options Christian Couder
2023-11-02 13:51           ` [PATCH v6 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-02 13:51           ` [PATCH v6 08/14] replay: remove progress and info output Christian Couder
2023-11-02 13:51           ` [PATCH v6 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-02 13:51           ` [PATCH v6 10/14] replay: make it a minimal server side command Christian Couder
2023-11-02 13:51           ` [PATCH v6 11/14] replay: use standard revision ranges Christian Couder
2023-11-02 13:51           ` [PATCH v6 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-02 13:51           ` [PATCH v6 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-02 13:51           ` [PATCH v6 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-07  2:43           ` [PATCH v6 00/14] Introduce new `git replay` command Elijah Newren
2023-11-07  9:43             ` Christian Couder
2023-11-15 14:51               ` Christian Couder
2023-11-08 12:19             ` Johannes Schindelin
2023-11-08 12:47           ` Johannes Schindelin
2023-11-15 14:46             ` Christian Couder
2023-11-16  8:45               ` Johannes Schindelin
2023-11-16  8:52                 ` Christian Couder
2023-11-15 14:33           ` [PATCH v7 " Christian Couder
2023-11-15 14:33             ` [PATCH v7 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-15 14:33             ` [PATCH v7 02/14] replay: introduce new builtin Christian Couder
2023-11-15 14:33             ` [PATCH v7 03/14] replay: start using parse_options API Christian Couder
2023-11-15 14:33             ` [PATCH v7 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-15 14:33             ` [PATCH v7 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-15 14:33             ` [PATCH v7 06/14] replay: change rev walking options Christian Couder
2023-11-15 14:33             ` [PATCH v7 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-15 14:33             ` [PATCH v7 08/14] replay: remove progress and info output Christian Couder
2023-11-15 14:33             ` [PATCH v7 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-15 14:33             ` [PATCH v7 10/14] replay: make it a minimal server side command Christian Couder
2023-11-15 14:33             ` [PATCH v7 11/14] replay: use standard revision ranges Christian Couder
2023-11-15 14:33             ` [PATCH v7 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-15 14:33             ` [PATCH v7 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-15 14:33             ` [PATCH v7 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-16  8:53             ` [PATCH v7 00/14] Introduce new `git replay` command Johannes Schindelin
2023-11-23 19:32               ` Elijah Newren
2023-11-24  0:28                 ` Junio C Hamano
2023-11-24 11:10             ` [PATCH v8 " Christian Couder
2023-11-24 11:10               ` [PATCH v8 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-24 11:10               ` [PATCH v8 02/14] replay: introduce new builtin Christian Couder
2023-11-24 11:10               ` [PATCH v8 03/14] replay: start using parse_options API Christian Couder
2023-11-24 11:10               ` [PATCH v8 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-24 11:10               ` [PATCH v8 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-24 11:10               ` [PATCH v8 06/14] replay: change rev walking options Christian Couder
2023-11-24 11:10               ` [PATCH v8 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-24 11:10               ` [PATCH v8 08/14] replay: remove progress and info output Christian Couder
2023-11-24 11:10               ` [PATCH v8 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-24 11:10               ` [PATCH v8 10/14] replay: make it a minimal server side command Christian Couder
2023-11-24 11:10               ` [PATCH v8 11/14] replay: use standard revision ranges Christian Couder
2023-11-24 11:10               ` [PATCH v8 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-24 11:10               ` [PATCH v8 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-24 11:10               ` [PATCH v8 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-25  0:02               ` [PATCH v8 00/14] Introduce new `git replay` command Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6qzst8u.fsf@iotcl.com \
    --to=toon@iotcl.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=calvinwan@google.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).