Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/2] git-merge-file doc: drop "-file" from argument placeholders
From: Junio C Hamano @ 2023-11-02 16:28 UTC (permalink / raw)
  To: Martin Ågren
  Cc: brian m. carlson, git, Elijah Newren, Phillip Wood, Eric Sunshine,
	Taylor Blau
In-Reply-To: <CAN0heSrv7MPcEwkq4uEtv9uBbqm4FLKQLE3gdsEbqKkxPXOj5A@mail.gmail.com>

Martin Ågren <martin.agren@gmail.com> writes:

> Maybe you having a similar reaction a second time makes this smell a bit
> more?

Not at all.  I am perfectly OK with --object-*, not --blob-*, as the
end-user facing option name.  I however strongly prefer to see our
log messages record the thought behind the design accurately in
order to help future developers when they wonder what our intention
was back when the commit was created.

In this case, I want to see that we tell our future selves "even
though we named the option 'object', we plan to support blobs and
nothing else, at least for now".

Thanks.

^ permalink raw reply

* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-02 15:06 UTC (permalink / raw)
  To: Johannes Schindelin, Elijah Newren
  Cc: git, Junio C Hamano, Patrick Steinhardt, John Cai, Derrick Stolee,
	Phillip Wood, Calvin Wan, Toon Claes, Dragan Simic, Linus Arver
In-Reply-To: <bd872b81-80a9-5e4e-dcb6-faebc9671848@gmx.de>

Hi Dscho and Elijah,

On Thu, Oct 26, 2023 at 3:44 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:

> In addition, I am still a bit uneasy with introducing both the manual page
> and the test script in this commit (see my comments in
> https://lore.kernel.org/git/03460733-0219-c648-5757-db1958f8042e@gmx.de/).
> It would be better to uphold our high standard and introduce scaffolds for
> both files in the first commit, then populate the file contents
> incrementally in the same the patches that introduce the corresponding
> options/features/changes.

I have tried to improve on that in the v6 I just sent, but there are
many patches implementing changes in behavior that I think weren't
worth documenting and testing in `test-tool fast-rebase` (which had no
doc and no test) and that aren't worth documenting and testing
specifically in `git replay` either.

Thanks!

^ permalink raw reply

* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-02 14:59 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Johannes Schindelin, git, Junio C Hamano, Patrick Steinhardt,
	John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
	Dragan Simic, Linus Arver
In-Reply-To: <CABPp-BFrVfGHOrBk7g=4TkGxDv=oSqF1FOkhp6WVbxUV-2yveQ@mail.gmail.com>

On Sun, Oct 29, 2023 at 7:02 AM Elijah Newren <newren@gmail.com> wrote:

> On Thu, Oct 26, 2023 at 6:44 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Tue, 10 Oct 2023, Christian Couder wrote:
> >
> [...]
> > >      +  /* requirements/overrides for revs */
> > >     -+  revs.reverse = 1;
> > >     ++  revs.reverse = !revs.reverse;
> > >      +  revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
> > >      +  revs.topo_order = 1;
> > >      +  revs.simplify_history = 0;
> >
> > This still overrides a couple of command-line options, _silently_. I would
> > prefer those three assignments to be moved just before the
> > `setup_revisions()` call.
> >
> > Letting users override these settings may not make much sense, but it
> > makes even less sense to pretend to let them override the settings and
> > then just ignore them without warning. (See also
> > https://en.wikipedia.org/wiki/Principle_of_least_astonishment.)
> >
> > Moving these three assignments before the `setup_revisions()` call would
> > neatly remedy that.
>
> I agree that warnings or error messages would be better.

Ok, a warning() is emitted now in case an command-line option will be
overridden.

> But if we're talking about something short of that, I'd actually argue
> the opposite of what you do here.  I intentionally moved these
> assignments after setup_revisions(), and in my mind, the purpose in
> doing so was to satisfy the Principle of Least Astonishment.  My
> experience with git-fast-export, where some settings are made before
> calling setup_revisions() and then can be overridden, and then do
> completely hideous things, was much worse to me than just admitting
> the flags are bad given the various assumptions the tool makes.  I
> have some patches sitting around to fix fast-export that I never got
> around to upstreaming, but when it came time to implement git-replay,
> I made sure to fix what I viewed as the bigger problem.

I hope you will be able to upstream such changes.

> [...]
> > >     @@ Documentation/git-replay.txt (new)
> > >      +
> > >      +NAME
> > >      +----
> > >     -+git-replay - Replay commits on a different base, without touching working tree
> > >     ++git-replay - Replay commits on a new base, works on bare repos too
> > >      +
> > >      +
> > >      +SYNOPSIS
> >
> > As mentioned in
> > https://lore.kernel.org/git/03460733-0219-c648-5757-db1958f8042e@gmx.de/,
> > I would like the `EXPERIMENTAL` label to be shown prominently here.
> > Probably not only the `SYNOPSIS` as I had originally suggested but also in
> > the `NAME`.

Ok, I have made changes in the v6 I just sent, so that there is
EXPERIMENTAL both in the NAME and SYNOPSIS.

> > Otherwise we may end up with the same situation as with the (from my
> > perspective, failed) `git switch`/`git restore` experiment, where we
> > wanted to explore a better user experience than the overloaded `git
> > checkout` command, only to now be stuck with having to maintain
> > backward-compatibility for `git switch`/`git restore` command-line options
> > that were not meant to be set in stone but to be iterated on, instead. A
> > real-life demonstration of [Hyrum's Law](hyrumslaw.com/), if you like. Or,
> > from a different angle, we re-enacted https://xkcd.com/927/ in a way.

Nit: Hyrum's Law says:

"With a sufficient number of users of an API,
it does not matter what you promise in the contract:
all observable behaviors of your system
will be depended on by somebody."

The doc is part of the contract, which according to this law doesn't
matter. So I don't see why you use this law to suggest a doc change.

> > I'd like to suggest to learn from history and avoid this by tacking on a
> > warning label right at the top of the documentation. We may eventually
> > manage to iterate `git replay` to a point where it is totally capable to
> > supersede `git rebase`, by doing everything the latter does, except
> > better, who knows? But we _do_ need the liberty to make sweeping changes
> > to this new builtin if we want to have a prayer of doing that. And I fear
> > that not even mentioning the EXPERIMENTAL nature right at the top of the
> > manual page would just render us into that undesirable corner.
>
> I fully support this.  Absolutely, 100%.

Ok. Note that as I changed the SYNOPSIS, I also had to change the
usage string, so that it matches the SYNOPSIS, otherwise a test would
fail. So there is "EXPERIMENTAL" in the usage string too.

^ permalink raw reply

* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-02 14:48 UTC (permalink / raw)
  To: Elijah Newren
  Cc: git, Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
	Dragan Simic, Linus Arver
In-Reply-To: <CABPp-BGCzxL-kpOvZzWRTJcx2v18QHm5ev8bFv7bm0dyNqhKug@mail.gmail.com>

On Sun, Oct 29, 2023 at 7:00 AM Elijah Newren <newren@gmail.com> wrote:

> On Tue, Oct 10, 2023 at 5:39 AM Christian Couder
> <christian.couder@gmail.com> wrote:

> >     @@ Documentation/git-replay.txt (new)
> >      +
> >      +NAME
> >      +----
> >     -+git-replay - Replay commits on a different base, without touching working tree
> >     ++git-replay - Replay commits on a new base, works on bare repos too
>
> really minor point: "works on" or "works in" or "works with" ?

I have changed it to "works with", I hope it sounds better. Also I
forgot to talk about this change in the cover letter, sorry about
that.

^ permalink raw reply

* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-02 14:44 UTC (permalink / raw)
  To: Elijah Newren, Johannes Schindelin
  Cc: git, Junio C Hamano, Patrick Steinhardt, John Cai, Derrick Stolee,
	Phillip Wood, Calvin Wan, Toon Claes, Dragan Simic, Linus Arver
In-Reply-To: <CABPp-BGfsda-8CK7_YPJfhGMfpLqdDeB8X6wnqPAGmhiY4KjDA@mail.gmail.com>

Hi Elijah and Dscho,

On Mon, Oct 30, 2023 at 6:18 PM Elijah Newren <newren@gmail.com> wrote:
> On Sun, Oct 29, 2023 at 7:14 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Sat, 28 Oct 2023, Elijah Newren wrote:

> > > I can see why this might sometimes be useful for exclusively linear
> > > history, but it seems to open a can of worms and possibly unfixable
> > > corner cases for non-linear history.  I'd rather not do this, or at
> > > least pull it out of this series and let us discuss it in some follow
> > > up series.  There are some other alternatives that might handle such
> > > usecases better.
> >
> > I find myself wishing for an easy way to reverse commits, if only to
> > switch around the latest two commits while stopped during a rebase.
> >
> > So it would have been nice for me if there had been an easy, worktree-less
> > way to make that happen.
>
> Seems reasonable; we'll definitely want to keep this in mind.
>
> > I guess this would be going in the direction of reordering commits,
> > though, something we deliberately left for later?
>
> Yes, I think that's a good framing for it.

Ok, in the v6 I just sent, a warning() is emitted when `--reverse` is
passed and the option has no effect.

I agree that handling such options in a better way should be left for
later patch series.

Thanks both!

^ permalink raw reply

* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-11-02 14:19 UTC (permalink / raw)
  To: Thomas Guyot; +Cc: Junio C Hamano, Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <d54eedf0-7825-44f5-908c-a51541345872@gmail.com>

On 2023-11-02 14:19, Thomas Guyot wrote:
> On 2023-11-02 02:48, Dragan Simic wrote:
>> On 2023-11-02 06:48, Thomas Guyot wrote:
>>>         -c or --clear-screen ( and backward compat. -C or
>>> --CLEAR-SCREEN )
>>>                Causes full screen repaints to be painted from the top
>>> line down.  By default, full screen repaints are done by scrolling
>>> from the  bottom  of the screen.
>> 
>> AFAIK, the "-c" option is about the way screen contents is updated 
>> when
>> scrolled, and it exists to aid in resolving possible issues with some
>> terminal emulators.  To make sure, I just tested it, and "-c" doesn't
>> replace "-X".
> 
> That's correct, you need both and also -y0

Hmm, I tried the following:

     GIT_PAGER='less -R -F -X -c -y0'

In my environment (Xfce), the result after scrolling the output of "git 
log -p" up and down a bit was about 20 copies of the same screen "page" 
in the scrollback, plus a couple of blank "pages".  Not good, 
unfortunately, and actually much worse than having just "-R -F -X".

>> Huh, this confuses me a bit, quite frankly.  Isn't the "-F" option 
>> used
>> specifically to make pagination invisible in case fewer lines than one
>> full screen are displayed?
> 
> Indeed, but when less update from the bottom, it can add new lines and
> let the overflow lines scroll up into the scrollback buffer.
> 
> Then updating it from the top, it draws the whole page, top to bottom.
> That's fine for a full page but not desired for a partial one. Also
> note that on my terminal (rxvt-unicode) when less clears the screen to
> draw the first page the current screen is rolled up into scrollback -
> iirc that's a configurable option, it would be worth testing other
> terminal's behavior on that. IIRC it may also erase it when using the
> wrong termcap file.
> 
> I haven't looked at the code, but I think it could be possibly to
> start the -c behavior only after a full page is drawn, after exiting
> on partial pages, which would give us the best of both worlds.

Does the GIT_PAGER setup, as I described it above, work for you without 
the described artifacts, in any of the environments you have access to?

>>> OTOH by repainting from the top, the scrollback buffer is never
>>> affected. only the last displayed page remains on the terminal.
>> 
>> Just to clarify, it's the "-X" option that creates all the issues, and
>> the "--redraw-on-quit" option is already there to replace it with no
>> associated issues, but the trouble is that only newer versions of
>> less(1) support the "--redraw-on-quit" option.  IOW, it's all about
>> improving less(1) to avoid complex workarounds required to handle
>> different versions, such as the workarounds used in bat(1).
> 
> TBH I haven't tested --redraw-on-quit, even on Debian Bookworm which
> was just released a couple months ago this option isn't available. I
> suspect that the issue isn't -X, but the scrolling behavior controlled
> by -y and the full redraw controlled by -c.

When you get into the terminfo entry definitions, the root cause is that 
the terminal initialization sequences contain switching to alternate 
screen, which causes screen contents to be lost when less(1) exits.  
Thus, "-X" has been actually abused in the pager setups to skip the 
terminal initialization sequences, which may also result in other 
issues.

One of the solutions is to edit the terminfo entry manually and remove 
the escape codes that cause the switching to and from alternate screen, 
which I tested, but that also introduced another issue -- the screen 
contents was always present after less(1) exited, which isn't always the 
desired behavior.

> Actually I just tested my
> solution on xfce4-terminal and it doesn't work, the terminal still
> push up stuff above on redraw (noteworthy is with rxvt-unicode the
> first draw pushes the current screen contents up but no other redraw
> does, which is what makes it work so well - I haven't tried to find
> out what is being done exactly... OTOH the redraw on scroll down is
> slightly noticeable there, while impossible to see on xfce4-terminal.
> I'll install the latest less and see what happens with --redraw on

Please test the "--redraw-on-quit" option, so far it's the best 
available solution, IMHO.

>>> If less could only enable this behavior after the first full page
>>> draw, that would be perfect!
>> 
>> Could you, please, elaborate a bit on that?
> 
> I mentioned it slightly above, to be clear it would mean that:
> 
> 1. less starts by just writing lined down as usual, making any lines
> above scroll up and overflow into the scrollback buffer as usual
> 2.  If less draws less than a page, exits as before - the effective
> result is as if pager was cat
> 3. If less reaches a full page and still has lines to write, it turns
> on -c's behavior and further updates happen from the top of the
> screen, preventing scroll up (at least on rxvt-unicode)
> 
> Now, if all other terms misbehave here, that's an issue, making this
> suggestion mostly useless. And considering the number of Windows users
> we absolutely need to test Windows Terminal, and should probably test
> MacOS's term too (whatever that is).

Quite frankly, I think that such a solution would be like "fixing the 
fix, which is actually an abuse", as I described it above, eventually 
introducing even more issues, instead of solving the original issue.

>>> Dragan, that may be useful if you're discussing with less
>>> developers...
>> 
>> We've basically reached some kind of an agreement about the need for a
>> good solution, which turned out to be rather complex as a result of
>> being quite universal and extensible, which was required for it to,
>> hopefully, be accepted into less(1).  Also, the author of less(1) 
>> seems
>> to be quite busy with some other things, and he prefers to implement 
>> new
>> features himself.
>> 
>> We've also agreed on another new feature for less(1), hopefully, which
>> isn't exactly related, but should be quite useful.  It's about the
>> secure mode for less(1).
> 
> Feel free to cc me on your next correspondence. If there are mailing
> lists archives for the thread I'll fetch them as needed. We have at
> least one working term/switch combination, which IMO is a better start
> than nothing :)

Please test the "--redraw-on-quit" option, AFAICT that's all we need 
(plus the already mentioned other improvements to less(1), to avoid the 
version-dependent workarounds), and the distributions will eventually 
catch up with the newer versions of less(1).  If the whole thing has 
worked for decades as-is, it can continue working that way for a year or 
two until the packages get updated.

There's actually no two-way mailing list for less(1), the entire project 
is pretty much a one-man show, so to speak.  There's a GitHub page that 
allows issues to be submitted, but I didn't use that, so I exchanged a 
few private email messages instead with the author.  I've already summed 
up the important parts of those messages.

^ permalink raw reply

* [PATCH v6 14/14] replay: stop assuming replayed branches do not diverge
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

The replay command is able to replay multiple branches but when some of
them are based on other replayed branches, their commit should be
replayed onto already replayed commits.

For this purpose, let's store the replayed commit and its original
commit in a key value store, so that we can easily find and reuse a
replayed commit instead of the original one.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c         | 44 ++++++++++++++++++++++++++--------
 t/t3650-replay-basics.sh | 52 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 08ff9bab5e..c3d53ff0cd 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -223,20 +223,33 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
 	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 commit *pickme,
-					  struct commit *last_commit,
+					  kh_oid_map_t *replayed_commits,
+					  struct commit *onto,
 					  struct merge_options *merge_opt,
 					  struct merge_result *result)
 {
-	struct commit *base;
+	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(the_repository, replayed_base);
 	pickme_tree = repo_get_commit_tree(the_repository, pickme);
 	base_tree = repo_get_commit_tree(the_repository, base);
 
-	merge_opt->branch1 = short_commit_name(last_commit);
+	merge_opt->branch1 = short_commit_name(replayed_base);
 	merge_opt->branch2 = short_commit_name(pickme);
 	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
 
@@ -250,7 +263,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 	merge_opt->ancestor = NULL;
 	if (!result->clean)
 		return NULL;
-	return create_commit(result->tree, pickme, last_commit);
+	return create_commit(result->tree, pickme, replayed_base);
 }
 
 int cmd_replay(int argc, const char **argv, const char *prefix)
@@ -266,6 +279,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct merge_options merge_opt;
 	struct merge_result result;
 	struct strset *update_refs = NULL;
+	kh_oid_map_t *replayed_commits;
 	int i, ret = 0;
 
 	const char * const replay_usage[] = {
@@ -338,21 +352,30 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
-
-	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
+	replayed_commits = kh_init_oid_map();
 	while ((commit = get_revision(&revs))) {
 		const struct name_decoration *decoration;
+		khint_t pos;
+		int hr;
 
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		last_commit = pick_regular_commit(commit, replayed_commits, onto,
+						  &merge_opt, &result);
 		if (!last_commit)
 			break;
 
+		/* Record commit -> last_commit mapping */
+		pos = kh_put_oid_map(replayed_commits, commit->object.oid, &hr);
+		if (hr == 0)
+			BUG("Duplicate rewritten commit: %s\n",
+			    oid_to_hex(&commit->object.oid));
+		kh_value(replayed_commits, pos) = last_commit;
+
 		/* Update any necessary branches */
 		if (advance_name)
 			continue;
@@ -381,13 +404,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	}
 
 	merge_finalize(&merge_opt, &result);
-	ret = result.clean;
-
-cleanup:
+	kh_destroy_oid_map(replayed_commits);
 	if (update_refs) {
 		strset_clear(update_refs);
 		free(update_refs);
 	}
+	ret = result.clean;
+
+cleanup:
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index d6286f9580..389670262e 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -143,4 +143,56 @@ test_expect_success 'using replay on bare repo to also rebase a contained branch
 	test_cmp expect result-bare
 '
 
+test_expect_success 'using replay to rebase multiple divergent branches' '
+	git replay --onto main ^topic1 topic2 topic4 >result &&
+
+	test_line_count = 2 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+	test_write_lines E D M L B A >expect &&
+	test_cmp expect actual &&
+
+	git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+	test_write_lines J I M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic2 " >expect &&
+	printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic2 >>expect &&
+	printf "update refs/heads/topic4 " >>expect &&
+	printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic4 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase multiple divergent branches, including contained ones' '
+	git -C bare replay --contained --onto main ^main topic2 topic3 topic4 >result &&
+
+	test_line_count = 4 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	>expect &&
+	for i in 2 1 3 4
+	do
+		printf "update refs/heads/topic$i " >>expect &&
+		printf "%s " $(grep topic$i result | cut -f 3 -d " ") >>expect &&
+		git -C bare rev-parse topic$i >>expect || return 1
+	done &&
+
+	test_cmp expect result &&
+
+	test_write_lines F C M L B A >expect1 &&
+	test_write_lines E D C M L B A >expect2 &&
+	test_write_lines H G F C M L B A >expect3 &&
+	test_write_lines J I M L B A >expect4 &&
+
+	for i in 1 2 3 4
+	do
+		git -C bare log --format=%s $(grep topic$i result | cut -f 3 -d " ") >actual &&
+		test_cmp expect$i actual || return 1
+	done
+'
+
 test_done
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 13/14] replay: add --contained to rebase contained branches
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's add a `--contained` option that can be used along with
`--onto` to rebase all the branches contained in the <revision-range>
argument.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt | 12 +++++++++++-
 builtin/replay.c             | 13 +++++++++++--
 t/t3650-replay-basics.sh     | 29 +++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index e0c85cebf1..e7551aec54 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
+'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
@@ -96,6 +96,16 @@ top of the exact same new base, they only differ in that the first
 provides instructions to make mybranch point at the new commits and
 the second provides instructions to make target point at them.
 
+What if you have a stack of branches, one depending upon another, and
+you'd really like to rebase the whole set?
+
+------------
+$ git replay --contained --onto origin/main origin/main..tipbranch
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_HASH}
+------------
+
 When calling `git replay`, one does not need to specify a range of
 commits to replay using the syntax `A..B`; any range expression will
 do:
diff --git a/builtin/replay.c b/builtin/replay.c
index 145ce9d9a3..08ff9bab5e 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -258,6 +258,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	const char *advance_name = NULL;
 	struct commit *onto = NULL;
 	const char *onto_name = NULL;
+	int contained = 0;
 
 	struct rev_info revs;
 	struct commit *last_commit = NULL;
@@ -268,7 +269,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	int i, ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
+		N_("git replay ([--contained] --onto <newbase> | --advance <branch>) "
+		   "<revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
@@ -278,6 +280,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "onto", &onto_name,
 			   N_("revision"),
 			   N_("replay onto given commit")),
+		OPT_BOOL(0, "contained", &contained,
+			 N_("advance all branches contained in revision-range")),
 		OPT_END()
 	};
 
@@ -289,6 +293,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		usage_with_options(replay_usage, replay_options);
 	}
 
+	if (advance_name && contained)
+		die(_("options '%s' and '%s' cannot be used together"),
+		    "--advance", "--contained");
+
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	/*
@@ -353,7 +361,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			continue;
 		while (decoration) {
 			if (decoration->type == DECORATION_REF_LOCAL &&
-			    strset_contains(update_refs, decoration->name)) {
+			    (contained || strset_contains(update_refs,
+							  decoration->name))) {
 				printf("update %s %s %s\n",
 				       decoration->name,
 				       oid_to_hex(&last_commit->object.oid),
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 68a87e7803..d6286f9580 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -114,4 +114,33 @@ test_expect_success 'replay fails when both --advance and --onto are omitted' '
 	test_must_fail git replay topic1..topic2 >result
 '
 
+test_expect_success 'using replay to also rebase a contained branch' '
+	git replay --contained --onto main main..topic3 >result &&
+
+	test_line_count = 2 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+	test_write_lines F C M L B A >expect &&
+	test_cmp expect actual &&
+
+	git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+	test_write_lines H G F C M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic1 " >expect &&
+	printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic1 >>expect &&
+	printf "update refs/heads/topic3 " >>expect &&
+	printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic3 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to also rebase a contained branch' '
+	git -C bare replay --contained --onto main main..topic3 >result-bare &&
+	test_cmp expect result-bare
+'
+
 test_done
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 12/14] replay: add --advance or 'cherry-pick' mode
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

There is already a 'rebase' mode with `--onto`. Let's add an 'advance' or
'cherry-pick' mode with `--advance`. This new mode will make the target
branch advance as we replay commits onto it.

The replayed commits should have a single tip, so that it's clear where
the target branch should be advanced. If they have more than one tip,
this new mode will error out.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt |  41 ++++++--
 builtin/replay.c             | 185 +++++++++++++++++++++++++++++++++--
 t/t3650-replay-basics.sh     |  34 +++++++
 3 files changed, 243 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 36ddd7daed..e0c85cebf1 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
+'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
@@ -29,14 +29,25 @@ OPTIONS
 	Starting point at which to create the new commits.  May be any
 	valid commit, and not just an existing branch name.
 +
-The update-ref command(s) in the output will update the branch(es) in
-the revision range to point at the new commits, similar to the way how
-`git rebase --update-refs` updates multiple branches in the affected
-range.
+When `--onto` is specified, the update-ref command(s) in the output will
+update the branch(es) in the revision range to point at the new
+commits, similar to the way how `git rebase --update-refs` updates
+multiple branches in the affected range.
+
+--advance <branch>::
+	Starting point at which to create the new commits; must be a
+	branch name.
++
+When `--advance` is specified, the update-ref command(s) in the output
+will update the branch passed as an argument to `--advance` to point at
+the new commits (in other words, this mimics a cherry-pick operation).
 
 <revision-range>::
-	Range of commits to replay; see "Specifying Ranges" in
-	linkgit:git-rev-parse and the "Commit Limiting" options below.
+	Range of commits to replay. More than one <revision-range> can
+	be passed, but in `--advance <branch>` mode, they should have
+	a single tip, so that it's clear where <branch> should point
+	to. See "Specifying Ranges" in linkgit:git-rev-parse and the
+	"Commit Limiting" options below.
 
 include::rev-list-options.txt[]
 
@@ -51,7 +62,9 @@ input to `git update-ref --stdin`.  It is of the form:
 	update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
 
 where the number of refs updated depends on the arguments passed and
-the shape of the history being replayed.
+the shape of the history being replayed.  When using `--advance`, the
+number of refs updated is always one, but for `--onto`, it can be one
+or more (rebasing multiple branches simultaneously is supported).
 
 EXIT STATUS
 -----------
@@ -71,6 +84,18 @@ $ git replay --onto target origin/main..mybranch
 update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
 ------------
 
+To cherry-pick the commits from mybranch onto target:
+
+------------
+$ git replay --advance target origin/main..mybranch
+update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH}
+------------
+
+Note that the first two examples replay the exact same commits and on
+top of the exact same new base, they only differ in that the first
+provides instructions to make mybranch point at the new commits and
+the second provides instructions to make target point at them.
+
 When calling `git replay`, one does not need to specify a range of
 commits to replay using the syntax `A..B`; any range expression will
 do:
diff --git a/builtin/replay.c b/builtin/replay.c
index 8fe4391976..145ce9d9a3 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,6 +14,7 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
+#include "strmap.h"
 #include <oidset.h>
 #include <tree.h>
 
@@ -82,6 +83,146 @@ static struct commit *create_commit(struct tree *tree,
 	return (struct commit *)obj;
 }
 
+struct ref_info {
+	struct commit *onto;
+	struct strset positive_refs;
+	struct strset negative_refs;
+	int positive_refexprs;
+	int negative_refexprs;
+};
+
+static void get_ref_information(struct rev_cmdline_info *cmd_info,
+				struct ref_info *ref_info)
+{
+	int i;
+
+	ref_info->onto = NULL;
+	strset_init(&ref_info->positive_refs);
+	strset_init(&ref_info->negative_refs);
+	ref_info->positive_refexprs = 0;
+	ref_info->negative_refexprs = 0;
+
+	/*
+	 * When the user specifies e.g.
+	 *   git replay origin/main..mybranch
+	 *   git replay ^origin/next mybranch1 mybranch2
+	 * we want to be able to determine where to replay the commits.  In
+	 * these examples, the branches are probably based on an old version
+	 * of either origin/main or origin/next, so we want to replay on the
+	 * newest version of that branch.  In contrast we would want to error
+	 * out if they ran
+	 *   git replay ^origin/master ^origin/next mybranch
+	 *   git replay mybranch~2..mybranch
+	 * the first of those because there's no unique base to choose, and
+	 * the second because they'd likely just be replaying commits on top
+	 * of the same commit and not making any difference.
+	 */
+	for (i = 0; i < cmd_info->nr; i++) {
+		struct rev_cmdline_entry *e = cmd_info->rev + i;
+		struct object_id oid;
+		const char *refexpr = e->name;
+		char *fullname = NULL;
+		int can_uniquely_dwim = 1;
+
+		if (*refexpr == '^')
+			refexpr++;
+		if (repo_dwim_ref(the_repository, refexpr, strlen(refexpr), &oid, &fullname, 0) != 1)
+			can_uniquely_dwim = 0;
+
+		if (e->flags & BOTTOM) {
+			if (can_uniquely_dwim)
+				strset_add(&ref_info->negative_refs, fullname);
+			if (!ref_info->negative_refexprs)
+				ref_info->onto = lookup_commit_reference_gently(the_repository,
+										&e->item->oid, 1);
+			ref_info->negative_refexprs++;
+		} else {
+			if (can_uniquely_dwim)
+				strset_add(&ref_info->positive_refs, fullname);
+			ref_info->positive_refexprs++;
+		}
+
+		free(fullname);
+	}
+}
+
+static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
+				  const char *onto_name,
+				  const char **advance_name,
+				  struct commit **onto,
+				  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"));
+	else if (onto_name) {
+		*onto = peel_committish(onto_name);
+		if (rinfo.positive_refexprs <
+		    strset_get_size(&rinfo.positive_refs))
+			die(_("all positive revisions given must be references"));
+	} 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 sources because ordering would be ill-defined"));
+	} else {
+		int positive_refs_complete = (
+			rinfo.positive_refexprs ==
+			strset_get_size(&rinfo.positive_refs));
+		int negative_refs_complete = (
+			rinfo.negative_refexprs ==
+			strset_get_size(&rinfo.negative_refs));
+		/*
+		 * We need either positive_refs_complete or
+		 * negative_refs_complete, but not both.
+		 */
+		if (rinfo.negative_refexprs > 0 &&
+		    positive_refs_complete == negative_refs_complete)
+			die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+		if (negative_refs_complete) {
+			struct hashmap_iter iter;
+			struct strmap_entry *entry;
+
+			if (rinfo.negative_refexprs == 0)
+				die(_("all positive revisions given must be references"));
+			else if (rinfo.negative_refexprs > 1)
+				die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+			else if (rinfo.positive_refexprs > 1)
+				die(_("cannot advance target with multiple source branches because ordering would be ill-defined"));
+
+			/* Only one entry, but we have to loop to get it */
+			strset_for_each_entry(&rinfo.negative_refs,
+					      &iter, entry) {
+				*advance_name = entry->key;
+			}
+		} else { /* positive_refs_complete */
+			if (rinfo.negative_refexprs > 1)
+				die(_("cannot implicitly determine correct base for --onto"));
+			if (rinfo.negative_refexprs == 1)
+				*onto = rinfo.onto;
+		}
+	}
+	if (!*advance_name) {
+		*update_refs = xcalloc(1, sizeof(**update_refs));
+		**update_refs = rinfo.positive_refs;
+		memset(&rinfo.positive_refs, 0, sizeof(**update_refs));
+	}
+	strset_clear(&rinfo.negative_refs);
+	strset_clear(&rinfo.positive_refs);
+}
+
 static struct commit *pick_regular_commit(struct commit *pickme,
 					  struct commit *last_commit,
 					  struct merge_options *merge_opt,
@@ -114,20 +255,26 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
-	struct commit *onto;
+	const char *advance_name = NULL;
+	struct commit *onto = NULL;
 	const char *onto_name = NULL;
-	struct commit *last_commit = NULL;
+
 	struct rev_info revs;
+	struct commit *last_commit = NULL;
 	struct commit *commit;
 	struct merge_options merge_opt;
 	struct merge_result result;
+	struct strset *update_refs = NULL;
 	int i, ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
+		N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
+		OPT_STRING(0, "advance", &advance_name,
+			   N_("branch"),
+			   N_("make replay advance given branch")),
 		OPT_STRING(0, "onto", &onto_name,
 			   N_("revision"),
 			   N_("replay onto given commit")),
@@ -137,13 +284,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
 
-	if (!onto_name) {
-		error(_("option --onto is mandatory"));
+	if (!onto_name && !advance_name) {
+		error(_("option --onto or --advance is mandatory"));
 		usage_with_options(replay_usage, replay_options);
 	}
 
-	onto = peel_committish(onto_name);
-
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	/*
@@ -171,6 +316,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	revs.topo_order = 1;
 	revs.simplify_history = 0;
 
+	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!");
+
 	if (prepare_revision_walk(&revs) < 0) {
 		ret = error(_("error preparing revisions"));
 		goto cleanup;
@@ -179,6 +330,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
+
 	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
@@ -193,12 +345,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (!last_commit)
 			break;
 
+		/* Update any necessary branches */
+		if (advance_name)
+			continue;
 		decoration = get_name_decoration(&commit->object);
 		if (!decoration)
 			continue;
-
 		while (decoration) {
-			if (decoration->type == DECORATION_REF_LOCAL) {
+			if (decoration->type == DECORATION_REF_LOCAL &&
+			    strset_contains(update_refs, decoration->name)) {
 				printf("update %s %s %s\n",
 				       decoration->name,
 				       oid_to_hex(&last_commit->object.oid),
@@ -208,10 +363,22 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	/* In --advance mode, advance the target ref */
+	if (result.clean == 1 && advance_name) {
+		printf("update %s %s %s\n",
+		       advance_name,
+		       oid_to_hex(&last_commit->object.oid),
+		       oid_to_hex(&onto->object.oid));
+	}
+
 	merge_finalize(&merge_opt, &result);
 	ret = result.clean;
 
 cleanup:
+	if (update_refs) {
+		strset_clear(update_refs);
+		free(update_refs);
+	}
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index a1da4f9ef9..68a87e7803 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -80,4 +80,38 @@ test_expect_success 'using replay on bare repo to rebase with a conflict' '
 	test_expect_code 1 git -C bare replay --onto topic1 B..conflict
 '
 
+test_expect_success 'using replay to perform basic cherry-pick' '
+	# The differences between this test and previous ones are:
+	#   --advance vs --onto
+	# 2nd field of result is refs/heads/main vs. refs/heads/topic2
+	# 4th field of result is hash for main instead of hash for topic2
+
+	git replay --advance main topic1..topic2 >result &&
+
+	test_line_count = 1 result &&
+
+	git log --format=%s $(cut -f 3 -d " " result) >actual &&
+	test_write_lines E D M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/main " >expect &&
+	printf "%s " $(cut -f 3 -d " " result) >>expect &&
+	git rev-parse main >>expect &&
+
+	test_cmp expect result
+'
+
+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_expect_success 'replay on bare repo fails with both --advance and --onto' '
+	test_must_fail git -C bare replay --advance main --onto main topic1..topic2 >result-bare
+'
+
+test_expect_success 'replay fails when both --advance and --onto are omitted' '
+	test_must_fail git replay topic1..topic2 >result
+'
+
 test_done
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 11/14] replay: use standard revision ranges
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Instead of the fixed "<oldbase> <branch>" arguments, the replay
command now accepts "<revision-range>..." arguments in a similar
way as many other Git commands. This makes its interface more
standard and more flexible.

This also enables many revision related options accepted and
eaten by setup_revisions(). If the replay command was a high level
one or had a high level mode, it would make sense to restrict some
of the possible options, like those generating non-contiguous
history, as they could be confusing for most users.

Also as the interface of the command is now mostly finalized,
we can add more documentation and more testcases to make sure
the command will continue to work as designed in the future.

We only document the rev-list related options among all the
revision related options that are now accepted, as the rev-list
related ones are probably the most useful for now.

Helped-by: Dragan Simic <dsimic@manjaro.org>
Helped-by: Linus Arver <linusa@google.com>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt             | 55 +++++++++++++++++++++++-
 builtin/replay.c                         | 21 ++-------
 t/t3650-replay-basics.sh                 | 12 +++++-
 t/t6429-merge-sequence-rename-caching.sh | 18 ++++----
 4 files changed, 76 insertions(+), 30 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 4c852ff3bd..36ddd7daed 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
+'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
@@ -17,7 +17,8 @@ DESCRIPTION
 Takes a range of commits and replays them onto a new location. Leaves
 the working tree and the index untouched, and updates no
 references. The output of this command is meant to be used as input to
-`git update-ref --stdin`, which would update the relevant branches.
+`git update-ref --stdin`, which would update the relevant branches
+(see the OUTPUT section below).
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
 
@@ -27,6 +28,30 @@ OPTIONS
 --onto <newbase>::
 	Starting point at which to create the new commits.  May be any
 	valid commit, and not just an existing branch name.
++
+The update-ref command(s) in the output will update the branch(es) in
+the revision range to point at the new commits, similar to the way how
+`git rebase --update-refs` updates multiple branches in the affected
+range.
+
+<revision-range>::
+	Range of commits to replay; see "Specifying Ranges" in
+	linkgit:git-rev-parse and the "Commit Limiting" options below.
+
+include::rev-list-options.txt[]
+
+OUTPUT
+------
+
+When there are no conflicts, the output of this command is usable as
+input to `git update-ref --stdin`.  It is of the form:
+
+	update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+	update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+	update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+
+where the number of refs updated depends on the arguments passed and
+the shape of the history being replayed.
 
 EXIT STATUS
 -----------
@@ -36,6 +61,32 @@ the replay has conflicts, the exit status is 1.  If the replay is not
 able to complete (or start) due to some kind of error, the exit status
 is something other than 0 or 1.
 
+EXAMPLES
+--------
+
+To simply rebase `mybranch` onto `target`:
+
+------------
+$ git replay --onto target origin/main..mybranch
+update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
+------------
+
+When calling `git replay`, one does not need to specify a range of
+commits to replay using the syntax `A..B`; any range expression will
+do:
+
+------------
+$ git replay --onto origin/main ^base branch1 branch2 branch3
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+------------
+
+This will simultaneously rebase `branch1`, `branch2`, and `branch3`,
+all commits they have since `base`, playing them on top of
+`origin/main`. These three branches may have commits on top of `base`
+that they have in common, but that does not need to be the case.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/replay.c b/builtin/replay.c
index 7e3ebac3db..8fe4391976 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,7 +14,6 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
-#include "strvec.h"
 #include <oidset.h>
 #include <tree.h>
 
@@ -118,16 +117,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct commit *onto;
 	const char *onto_name = NULL;
 	struct commit *last_commit = NULL;
-	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
 	struct merge_result result;
-	struct strbuf branch_name = STRBUF_INIT;
 	int i, ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
+		N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
@@ -145,18 +142,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		usage_with_options(replay_usage, replay_options);
 	}
 
-	if (argc != 3) {
-		error(_("bad number of arguments"));
-		usage_with_options(replay_usage, replay_options);
-	}
-
 	onto = peel_committish(onto_name);
-	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
 	repo_init_revisions(the_repository, &revs, prefix);
 
-	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
-
 	/*
 	 * TODO: For now, let's warn when we see an option that we are
 	 * going to override after setup_revisions() below. In the
@@ -170,8 +159,9 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			warning(_("option '%s' will be overridden"), argv[i]);
 	}
 
-	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
-		ret = error(_("unhandled options"));
+	argc = setup_revisions(argc, argv, &revs, NULL);
+	if (argc > 1) {
+		ret = error(_("unrecognized argument: %s"), argv[1]);
 		goto cleanup;
 	}
 
@@ -181,8 +171,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	revs.topo_order = 1;
 	revs.simplify_history = 0;
 
-	strvec_clear(&rev_walk_args);
-
 	if (prepare_revision_walk(&revs) < 0) {
 		ret = error(_("error preparing revisions"));
 		goto cleanup;
@@ -224,7 +212,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	ret = result.clean;
 
 cleanup:
-	strbuf_release(&branch_name);
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 3567c98362..a1da4f9ef9 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -52,7 +52,7 @@ test_expect_success 'setup bare' '
 '
 
 test_expect_success 'using replay to rebase two branches, one on top of other' '
-	git replay --onto main topic1 topic2 >result &&
+	git replay --onto main topic1..topic2 >result &&
 
 	test_line_count = 1 result &&
 
@@ -68,8 +68,16 @@ test_expect_success 'using replay to rebase two branches, one on top of other' '
 '
 
 test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
-	git -C bare replay --onto main topic1 topic2 >result-bare &&
+	git -C bare replay --onto main topic1..topic2 >result-bare &&
 	test_cmp expect result-bare
 '
 
+test_expect_success 'using replay to rebase with a conflict' '
+	test_expect_code 1 git replay --onto topic1 B..conflict
+'
+
+test_expect_success 'using replay on bare repo to rebase with a conflict' '
+	test_expect_code 1 git -C bare replay --onto topic1 B..conflict
+'
+
 test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 099aefeffc..0f39ed0d08 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,7 +71,7 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 
 		git switch upstream &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -201,7 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -279,7 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -357,7 +357,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test_must_fail git replay --onto HEAD upstream~1 topic >output &&
+		test_must_fail git replay --onto HEAD upstream~1..topic >output &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls
@@ -456,7 +456,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -523,7 +523,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -626,7 +626,7 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -685,7 +685,7 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 10/14] replay: make it a minimal server side command
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want this command to be a minimal command that just does server side
picking of commits, displaying the results on stdout for higher level
scripts to consume.

So let's simplify it:
  * remove the worktree and index reading/writing,
  * remove the ref (and reflog) updating,
  * remove the assumptions tying us to HEAD, since (a) this is not a
    rebase and (b) we want to be able to pick commits in a bare repo,
    i.e. to/from branches that are not checked out and not the main
    branch,
  * remove unneeded includes,
  * handle rebasing multiple branches by printing on stdout the update
    ref commands that should be performed.

The output can be piped into `git update-ref --stdin` for the ref
updates to happen.

In the future to make it easier for users to use this command
directly maybe an option can be added to automatically pipe its output
into `git update-ref`.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt             |  5 +-
 builtin/replay.c                         | 78 ++++++++----------------
 t/t3650-replay-basics.sh                 | 19 +++++-
 t/t6429-merge-sequence-rename-caching.sh | 39 +++++++-----
 4 files changed, 72 insertions(+), 69 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 44bf584fed..4c852ff3bd 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -14,7 +14,10 @@ SYNOPSIS
 DESCRIPTION
 -----------
 
-Takes a range of commits and replays them onto a new location.
+Takes a range of commits and replays them onto a new location. Leaves
+the working tree and the index untouched, and updates no
+references. The output of this command is meant to be used as input to
+`git update-ref --stdin`, which would update the relevant branches.
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
 
diff --git a/builtin/replay.c b/builtin/replay.c
index 9331f5c6ec..7e3ebac3db 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -6,11 +6,7 @@
 #include "git-compat-util.h"
 
 #include "builtin.h"
-#include "cache-tree.h"
-#include "commit.h"
 #include "environment.h"
-#include "gettext.h"
-#include "hash.h"
 #include "hex.h"
 #include "lockfile.h"
 #include "merge-ort.h"
@@ -18,8 +14,6 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
-#include "sequencer.h"
-#include "setup.h"
 #include "strvec.h"
 #include <oidset.h>
 #include <tree.h>
@@ -102,6 +96,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 	pickme_tree = repo_get_commit_tree(the_repository, pickme);
 	base_tree = repo_get_commit_tree(the_repository, base);
 
+	merge_opt->branch1 = short_commit_name(last_commit);
 	merge_opt->branch2 = short_commit_name(pickme);
 	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
 
@@ -122,15 +117,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
 	const char *onto_name = NULL;
-	struct commit *last_commit = NULL, *last_picked_commit = NULL;
-	struct lock_file lock = LOCK_INIT;
+	struct commit *last_commit = NULL;
 	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
-	struct tree *head_tree;
 	struct merge_result result;
-	struct strbuf reflog_msg = STRBUF_INIT;
 	struct strbuf branch_name = STRBUF_INIT;
 	int i, ret = 0;
 
@@ -161,10 +153,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	onto = peel_committish(onto_name);
 	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
-	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
-	if (repo_read_index(the_repository) < 0)
-		BUG("Could not read index");
-
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
@@ -203,58 +191,44 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
-	merge_opt.branch1 = "HEAD";
-	head_tree = repo_get_commit_tree(the_repository, onto);
-	result.tree = head_tree;
+	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
-		struct commit *pick;
+		const struct name_decoration *decoration;
 
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
-		if (!pick)
+		last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		if (!last_commit)
 			break;
-		last_commit = pick;
-		last_picked_commit = commit;
+
+		decoration = get_name_decoration(&commit->object);
+		if (!decoration)
+			continue;
+
+		while (decoration) {
+			if (decoration->type == DECORATION_REF_LOCAL) {
+				printf("update %s %s %s\n",
+				       decoration->name,
+				       oid_to_hex(&last_commit->object.oid),
+				       oid_to_hex(&commit->object.oid));
+			}
+			decoration = decoration->next;
+		}
 	}
 
 	merge_finalize(&merge_opt, &result);
+	ret = result.clean;
 
-	if (result.clean < 0)
-		exit(128);
-
-	if (result.clean) {
-		strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
-			    oid_to_hex(&last_picked_commit->object.oid),
-			    oid_to_hex(&last_commit->object.oid));
-		if (update_ref(reflog_msg.buf, branch_name.buf,
-			       &last_commit->object.oid,
-			       &last_picked_commit->object.oid,
-			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[2]);
-			die("Failed to update %s", argv[2]);
-		}
-		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
-			die(_("unable to update HEAD"));
-	} else {
-		strbuf_addf(&reflog_msg, "rebase progress up to %s",
-			    oid_to_hex(&last_picked_commit->object.oid));
-		if (update_ref(reflog_msg.buf, "HEAD",
-			       &last_commit->object.oid,
-			       &onto->object.oid,
-			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[2]);
-			die("Failed to update %s", argv[2]);
-		}
-	}
-	ret = (result.clean == 0);
 cleanup:
-	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_name);
 	release_revisions(&revs);
-	return ret;
+
+	/* Return */
+	if (ret < 0)
+		exit(128);
+	return ret ? 0 : 1;
 }
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index b5b9f9ade2..3567c98362 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -47,12 +47,29 @@ test_expect_success 'setup' '
 	test_commit C.conflict C.t conflict
 '
 
+test_expect_success 'setup bare' '
+	git clone --bare . bare
+'
+
 test_expect_success 'using replay to rebase two branches, one on top of other' '
 	git replay --onto main topic1 topic2 >result &&
 
+	test_line_count = 1 result &&
+
 	git log --format=%s $(cut -f 3 -d " " result) >actual &&
 	test_write_lines E D M L B A >expect &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic2 " >expect &&
+	printf "%s " $(cut -f 3 -d " " result) >>expect &&
+	git rev-parse topic2 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
+	git -C bare replay --onto main topic1 topic2 >result-bare &&
+	test_cmp expect result-bare
 '
 
 test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 7670b72008..099aefeffc 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,8 +71,9 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 
 		git switch upstream &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked-files &&
 		test_line_count = 2 tracked-files &&
@@ -140,7 +141,9 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls
@@ -198,8 +201,9 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 2 tracked &&
@@ -275,8 +279,9 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 4 tracked &&
@@ -451,8 +456,9 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
@@ -517,8 +523,9 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 3 calls &&
@@ -619,8 +626,9 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls &&
@@ -677,8 +685,9 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 09/14] replay: remove HEAD related sanity check
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want replay to be a command that can be used on the server side on
any branch, not just the current one, so we are going to stop updating
HEAD in a future commit.

A "sanity check" that makes sure we are replaying the current branch
doesn't make sense anymore. Let's remove it.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c         | 8 +-------
 t/t3650-replay-basics.sh | 2 --
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 74680b0c8f..9331f5c6ec 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -123,7 +123,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct commit *onto;
 	const char *onto_name = NULL;
 	struct commit *last_commit = NULL, *last_picked_commit = NULL;
-	struct object_id head;
 	struct lock_file lock = LOCK_INIT;
 	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
@@ -162,11 +161,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	onto = peel_committish(onto_name);
 	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
-	/* Sanity check */
-	if (repo_get_oid(the_repository, "HEAD", &head))
-		die(_("Cannot read HEAD"));
-	assert(oideq(&onto->object.oid, &head));
-
 	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
 	if (repo_read_index(the_repository) < 0)
 		BUG("Could not read index");
@@ -251,7 +245,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			    oid_to_hex(&last_picked_commit->object.oid));
 		if (update_ref(reflog_msg.buf, "HEAD",
 			       &last_commit->object.oid,
-			       &head,
+			       &onto->object.oid,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
 			error(_("could not update %s"), argv[2]);
 			die("Failed to update %s", argv[2]);
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 36c1b5082a..b5b9f9ade2 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -48,8 +48,6 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'using replay to rebase two branches, one on top of other' '
-	git switch main &&
-
 	git replay --onto main topic1 topic2 >result &&
 
 	git log --format=%s $(cut -f 3 -d " " result) >actual &&
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 08/14] replay: remove progress and info output
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

The replay command will be changed in a follow up commit, so that it
will not update refs directly, but instead it will print on stdout a
list of commands that can be consumed by `git update-ref --stdin`.

We don't want this output to be polluted by its current low value
output, so let's just remove the latter.

In the future, when the command gets an option to update refs by
itself, it will make a lot of sense to display a progress meter, but
we are not there yet.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 544a546674..74680b0c8f 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -208,7 +208,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
-	merge_opt.show_rename_progress = 1;
+	merge_opt.show_rename_progress = 0;
 	merge_opt.branch1 = "HEAD";
 	head_tree = repo_get_commit_tree(the_repository, onto);
 	result.tree = head_tree;
@@ -216,9 +216,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	while ((commit = get_revision(&revs))) {
 		struct commit *pick;
 
-		fprintf(stderr, "Rebasing %s...\r",
-			oid_to_hex(&commit->object.oid));
-
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
@@ -237,7 +234,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		exit(128);
 
 	if (result.clean) {
-		fprintf(stderr, "\nDone.\n");
 		strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
 			    oid_to_hex(&last_picked_commit->object.oid),
 			    oid_to_hex(&last_commit->object.oid));
@@ -251,7 +247,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
 			die(_("unable to update HEAD"));
 	} else {
-		fprintf(stderr, "\nAborting: Hit a conflict.\n");
 		strbuf_addf(&reflog_msg, "rebase progress up to %s",
 			    oid_to_hex(&last_picked_commit->object.oid));
 		if (update_ref(reflog_msg.buf, "HEAD",
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 06/14] replay: change rev walking options
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's set the rev walking options we need after calling
setup_revisions() instead of before. This enforces options we always
want for now.

We want the command to work from older commits to newer ones by default.
Also we don't want history simplification, as we want to deal with all
the commits in the affected range.

When we see an option that we are going to override, we emit a warning
to avoid confusion as much as possible though.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 5c4cbd11db..a0d27ab249 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -133,7 +133,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct merge_result result;
 	struct strbuf reflog_msg = STRBUF_INIT;
 	struct strbuf branch_name = STRBUF_INIT;
-	int ret = 0;
+	int i, ret = 0;
 
 	const char * const replay_usage[] = {
 		N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
@@ -173,22 +173,32 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 	repo_init_revisions(the_repository, &revs, prefix);
 
-	revs.verbose_header = 1;
-	revs.max_parents = 1;
-	revs.cherry_mark = 1;
-	revs.limited = 1;
-	revs.reverse = 1;
-	revs.right_only = 1;
-	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
-	revs.topo_order = 1;
-
 	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
 
+	/*
+	 * TODO: For now, let's warn when we see an option that we are
+	 * going to override after setup_revisions() below. In the
+	 * future we might want to either die() or allow them if we
+	 * think they could be useful though.
+	 */
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
+		    !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
+		    !strcmp(argv[i], "--full-history"))
+			warning(_("option '%s' will be overridden"), argv[i]);
+	}
+
 	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
 		ret = error(_("unhandled options"));
 		goto cleanup;
 	}
 
+	/* requirements/overrides for revs */
+	revs.reverse = 1;
+	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+	revs.topo_order = 1;
+	revs.simplify_history = 0;
+
 	strvec_clear(&rev_walk_args);
 
 	if (prepare_revision_walk(&revs) < 0) {
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 07/14] replay: add an important FIXME comment about gpg signing
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want to be able to handle signed commits in some way in the future,
but we are not ready to do it now. So for the time being let's just add
a FIXME comment to remind us about it.

These are different ways we could handle them:

  - in case of a cli user and if there was an interactive mode, we could
    perhaps ask if the user wants to sign again
  - we could add an option to just fail if there are signed commits

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index a0d27ab249..544a546674 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -62,7 +62,7 @@ static struct commit *create_commit(struct tree *tree,
 	struct object *obj;
 	struct commit_list *parents = NULL;
 	char *author;
-	char *sign_commit = NULL;
+	char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
 	struct commit_extra_header *extra;
 	struct strbuf msg = STRBUF_INIT;
 	const char *out_enc = get_commit_output_encoding();
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 02/14] replay: introduce new builtin
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

For now, this is just a rename from `t/helper/test-fast-rebase.c` into
`builtin/replay.c` with minimal changes to make it build appropriately.

Let's add a stub documentation and a stub test script though.

Subsequent commits will flesh out the capabilities of the new command
and make it a more standard regular builtin.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 .gitignore                                    |  1 +
 Documentation/git-replay.txt                  | 38 ++++++++++++
 Makefile                                      |  2 +-
 builtin.h                                     |  1 +
 .../test-fast-rebase.c => builtin/replay.c    | 29 +++------
 command-list.txt                              |  1 +
 git.c                                         |  1 +
 t/helper/test-tool.c                          |  1 -
 t/helper/test-tool.h                          |  1 -
 t/t3650-replay-basics.sh                      | 60 +++++++++++++++++++
 t/t6429-merge-sequence-rename-caching.sh      | 27 +++------
 11 files changed, 121 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/git-replay.txt
 rename t/helper/test-fast-rebase.c => builtin/replay.c (87%)
 create mode 100755 t/t3650-replay-basics.sh

diff --git a/.gitignore b/.gitignore
index 5e56e471b3..612c0f6a0f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -135,6 +135,7 @@
 /git-remote-ext
 /git-repack
 /git-replace
+/git-replay
 /git-request-pull
 /git-rerere
 /git-reset
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
new file mode 100644
index 0000000000..44bf584fed
--- /dev/null
+++ b/Documentation/git-replay.txt
@@ -0,0 +1,38 @@
+git-replay(1)
+=============
+
+NAME
+----
+git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos too
+
+
+SYNOPSIS
+--------
+[verse]
+'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
+
+DESCRIPTION
+-----------
+
+Takes a range of commits and replays them onto a new location.
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
+OPTIONS
+-------
+
+--onto <newbase>::
+	Starting point at which to create the new commits.  May be any
+	valid commit, and not just an existing branch name.
+
+EXIT STATUS
+-----------
+
+For a successful, non-conflicted replay, the exit status is 0.  When
+the replay has conflicts, the exit status is 1.  If the replay is not
+able to complete (or start) due to some kind of error, the exit status
+is something other than 0 or 1.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 03adcb5a48..3834bc1544 100644
--- a/Makefile
+++ b/Makefile
@@ -799,7 +799,6 @@ TEST_BUILTINS_OBJS += test-dump-split-index.o
 TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
 TEST_BUILTINS_OBJS += test-env-helper.o
 TEST_BUILTINS_OBJS += test-example-decorate.o
-TEST_BUILTINS_OBJS += test-fast-rebase.o
 TEST_BUILTINS_OBJS += test-find-pack.o
 TEST_BUILTINS_OBJS += test-fsmonitor-client.o
 TEST_BUILTINS_OBJS += test-genrandom.o
@@ -1290,6 +1289,7 @@ BUILTIN_OBJS += builtin/remote-fd.o
 BUILTIN_OBJS += builtin/remote.o
 BUILTIN_OBJS += builtin/repack.o
 BUILTIN_OBJS += builtin/replace.o
+BUILTIN_OBJS += builtin/replay.o
 BUILTIN_OBJS += builtin/rerere.o
 BUILTIN_OBJS += builtin/reset.o
 BUILTIN_OBJS += builtin/rev-list.o
diff --git a/builtin.h b/builtin.h
index d560baa661..28280636da 100644
--- a/builtin.h
+++ b/builtin.h
@@ -211,6 +211,7 @@ int cmd_remote(int argc, const char **argv, const char *prefix);
 int cmd_remote_ext(int argc, const char **argv, const char *prefix);
 int cmd_remote_fd(int argc, const char **argv, const char *prefix);
 int cmd_repack(int argc, const char **argv, const char *prefix);
+int cmd_replay(int argc, const char **argv, const char *prefix);
 int cmd_rerere(int argc, const char **argv, const char *prefix);
 int cmd_reset(int argc, const char **argv, const char *prefix);
 int cmd_restore(int argc, const char **argv, const char *prefix);
diff --git a/t/helper/test-fast-rebase.c b/builtin/replay.c
similarity index 87%
rename from t/helper/test-fast-rebase.c
rename to builtin/replay.c
index 2bfab66b1b..f2d8444417 100644
--- a/t/helper/test-fast-rebase.c
+++ b/builtin/replay.c
@@ -1,17 +1,11 @@
 /*
- * "git fast-rebase" builtin command
- *
- * FAST: Forking Any Subprocesses (is) Taboo
- *
- * This is meant SOLELY as a demo of what is possible.  sequencer.c and
- * rebase.c should be refactored to use the ideas here, rather than attempting
- * to extend this file to replace those (unless Phillip or Dscho say that
- * refactoring is too hard and we need a clean slate, but I'm guessing that
- * refactoring is the better route).
+ * "git replay" builtin command
  */
 
 #define USE_THE_INDEX_VARIABLE
-#include "test-tool.h"
+#include "git-compat-util.h"
+
+#include "builtin.h"
 #include "cache-tree.h"
 #include "commit.h"
 #include "environment.h"
@@ -27,7 +21,8 @@
 #include "sequencer.h"
 #include "setup.h"
 #include "strvec.h"
-#include "tree.h"
+#include <oidset.h>
+#include <tree.h>
 
 static const char *short_commit_name(struct commit *commit)
 {
@@ -94,7 +89,7 @@ static struct commit *create_commit(struct tree *tree,
 	return (struct commit *)obj;
 }
 
-int cmd__fast_rebase(int argc, const char **argv)
+int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
 	struct commit *last_commit = NULL, *last_picked_commit = NULL;
@@ -110,14 +105,8 @@ int cmd__fast_rebase(int argc, const char **argv)
 	struct strbuf branch_name = STRBUF_INIT;
 	int ret = 0;
 
-	/*
-	 * test-tool stuff doesn't set up the git directory by default; need to
-	 * do that manually.
-	 */
-	setup_git_directory();
-
 	if (argc == 2 && !strcmp(argv[1], "-h")) {
-		printf("Sorry, I am not a psychiatrist; I can not give you the help you need.  Oh, you meant usage...\n");
+		printf("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL\n");
 		exit(129);
 	}
 
@@ -136,7 +125,7 @@ int cmd__fast_rebase(int argc, const char **argv)
 	if (repo_read_index(the_repository) < 0)
 		BUG("Could not read index");
 
-	repo_init_revisions(the_repository, &revs, NULL);
+	repo_init_revisions(the_repository, &revs, prefix);
 	revs.verbose_header = 1;
 	revs.max_parents = 1;
 	revs.cherry_mark = 1;
diff --git a/command-list.txt b/command-list.txt
index 54b2a50f5f..c4cd0f352b 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -160,6 +160,7 @@ git-reflog                              ancillarymanipulators           complete
 git-remote                              ancillarymanipulators           complete
 git-repack                              ancillarymanipulators           complete
 git-replace                             ancillarymanipulators           complete
+git-replay                              plumbingmanipulators
 git-request-pull                        foreignscminterface             complete
 git-rerere                              ancillaryinterrogators
 git-reset                               mainporcelain           history
diff --git a/git.c b/git.c
index c67e44dd82..7068a184b0 100644
--- a/git.c
+++ b/git.c
@@ -594,6 +594,7 @@ static struct cmd_struct commands[] = {
 	{ "remote-fd", cmd_remote_fd, NO_PARSEOPT },
 	{ "repack", cmd_repack, RUN_SETUP },
 	{ "replace", cmd_replace, RUN_SETUP },
+	{ "replay", cmd_replay, RUN_SETUP },
 	{ "rerere", cmd_rerere, RUN_SETUP },
 	{ "reset", cmd_reset, RUN_SETUP },
 	{ "restore", cmd_restore, RUN_SETUP | NEED_WORK_TREE },
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 876cd2dc31..37ba996539 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -30,7 +30,6 @@ static struct test_cmd cmds[] = {
 	{ "dump-untracked-cache", cmd__dump_untracked_cache },
 	{ "env-helper", cmd__env_helper },
 	{ "example-decorate", cmd__example_decorate },
-	{ "fast-rebase", cmd__fast_rebase },
 	{ "find-pack", cmd__find_pack },
 	{ "fsmonitor-client", cmd__fsmonitor_client },
 	{ "genrandom", cmd__genrandom },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 70dd4eba11..8a1a7c63da 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -24,7 +24,6 @@ int cmd__dump_untracked_cache(int argc, const char **argv);
 int cmd__dump_reftable(int argc, const char **argv);
 int cmd__env_helper(int argc, const char **argv);
 int cmd__example_decorate(int argc, const char **argv);
-int cmd__fast_rebase(int argc, const char **argv);
 int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
new file mode 100755
index 0000000000..36c1b5082a
--- /dev/null
+++ b/t/t3650-replay-basics.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='basic git replay tests'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+GIT_AUTHOR_NAME=author@name
+GIT_AUTHOR_EMAIL=bogus@email@address
+export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
+
+test_expect_success 'setup' '
+	test_commit A &&
+	test_commit B &&
+
+	git switch -c topic1 &&
+	test_commit C &&
+	git switch -c topic2 &&
+	test_commit D &&
+	test_commit E &&
+	git switch topic1 &&
+	test_commit F &&
+	git switch -c topic3 &&
+	test_commit G &&
+	test_commit H &&
+	git switch -c topic4 main &&
+	test_commit I &&
+	test_commit J &&
+
+	git switch -c next main &&
+	test_commit K &&
+	git merge -m "Merge topic1" topic1 &&
+	git merge -m "Merge topic2" topic2 &&
+	git merge -m "Merge topic3" topic3 &&
+	>evil &&
+	git add evil &&
+	git commit --amend &&
+	git merge -m "Merge topic4" topic4 &&
+
+	git switch main &&
+	test_commit L &&
+	test_commit M &&
+
+	git switch -c conflict B &&
+	test_commit C.conflict C.t conflict
+'
+
+test_expect_success 'using replay to rebase two branches, one on top of other' '
+	git switch main &&
+
+	git replay --onto main topic1 topic2 >result &&
+
+	git log --format=%s $(cut -f 3 -d " " result) >actual &&
+	test_write_lines E D M L B A >expect &&
+	test_cmp expect actual
+'
+
+test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 75d3fd2dba..7670b72008 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,9 +71,8 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 
 		git switch upstream &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream~1..topic
 
 		git ls-files >tracked-files &&
 		test_line_count = 2 tracked-files &&
@@ -141,8 +140,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
-		#git cherry-pick upstream~1..topic &&
+		git replay --onto HEAD upstream~1 topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls
@@ -200,9 +198,8 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream~1..topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 2 tracked &&
@@ -278,9 +275,8 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream~1..topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 4 tracked &&
@@ -356,8 +352,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test_must_fail test-tool fast-rebase --onto HEAD upstream~1 topic >output &&
-		#git cherry-pick upstream..topic &&
+		test_must_fail git replay --onto HEAD upstream~1 topic >output &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls
@@ -456,9 +451,8 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
@@ -523,9 +517,8 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 3 calls &&
@@ -626,9 +619,8 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls &&
@@ -685,9 +677,8 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic &&
 		git reset --hard topic &&
-		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 05/14] replay: introduce pick_regular_commit()
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's refactor the code to handle a regular commit (a commit that is
neither a root commit nor a merge commit) into a single function instead
of keeping it inside cmd_replay().

This is good for separation of concerns, and this will help further work
in the future to replay merge commits.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 54 ++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 32dbaaf028..5c4cbd11db 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -89,6 +89,35 @@ static struct commit *create_commit(struct tree *tree,
 	return (struct commit *)obj;
 }
 
+static struct commit *pick_regular_commit(struct commit *pickme,
+					  struct commit *last_commit,
+					  struct merge_options *merge_opt,
+					  struct merge_result *result)
+{
+	struct commit *base;
+	struct tree *pickme_tree, *base_tree;
+
+	base = pickme->parents->item;
+
+	pickme_tree = repo_get_commit_tree(the_repository, pickme);
+	base_tree = repo_get_commit_tree(the_repository, base);
+
+	merge_opt->branch2 = short_commit_name(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(result->tree, pickme, last_commit);
+}
+
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
@@ -100,7 +129,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
-	struct tree *next_tree, *base_tree, *head_tree;
+	struct tree *head_tree;
 	struct merge_result result;
 	struct strbuf reflog_msg = STRBUF_INIT;
 	struct strbuf branch_name = STRBUF_INIT;
@@ -175,7 +204,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	result.tree = head_tree;
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
-		struct commit *base;
+		struct commit *pick;
 
 		fprintf(stderr, "Rebasing %s...\r",
 			oid_to_hex(&commit->object.oid));
@@ -185,26 +214,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		base = commit->parents->item;
-
-		next_tree = repo_get_commit_tree(the_repository, commit);
-		base_tree = repo_get_commit_tree(the_repository, base);
-
-		merge_opt.branch2 = short_commit_name(commit);
-		merge_opt.ancestor = xstrfmt("parent of %s", merge_opt.branch2);
-
-		merge_incore_nonrecursive(&merge_opt,
-					  base_tree,
-					  result.tree,
-					  next_tree,
-					  &result);
-
-		free((char*)merge_opt.ancestor);
-		merge_opt.ancestor = NULL;
-		if (!result.clean)
+		pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		if (!pick)
 			break;
+		last_commit = pick;
 		last_picked_commit = commit;
-		last_commit = create_commit(result.tree, commit, last_commit);
 	}
 
 	merge_finalize(&merge_opt, &result);
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 04/14] replay: die() instead of failing assert()
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

It's not a good idea for regular Git commands to use an assert() to
check for things that could happen but are not supported.

Let's die() with an explanation of the issue instead.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index afabb844d3..32dbaaf028 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -179,7 +179,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 		fprintf(stderr, "Rebasing %s...\r",
 			oid_to_hex(&commit->object.oid));
-		assert(commit->parents && !commit->parents->next);
+
+		if (!commit->parents)
+			die(_("replaying down to root commit is not supported yet!"));
+		if (commit->parents->next)
+			die(_("replaying merge commits is not supported yet!"));
+
 		base = commit->parents->item;
 
 		next_tree = repo_get_commit_tree(the_repository, commit);
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 03/14] replay: start using parse_options API
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Instead of manually parsing arguments, let's start using the parse_options
API. This way this new builtin will look more standard, and in some
upcoming commits will more easily be able to handle more command line
options.

Note that we plan to later use standard revision ranges instead of
hardcoded "<oldbase> <branch>" arguments. When we will use standard
revision ranges, it will be easier to check if there are no spurious
arguments if we keep ARGV[0], so let's call parse_options() with
PARSE_OPT_KEEP_ARGV0 even if we don't need ARGV[0] right now to avoid
some useless code churn.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index f2d8444417..afabb844d3 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -15,7 +15,7 @@
 #include "lockfile.h"
 #include "merge-ort.h"
 #include "object-name.h"
-#include "read-cache-ll.h"
+#include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
 #include "sequencer.h"
@@ -92,6 +92,7 @@ static struct commit *create_commit(struct tree *tree,
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
+	const char *onto_name = NULL;
 	struct commit *last_commit = NULL, *last_picked_commit = NULL;
 	struct object_id head;
 	struct lock_file lock = LOCK_INIT;
@@ -105,16 +106,32 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct strbuf branch_name = STRBUF_INIT;
 	int ret = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h")) {
-		printf("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL\n");
-		exit(129);
+	const char * const replay_usage[] = {
+		N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
+		NULL
+	};
+	struct option replay_options[] = {
+		OPT_STRING(0, "onto", &onto_name,
+			   N_("revision"),
+			   N_("replay onto given commit")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
+
+	if (!onto_name) {
+		error(_("option --onto is mandatory"));
+		usage_with_options(replay_usage, replay_options);
 	}
 
-	if (argc != 5 || strcmp(argv[1], "--onto"))
-		die("usage: read the code, figure out how to use it, then do so");
+	if (argc != 3) {
+		error(_("bad number of arguments"));
+		usage_with_options(replay_usage, replay_options);
+	}
 
-	onto = peel_committish(argv[2]);
-	strbuf_addf(&branch_name, "refs/heads/%s", argv[4]);
+	onto = peel_committish(onto_name);
+	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
 	/* Sanity check */
 	if (repo_get_oid(the_repository, "HEAD", &head))
@@ -126,6 +143,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		BUG("Could not read index");
 
 	repo_init_revisions(the_repository, &revs, prefix);
+
 	revs.verbose_header = 1;
 	revs.max_parents = 1;
 	revs.cherry_mark = 1;
@@ -134,7 +152,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	revs.right_only = 1;
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
-	strvec_pushl(&rev_walk_args, "", argv[4], "--not", argv[3], NULL);
+
+	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
 
 	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
 		ret = error(_("unhandled options"));
@@ -197,8 +216,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			       &last_commit->object.oid,
 			       &last_picked_commit->object.oid,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[4]);
-			die("Failed to update %s", argv[4]);
+			error(_("could not update %s"), argv[2]);
+			die("Failed to update %s", argv[2]);
 		}
 		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
 			die(_("unable to update HEAD"));
@@ -210,8 +229,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			       &last_commit->object.oid,
 			       &head,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[4]);
-			die("Failed to update %s", argv[4]);
+			error(_("could not update %s"), argv[2]);
+			die("Failed to update %s", argv[2]);
 		}
 	}
 	ret = (result.clean == 0);
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* [PATCH v6 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>

# Intro

`git replay` has initially been developed entirely by Elijah Newren
mostly last year (2022) at:

https://github.com/newren/git/commits/replay

I took over this year to polish and upstream it as GitLab is
interested in replacing libgit2, and for that purpose needs a command
to do server side (so without using a worktree) rebases, cherry-picks
and reverts.

I reduced the number of commits and features in this patch series,
compared to what Elijah already developed. Especially I stopped short
of replaying merge commits and replaying interactively. These and
other features might be upstreamed in the future after this patch
series has graduated.

The focus in this series is to make it a good plumbing command that
can already be used server side and that replaces the "fast-rebase"
test-tool command. So things to make it easier to use on the command
line, and more advanced features (like replaying merges) are left out.

It looks like GitHub has actually already been using version 3 of this
patch series in production with good results. See:

https://github.blog/2023-07-27-scaling-merge-ort-across-github/
https://lore.kernel.org/git/304f2a49-5e05-7655-9f87-2011606df5db@gmx.de/

# Content of this cover letter

The "Quick Overview" and "Reasons for diverging from cherry-pick &
rebase" sections just below are describing the purpose of the new
command in the big scheme of things. They are taken from Elijah's
design notes
(https://github.com/newren/git/blob/replay/replay-design-notes.txt)
and describe what we want this command to become and the reasons for
that, not what the command is after only this patch series. Also these
design notes were written at least one year ago, so parts of those 2
sections are not true anymore. I have added Phillip Wood's or Felipe
Contreras' notes (thanks to them) where that's the case, but some now
flawed parts may have missed.

After these two sections, starting with the "Important limitations"
section, you will find sections describing what is actually in this
patch series.

More interesting material is available in Elijah's design notes like
an "Intro via examples"
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L37-L132),
a discussion about "Preserving topology, replaying merges"
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L264-L341)
and a "Current status" section describing Elijah's work
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L344-L392)
before I started working on upstreaming it.

I have not included this material here though, as the documentation
added by this patch series for the `git replay` command already
includes an "EXAMPLES" section, and other sections of Elijah's design
notes might not be interesting for now. Also this cover letter is
already pretty long.  But reviewers can refer to the links above if
they think it can help.

# Quick Overview (from Elijah's design notes)

`git replay`, at a basic level, can perhaps be thought of as a
"default-to-dry-run rebase" -- meaning no updates to the working tree,
or to the index, or to any references.  However, it differs from
rebase in that it:

  * Works for branches that aren't checked out

  * Works in a bare repository

  * Can replay multiple branches simultaneously (with or without common
    history in the range being replayed)

  * Preserves relative topology by default (merges are replayed too in
    Elijah's original work, not in this series)

  * Focuses on performance

  * Has several altered defaults as a result of the above

I sometimes think of `git replay` as "fast-replay", a patch-based
analogue to the snapshot-based fast-export & fast-import tools.

# Reasons for diverging from cherry-pick & rebase (from Elijah's
  design notes)

There are multiple reasons to diverge from the defaults in cherry-pick and
rebase.

* Server side needs

  * Both cherry-pick and rebase, via the sequencer, are heavily tied
    to updating the working tree, index, some refs, and a lot of
    control files with every commit replayed, and invoke a mess of
    hooks[1] that might be hard to avoid for backward compatibility
    reasons (at least, that's been brought up a few times on the
    list).

  * cherry-pick and rebase both fork various subprocesses
    unnecessarily, but somewhat intrinsically in part to ensure the
    same hooks are called that old scripted implementations would have
    called.

    Note: since 356ee4659bb (sequencer: try to commit without forking
    'git commit', 2017-11-24) cherry-pick and rebase do not fork
    subprocesses other than hooks for the cases covered by this patch
    series (i.e. they do not fork "git commit" for simple picks).

  * "Dry run" behavior, where there are no updates to worktree, index,
    or even refs might be important.

  * Should not assume users only want to operate on HEAD (see next
    section)

* Decapitate HEAD-centric assumptions

  * cherry-pick forces commits to be played on top of HEAD;
    inflexible.

  * rebase assumes the range of commits to be replayed is
    upstream..HEAD by default, though it allows one to replay
    upstream..otherbranch -- but it still forcibly and needlessly
    checks out 'otherbranch' before starting to replay things.

    Note: since 767a9c417eb (rebase -i: stop checking out the tip of
    the branch to rebase, 2020-01-24) it's not true that rebase
    forcibly and needlessly checks out 'otherbranch'.

  * Assuming HEAD is involved severely limits replaying multiple
    (possibly divergent) branches.

    Note: since 89fc0b53fdb (rebase: update refs from 'update-ref'
    commands, 2022-07-19) the sequencer can update multiple
    branches. The issue with divergent branch is with command line
    arguments and the todo list generation rather than the
    capabilities of the sequencer.

  * Once you stop assuming HEAD has a certain meaning, there's not
    much reason to have two separate commands anymore (except for the
    funny extra not-necessarily-compatible options both have gained
    over time).

  * (Micro issue: Assuming HEAD is involved also makes it harder for
    new users to learn what rebase means and does; it makes command
    lines hard to parse.  Not sure I want to harp on this too much, as
    I have a suspicion I might be creating a tool for experts with
    complicated use cases, but it's a minor quibble.)

* Performance

  * jj is slaughtering us on rebase speed[2].  I would like us to become
    competitive.  (I dropped a few comments in the link at [2] about why
    git is currently so bad.)

  * From [3], there was a simple 4-patch series in linux.git that took
    53 seconds to rebase.  Switching to ort dropped it to 16 seconds.
    While that sounds great, only 11 *milliseconds* were needed to do
    the actual merges.  That means almost *all* the time (>99%) was
    overhead!  Big offenders:

    * --reapply-cherry-picks should be the default

    * can_fast_forward() should be ripped out, and perhaps other extraneous
      revision walks

      Note: d42c9ffa0f (rebase: factor out branch_base calculation,
      2022-10-17) might already deal with that (according to Felipe
      Contreras).

    * avoid updating working tree, index, refs, reflogs, and control
      structures except when needed (e.g. hitting a conflict, or operation
      finished)

  * Other performance ideas (mostly for future work, not in this
    series)

    * single-file control structures instead of directory of files
      (when doing interactive things which is in Elijah's original
      work, but not in this series)

    * avoid forking subprocesses unless explicitly requested (e.g.
      --exec, --strategy, --run-hooks).  For example, definitely do not
      invoke `git commit` or `git merge`.

    * Sanitize hooks:

      * dispense with all per-commit hooks for sure (pre-commit,
        post-commit, post-checkout).

      * pre-rebase also seems to assume exactly 1 ref is written, and
        invoking it repeatedly would be stupid.  Plus, it's specific
        to "rebase".  So...ignore?  (Stolee's --ref-update option for
        rebase probably broke the pre-rebase assumptions already...)

      * post-rewrite hook might make sense, but fast-import got
        exempted, and I think of replay like a patch-based analogue
        to the snapshot-based fast-import.

    * When not running server side, resolve conflicts in a sparse-cone
      sparse-index worktree to reduce number of files written to a
      working tree.  (See below as well.)

    * [High risk of possible premature optimization] Avoid large
      numbers of newly created loose objects, when replaying large
      numbers of commits.  Two possibilities: (1) Consider using
      tmp-objdir and pack objects from the tmp-objdir at end of
      exercise, (2) Lift code from git-fast-import to immediately
      stuff new objects into a pack?

* Multiple branches and non-checked out branches

  * The ability to operate on non-checked out branches also implies
    that we should generally be able to replay when in a dirty working
    tree (exception being when we expect to update HEAD and any of the
    dirty files is one that needs to be updated by the replay).

  * Also, if we are operating locally on a non-checked out branch and
    hit a conflict, we should have a way to resolve the conflict
    without messing with the user's work on their current
    branch. (This is not is this patch series though.)

    * Idea: new worktree with sparse cone + sparse index checkout,
      containing only files in the root directory, and whatever is
      necessary to get the conflicts

    * Companion to above idea: control structures should be written to
      $GIT_COMMON_DIR/replay-${worktree}, so users can have multiple
      replay sessions, and so we know which worktrees are associated
      with which replay operations.

  - [1] https://lore.kernel.org/git/pull.749.v3.git.git.1586044818132.gitgitgadget@gmail.com/
  - [2] https://github.com/martinvonz/jj/discussions/49
  - [3] https://lore.kernel.org/git/CABPp-BE48=97k_3tnNqXPjSEfA163F8hoE+HY0Zvz1SWB2B8EA@mail.gmail.com/

# Important limitations

* The code exits with code 1 if there are any conflict. No
  resumability. No nice output. No interactivity. No special exit code
  depending on the reason.

* When a commit becomes empty as it is replayed, it is still replayed
  as an empty commit, instead of being dropped.

* No replaying merges, nor root commits. Only regular commits.

* Signed commits are not properly handled. It's not clear what to do
  to such commits when replaying on the server side.

* Notes associated with replayed commits are not updated nor
  duplicated. (Thanks to Phillip Wood for noticing.)

# Commit overview

* 1/14 t6429: remove switching aspects of fast-rebase

    Preparatory commit to make it easier to later replace the
    fast-rebase test-tool by `git replay` without breaking existing
    tests.

* 2/14 replay: introduce new builtin

    This creates a minimal `git replay` command by moving the code
    from the `fast-rebase` test helper from `t/helper/` into
    `builtin/` and doing some renames and a few other needed changes.
    Since v5, this also introduces stub documentation and tests for
    the new command as suggested by Dscho. Also in the doc,
    "EXPERIMENTAL" has been added both to the name line and the
    synopsis line of the command, which was also suggested by
    Dscho. This required to also change the "usage" message of the
    command.

* - 3/14 replay: start using parse_options API
  - 4/14 replay: die() instead of failing assert()
  - 5/14 replay: introduce pick_regular_commit()
  - 6/14 replay: change rev walking options
  - 7/14 replay: add an important FIXME comment about gpg signing
  - 8/14 replay: remove progress and info output
  - 9/14 replay: remove HEAD related sanity check

    These slowly change the command to make it behave more like
    regular commands and to start cleaning up its output. In patch
    6/14 (replay: change rev walking options) there are some changes
    compared to v5 as suggested by Elijah and Dscho. First we are now
    warning() in case a command line option would change some rev
    options that we are going to override. Second we don't let the
    `--reverse` option change the way the command works.

* 10/14 replay: make it a minimal server side command

    After the cleaning up in previous commits, it's now time to
    radically change the way it works by stopping it to do ref
    updates, to update the index and worktree, to consider HEAD as
    special. Instead just make it output commands that should be
    passed to `git update-ref --stdin`. A few test changes are new in
    this commit since v5.

* 11/14 replay: use standard revision ranges

    Start addind new interesting features and also documentation and
    tests, as the interface of the command is cristalizing into its
    final form. Since v5 this patch is not introducing the
    documentation and the tests of the new command anymore (as they
    are introduced in patch 2/14).

* - 12/14 replay: add --advance or 'cherry-pick' mode
  - 13/14 replay: add --contained to rebase contained branches

    Add new options and features to the command.

* 14/14 replay: stop assuming replayed branches do not diverge

    This adds another interesting feature, as well as related
    documentation and tests.

# Notes about `fast-rebase`, tests and documentation

The `fast-rebase` test-tool helper was developed by Elijah to
experiment with a rebasing tool that would be developed from scratch
based on his merge-ort work, could be used to test that merge-ort
work, and would not have the speed and interface limitations of `git
rebase` or `git cherry-pick`.

This `fast-rebase` helper was used before this series in:

t6429-merge-sequence-rename-caching.sh

So when `git replay` is created from `fast-rebase` in patch 2/14, the
t6429 test script is also converted to use `git replay`. This ensures
that `git replay` doesn't break too badly during the first 10 patches
in this patch series.

Tests and documentation are introduced specifically for `git replay`
as soon as patch 2/14, but they are not much improved since around
11/14 as it doesn't make much sense to document and test behavior that
we know is going to change soon.

# Possibly controversial issues 

* bare or not bare: this series works towards a plumbing command with
  the end goal of it being usable and used first on bare repos,
  contrary to existing commands like `git rebase` and `git
  cherry-pick`. The tests check that the command works on both bare
  and non-bare repo though.

* exit status: a successful, non-conflicted replay exits with code
  0. When the replay has conflicts, the exit status is 1. If the
  replay is not able to complete (or start) due to some kind of error,
  the exit status is something other than 0 or 1. There are a few
  tests checking that. It has been suggested in an internal review
  that conflicts might want to get a more specific error code as an
  error code of 1 might be quite easy to return by accident. It
  doesn't seem to me from their docs (which might want to be improved,
  I didn't look at the code) that other commands like `git merge` and
  `git rebase` exit with a special error code in case of conflict.

* make worktree and index changes optional: commit 10/14 stops
  updating the index and worktree, but it might be better especially
  for cli users to make that optional. The issue is that this would
  make the command more complex while we are developing a number of
  important features so that the command can be used on bare repos. It
  seems that this should rather be done in an iterative improvement
  after the important features have landed.

* --advance and --contained: these two advanced options might not
  belong to this first series and could perhaps be added in a followup
  series in separate commits. On the other hand the code for
  --contained seems involved with the code of --advance and it's nice
  to see soon that git replay can indeed do cherry-picking and rebase
  many refs at once, and this way fullfil these parts of its promise.

* replaying diverging branches: 14/14 the last patch in the series,
  which allow replaying diverging branches, can be seen as a
  fundamental fix or alternatively as adding an interesting
  feature. So it's debatable if it should be in its own patch along
  with its own tests as in this series, or if it should be merged into
  a previous patch and which one.

* only 2 patches: this patch series can be seen from a high level
  point of view as 1) introducing the new `git replay` command, and 2)
  using `git replay` to replace, and get rid of, the fast-rebase
  test-tool command. The fact that not much of the original
  fast-rebase code and interface is left would agree with that point
  of view. On the other hand, fast-rebase can also be seen as a first
  iteration towards `git replay`. So it can also make sense to see how
  `git replay` evolved from it.

# Changes between v5 and v6

Thanks to Dscho, Linus Arver and Dragan Simic for their suggestions on
the previous version! The few changes compared to v5 are:

* The patch series was rebased onto master at 2e8e77cbac (The
  twenty-first batch, 2023-10-23). This is to fix small conflicts
  with recently merged series, especially
  cc/repack-sift-filtered-objects-to-separate-pack.

* In patch 2/14 (replay: introduce new builtin), stub documentation
  and tests have been introduced as suggested by Dscho. They were
  introduced later (in patch 11/14) in v5.

* Also in patch 2/14, "EXPERIMENTAL" has been added both to the name
  line and the synopsis line of the command in the doc as suggested by
  Dscho.

* In patch 6/14 (replay: change rev walking options) we are now
  warning() in case a command line option would change some rev
  options that we are going to override. This follows discussions
  between Dscho and Elijah where they agreed that properly taking care
  of handling these command line options should be dealt with later in
  future patch series.

* Also in patch 6/14, we don't let the `--reverse` option change the
  way the command works. This also follows discussions between Dscho
  and Elijah about command line options related to rev walking.

* In patch 9/14 (replay: remove HEAD related sanity check) we can
  simplify a test a bit.

* In patch 10/14 (replay: make it a minimal server side command) we
  can further add tests.

* In patch 11/14 (replay: use standard revision ranges), as
  documentation and tests are not introduced by this commit anymore,
  but earlier by patch 2/14, fewer doc and test changes are made in
  this patch.

CI tests seem to pass according to:

https://github.com/chriscool/git/actions/runs/6732523160/

(A "Run ci/install-dependencies.sh" test on osx-clang initially
failed, but it worked when I just re-ran it. Hopefully this will alow
tests that couldn't be run to proceed and succeed, but I don't want to
wait for that right now.)

# Range-diff between v5 and v6

(Sorry it's very long mostly due to doc and test changes over a number
of patches.)

 1:  72c34a0eba =  1:  fac0a9dff4 t6429: remove switching aspects of fast-rebase
 2:  f85e6c823c !  2:  8a605ddef8 replay: introduce new builtin
    @@ Commit message
         For now, this is just a rename from `t/helper/test-fast-rebase.c` into
         `builtin/replay.c` with minimal changes to make it build appropriately.
     
    +    There is a stub documentation and a stub test script though.
    +
         Subsequent commits will flesh out its capabilities and make it a more
         standard regular builtin.
     
    @@ .gitignore
      /git-rerere
      /git-reset
     
    + ## Documentation/git-replay.txt (new) ##
    +@@
    ++git-replay(1)
    ++=============
    ++
    ++NAME
    ++----
    ++git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos too
    ++
    ++
    ++SYNOPSIS
    ++--------
    ++[verse]
    ++'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
    ++
    ++DESCRIPTION
    ++-----------
    ++
    ++Takes a range of commits and replays them onto a new location.
    ++
    ++THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
    ++
    ++OPTIONS
    ++-------
    ++
    ++--onto <newbase>::
    ++  Starting point at which to create the new commits.  May be any
    ++  valid commit, and not just an existing branch name.
    ++
    ++EXIT STATUS
    ++-----------
    ++
    ++For a successful, non-conflicted replay, the exit status is 0.  When
    ++the replay has conflicts, the exit status is 1.  If the replay is not
    ++able to complete (or start) due to some kind of error, the exit status
    ++is something other than 0 or 1.
    ++
    ++GIT
    ++---
    ++Part of the linkgit:git[1] suite
    +
      ## Makefile ##
     @@ Makefile: TEST_BUILTINS_OBJS += test-dump-split-index.o
      TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
      TEST_BUILTINS_OBJS += test-env-helper.o
      TEST_BUILTINS_OBJS += test-example-decorate.o
     -TEST_BUILTINS_OBJS += test-fast-rebase.o
    + TEST_BUILTINS_OBJS += test-find-pack.o
      TEST_BUILTINS_OBJS += test-fsmonitor-client.o
      TEST_BUILTINS_OBJS += test-genrandom.o
    - TEST_BUILTINS_OBJS += test-genzeros.o
     @@ Makefile: BUILTIN_OBJS += builtin/remote-fd.o
      BUILTIN_OBJS += builtin/remote.o
      BUILTIN_OBJS += builtin/repack.o
    @@ t/helper/test-tool.c: static struct test_cmd cmds[] = {
        { "env-helper", cmd__env_helper },
        { "example-decorate", cmd__example_decorate },
     -  { "fast-rebase", cmd__fast_rebase },
    +   { "find-pack", cmd__find_pack },
        { "fsmonitor-client", cmd__fsmonitor_client },
        { "genrandom", cmd__genrandom },
    -   { "genzeros", cmd__genzeros },
     
      ## t/helper/test-tool.h ##
     @@ t/helper/test-tool.h: int cmd__dump_untracked_cache(int argc, const char **argv);
    @@ t/helper/test-tool.h: int cmd__dump_untracked_cache(int argc, const char **argv)
      int cmd__env_helper(int argc, const char **argv);
      int cmd__example_decorate(int argc, const char **argv);
     -int cmd__fast_rebase(int argc, const char **argv);
    + int cmd__find_pack(int argc, const char **argv);
      int cmd__fsmonitor_client(int argc, const char **argv);
      int cmd__genrandom(int argc, const char **argv);
    - int cmd__genzeros(int argc, const char **argv);
    +
    + ## t/t3650-replay-basics.sh (new) ##
    +@@
    ++#!/bin/sh
    ++
    ++test_description='basic git replay tests'
    ++
    ++GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
    ++export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
    ++
    ++. ./test-lib.sh
    ++
    ++GIT_AUTHOR_NAME=author@name
    ++GIT_AUTHOR_EMAIL=bogus@email@address
    ++export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
    ++
    ++test_expect_success 'setup' '
    ++  test_commit A &&
    ++  test_commit B &&
    ++
    ++  git switch -c topic1 &&
    ++  test_commit C &&
    ++  git switch -c topic2 &&
    ++  test_commit D &&
    ++  test_commit E &&
    ++  git switch topic1 &&
    ++  test_commit F &&
    ++  git switch -c topic3 &&
    ++  test_commit G &&
    ++  test_commit H &&
    ++  git switch -c topic4 main &&
    ++  test_commit I &&
    ++  test_commit J &&
    ++
    ++  git switch -c next main &&
    ++  test_commit K &&
    ++  git merge -m "Merge topic1" topic1 &&
    ++  git merge -m "Merge topic2" topic2 &&
    ++  git merge -m "Merge topic3" topic3 &&
    ++  >evil &&
    ++  git add evil &&
    ++  git commit --amend &&
    ++  git merge -m "Merge topic4" topic4 &&
    ++
    ++  git switch main &&
    ++  test_commit L &&
    ++  test_commit M &&
    ++
    ++  git switch -c conflict B &&
    ++  test_commit C.conflict C.t conflict
    ++'
    ++
    ++test_expect_success 'using replay to rebase two branches, one on top of other' '
    ++  git switch main &&
    ++
    ++  git replay --onto main topic1 topic2 >result &&
    ++
    ++  git log --format=%s $(cut -f 3 -d " " result) >actual &&
    ++  test_write_lines E D M L B A >expect &&
    ++  test_cmp expect actual
    ++'
    ++
    ++test_done
     
      ## t/t6429-merge-sequence-rename-caching.sh ##
     @@ t/t6429-merge-sequence-rename-caching.sh: test_expect_success 'caching renames does not preclude finding new ones' '
 3:  11abb9d120 =  3:  a7886952b7 replay: start using parse_options API
 4:  9e568eae84 =  4:  bab0286f0a replay: die() instead of failing assert()
 5:  e7ebf3c5ef =  5:  469e95e5bf replay: introduce pick_regular_commit()
 6:  37d545d5d6 !  6:  edafe4846f replay: change rev walking options
    @@ Commit message
     
         Let's set the rev walking options we need after calling
         setup_revisions() instead of before. This enforces options we always
    -    want.
    -
    -    We want the command to work from older commits to newer ones by default,
    -    but we are Ok with letting users reverse that, using --reverse, if that's
    -    what they really want.
    +    want for now.
     
    +    We want the command to work from older commits to newer ones by default.
         Also we don't want history simplification, as we want to deal with all
         the commits in the affected range.
     
    +    When we see an option that we are going to override, we emit a warning
    +    to avoid confusion as much as possible though.
    +
         Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
         Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
         Signed-off-by: Elijah Newren <newren@gmail.com>
    @@ Commit message
     
      ## builtin/replay.c ##
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
    +   struct merge_result result;
    +   struct strbuf reflog_msg = STRBUF_INIT;
    +   struct strbuf branch_name = STRBUF_INIT;
    +-  int ret = 0;
    ++  int i, ret = 0;
    + 
    +   const char * const replay_usage[] = {
    +           N_("git replay --onto <newbase> <oldbase> <branch>"),
    +@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
      
        repo_init_revisions(the_repository, &revs, prefix);
      
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
     -
        strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
      
    ++  /*
    ++   * TODO: For now, let's warn when we see an option that we are
    ++   * going to override after setup_revisions() below. In the
    ++   * future we might want to either die() or allow them if we
    ++   * think they could be useful though.
    ++   */
    ++  for (i = 0; i < argc; i++) {
    ++          if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
    ++              !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
    ++              !strcmp(argv[i], "--full-history"))
    ++                  warning(_("option '%s' will be overridden"), argv[i]);
    ++  }
    ++
        if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
    -@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
    +           ret = error(_("unhandled options"));
                goto cleanup;
        }
      
     +  /* requirements/overrides for revs */
    -+  revs.reverse = !revs.reverse;
    ++  revs.reverse = 1;
     +  revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
     +  revs.topo_order = 1;
     +  revs.simplify_history = 0;
 7:  2943f08926 =  7:  b81574744a replay: add an important FIXME comment about gpg signing
 8:  f81962ba41 =  8:  b08ad2b2f1 replay: remove progress and info output
 9:  236747497e !  9:  5099c94d2e replay: remove HEAD related sanity check
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
                               REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
                        error(_("could not update %s"), argv[2]);
                        die("Failed to update %s", argv[2]);
    +
    + ## t/t3650-replay-basics.sh ##
    +@@ t/t3650-replay-basics.sh: test_expect_success 'setup' '
    + '
    + 
    + test_expect_success 'using replay to rebase two branches, one on top of other' '
    +-  git switch main &&
    +-
    +   git replay --onto main topic1 topic2 >result &&
    + 
    +   git log --format=%s $(cut -f 3 -d " " result) >actual &&
10:  3374d5be23 ! 10:  b4a5d1edd4 replay: make it a minimal server side command
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
        struct merge_result result;
     -  struct strbuf reflog_msg = STRBUF_INIT;
        struct strbuf branch_name = STRBUF_INIT;
    -   int ret = 0;
    +   int i, ret = 0;
      
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
        onto = peel_committish(onto_name);
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
     +  return ret ? 0 : 1;
      }
     
    + ## t/t3650-replay-basics.sh ##
    +@@ t/t3650-replay-basics.sh: test_expect_success 'setup' '
    +   test_commit C.conflict C.t conflict
    + '
    + 
    ++test_expect_success 'setup bare' '
    ++  git clone --bare . bare
    ++'
    ++
    + test_expect_success 'using replay to rebase two branches, one on top of other' '
    +   git replay --onto main topic1 topic2 >result &&
    + 
    ++  test_line_count = 1 result &&
    ++
    +   git log --format=%s $(cut -f 3 -d " " result) >actual &&
    +   test_write_lines E D M L B A >expect &&
    +-  test_cmp expect actual
    ++  test_cmp expect actual &&
    ++
    ++  printf "update refs/heads/topic2 " >expect &&
    ++  printf "%s " $(cut -f 3 -d " " result) >>expect &&
    ++  git rev-parse topic2 >>expect &&
    ++
    ++  test_cmp expect result
    ++'
    ++
    ++test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
    ++  git -C bare replay --onto main topic1 topic2 >result-bare &&
    ++  test_cmp expect result-bare
    + '
    + 
    + test_done
    +
      ## t/t6429-merge-sequence-rename-caching.sh ##
     @@ t/t6429-merge-sequence-rename-caching.sh: test_expect_success 'caching renames does not preclude finding new ones' '
      
11:  197d076a93 ! 11:  1d8a6ef432 replay: use standard revision ranges
    @@ Commit message
         history, as they could be confusing for most users.
     
         Also as the interface of the command is now mostly finalized,
    -    we can add some documentation as well as testcases to make sure
    +    we can add more documentation and more testcases to make sure
         the command will continue to work as designed in the future.
     
         We only document the rev-list related options among all the
    @@ Commit message
         Signed-off-by: Elijah Newren <newren@gmail.com>
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
    - ## Documentation/git-replay.txt (new) ##
    -@@
    -+git-replay(1)
    -+=============
    -+
    -+NAME
    -+----
    -+git-replay - Replay commits on a new base, works on bare repos too
    -+
    -+
    -+SYNOPSIS
    -+--------
    -+[verse]
    -+'git replay' --onto <newbase> <revision-range>...
    -+
    -+DESCRIPTION
    -+-----------
    -+
    + ## Documentation/git-replay.txt ##
    +@@ Documentation/git-replay.txt: SYNOPSIS
    + DESCRIPTION
    + -----------
    + 
    +-Takes a range of commits and replays them onto a new location.
     +Takes a range of commits and replays them onto a new location. Leaves
     +the working tree and the index untouched, and updates no
     +references. The output of this command is meant to be used as input to
     +`git update-ref --stdin`, which would update the relevant branches
     +(see the OUTPUT section below).
    -+
    -+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
    -+
    -+OPTIONS
    -+-------
    -+
    -+--onto <newbase>::
    -+  Starting point at which to create the new commits.  May be any
    -+  valid commit, and not just an existing branch name.
    + 
    + THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
    + 
    +@@ Documentation/git-replay.txt: OPTIONS
    + --onto <newbase>::
    +   Starting point at which to create the new commits.  May be any
    +   valid commit, and not just an existing branch name.
     ++
     +The update-ref command(s) in the output will update the branch(es) in
     +the revision range to point at the new commits, similar to the way how
    @@ Documentation/git-replay.txt (new)
     +
     +where the number of refs updated depends on the arguments passed and
     +the shape of the history being replayed.
    -+
    -+EXIT STATUS
    -+-----------
    -+
    -+For a successful, non-conflicted replay, the exit status is 0.  When
    -+the replay has conflicts, the exit status is 1.  If the replay is not
    -+able to complete (or start) due to some kind of error, the exit status
    -+is something other than 0 or 1.
    -+
    + 
    + EXIT STATUS
    + -----------
    +@@ Documentation/git-replay.txt: the replay has conflicts, the exit status is 1.  If the replay is not
    + able to complete (or start) due to some kind of error, the exit status
    + is something other than 0 or 1.
    + 
     +EXAMPLES
     +--------
     +
    @@ Documentation/git-replay.txt (new)
     +`origin/main`. These three branches may have commits on top of `base`
     +that they have in common, but that does not need to be the case.
     +
    -+GIT
    -+---
    -+Part of the linkgit:git[1] suite
    + GIT
    + ---
    + Part of the linkgit:git[1] suite
     
      ## builtin/replay.c ##
     @@
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
        struct merge_options merge_opt;
        struct merge_result result;
     -  struct strbuf branch_name = STRBUF_INIT;
    -   int ret = 0;
    +   int i, ret = 0;
      
        const char * const replay_usage[] = {
     -          N_("git replay --onto <newbase> <oldbase> <branch>"),
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
      
     -  strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
     -
    +   /*
    +    * TODO: For now, let's warn when we see an option that we are
    +    * going to override after setup_revisions() below. In the
    +@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
    +                   warning(_("option '%s' will be overridden"), argv[i]);
    +   }
    + 
     -  if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
     -          ret = error(_("unhandled options"));
     +  argc = setup_revisions(argc, argv, &revs, NULL);
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
      
        /* Return */
     
    - ## t/t3650-replay-basics.sh (new) ##
    -@@
    -+#!/bin/sh
    -+
    -+test_description='basic git replay tests'
    -+
    -+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
    -+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
    -+
    -+. ./test-lib.sh
    -+
    -+GIT_AUTHOR_NAME=author@name
    -+GIT_AUTHOR_EMAIL=bogus@email@address
    -+export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
    -+
    -+test_expect_success 'setup' '
    -+  test_commit A &&
    -+  test_commit B &&
    -+
    -+  git switch -c topic1 &&
    -+  test_commit C &&
    -+  git switch -c topic2 &&
    -+  test_commit D &&
    -+  test_commit E &&
    -+  git switch topic1 &&
    -+  test_commit F &&
    -+  git switch -c topic3 &&
    -+  test_commit G &&
    -+  test_commit H &&
    -+  git switch -c topic4 main &&
    -+  test_commit I &&
    -+  test_commit J &&
    -+
    -+  git switch -c next main &&
    -+  test_commit K &&
    -+  git merge -m "Merge topic1" topic1 &&
    -+  git merge -m "Merge topic2" topic2 &&
    -+  git merge -m "Merge topic3" topic3 &&
    -+  >evil &&
    -+  git add evil &&
    -+  git commit --amend &&
    -+  git merge -m "Merge topic4" topic4 &&
    -+
    -+  git switch main &&
    -+  test_commit L &&
    -+  test_commit M &&
    -+
    -+  git switch -c conflict B &&
    -+  test_commit C.conflict C.t conflict
    -+'
    -+
    -+test_expect_success 'setup bare' '
    -+  git clone --bare . bare
    -+'
    -+
    -+test_expect_success 'using replay to rebase two branches, one on top of other' '
    + ## t/t3650-replay-basics.sh ##
    +@@ t/t3650-replay-basics.sh: test_expect_success 'setup bare' '
    + '
    + 
    + test_expect_success 'using replay to rebase two branches, one on top of other' '
    +-  git replay --onto main topic1 topic2 >result &&
     +  git replay --onto main topic1..topic2 >result &&
    -+
    -+  test_line_count = 1 result &&
    -+
    -+  git log --format=%s $(cut -f 3 -d " " result) >actual &&
    -+  test_write_lines E D M L B A >expect &&
    -+  test_cmp expect actual &&
    -+
    -+  printf "update refs/heads/topic2 " >expect &&
    -+  printf "%s " $(cut -f 3 -d " " result) >>expect &&
    -+  git rev-parse topic2 >>expect &&
    -+
    -+  test_cmp expect result
    -+'
    -+
    -+test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
    + 
    +   test_line_count = 1 result &&
    + 
    +@@ t/t3650-replay-basics.sh: test_expect_success 'using replay to rebase two branches, one on top of other' '
    + '
    + 
    + test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
    +-  git -C bare replay --onto main topic1 topic2 >result-bare &&
     +  git -C bare replay --onto main topic1..topic2 >result-bare &&
    -+  test_cmp expect result-bare
    -+'
    -+
    +   test_cmp expect result-bare
    + '
    + 
     +test_expect_success 'using replay to rebase with a conflict' '
     +  test_expect_code 1 git replay --onto topic1 B..conflict
     +'
    @@ t/t3650-replay-basics.sh (new)
     +  test_expect_code 1 git -C bare replay --onto topic1 B..conflict
     +'
     +
    -+test_done
    + test_done
     
      ## t/t6429-merge-sequence-rename-caching.sh ##
     @@ t/t6429-merge-sequence-rename-caching.sh: test_expect_success 'caching renames does not preclude finding new ones' '
12:  e52d8b961c ! 12:  3a207e5d7f replay: add --advance or 'cherry-pick' mode
    @@ Commit message
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## Documentation/git-replay.txt ##
    -@@ Documentation/git-replay.txt: git-replay - Replay commits on a new base, works on bare repos too
    +@@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
      SYNOPSIS
      --------
      [verse]
    --'git replay' --onto <newbase> <revision-range>...
    -+'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
    +-'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
    ++'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
      
      DESCRIPTION
      -----------
    @@ builtin/replay.c: static struct commit *pick_regular_commit(struct commit *pickm
        struct merge_options merge_opt;
        struct merge_result result;
     +  struct strset *update_refs = NULL;
    -   int ret = 0;
    +   int i, ret = 0;
      
        const char * const replay_usage[] = {
     -          N_("git replay --onto <newbase> <revision-range>..."),
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
     -
        repo_init_revisions(the_repository, &revs, prefix);
      
    -   argc = setup_revisions(argc, argv, &revs, NULL);
    +   /*
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
        revs.topo_order = 1;
        revs.simplify_history = 0;
13:  fc79a930b5 ! 13:  23a164196b replay: add --contained to rebase contained branches
    @@ Commit message
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## Documentation/git-replay.txt ##
    -@@ Documentation/git-replay.txt: git-replay - Replay commits on a new base, works on bare repos too
    +@@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
      SYNOPSIS
      --------
      [verse]
    --'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
    -+'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>...
    +-'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
    ++'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
      
      DESCRIPTION
      -----------
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
        struct rev_info revs;
        struct commit *last_commit = NULL;
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
    -   int ret = 0;
    +   int i, ret = 0;
      
        const char * const replay_usage[] = {
     -          N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
     +
        repo_init_revisions(the_repository, &revs, prefix);
      
    -   argc = setup_revisions(argc, argv, &revs, NULL);
    +   /*
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
                        continue;
                while (decoration) {
14:  1160ff54e6 ! 14:  5c4b15cccd replay: stop assuming replayed branches do not diverge
    @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
        struct merge_result result;
        struct strset *update_refs = NULL;
     +  kh_oid_map_t *replayed_commits;
    -   int ret = 0;
    +   int i, ret = 0;
      
        const char * const replay_usage[] = {
     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)


Elijah Newren (14):
  t6429: remove switching aspects of fast-rebase
  replay: introduce new builtin
  replay: start using parse_options API
  replay: die() instead of failing assert()
  replay: introduce pick_regular_commit()
  replay: change rev walking options
  replay: add an important FIXME comment about gpg signing
  replay: remove progress and info output
  replay: remove HEAD related sanity check
  replay: make it a minimal server side command
  replay: use standard revision ranges
  replay: add --advance or 'cherry-pick' mode
  replay: add --contained to rebase contained branches
  replay: stop assuming replayed branches do not diverge

 .gitignore                               |   1 +
 Documentation/git-replay.txt             | 127 +++++++
 Makefile                                 |   2 +-
 builtin.h                                |   1 +
 builtin/replay.c                         | 421 +++++++++++++++++++++++
 command-list.txt                         |   1 +
 git.c                                    |   1 +
 t/helper/test-fast-rebase.c              | 241 -------------
 t/helper/test-tool.c                     |   1 -
 t/helper/test-tool.h                     |   1 -
 t/t3650-replay-basics.sh                 | 198 +++++++++++
 t/t6429-merge-sequence-rename-caching.sh |  45 +--
 12 files changed, 776 insertions(+), 264 deletions(-)
 create mode 100644 Documentation/git-replay.txt
 create mode 100644 builtin/replay.c
 delete mode 100644 t/helper/test-fast-rebase.c
 create mode 100755 t/t3650-replay-basics.sh


base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply

* [PATCH v6 01/14] t6429: remove switching aspects of fast-rebase
From: Christian Couder @ 2023-11-02 13:51 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

At the time t6429 was written, merge-ort was still under development,
did not have quite as many tests, and certainly was not widely deployed.
Since t6429 was exercising some codepaths just a little differently, we
thought having them also test the "merge_switch_to_result()" bits of
merge-ort was useful even though they weren't intrinsic to the real
point of these tests.

However, the value provided by doing extra testing of the
"merge_switch_to_result()" bits has decreased a bit over time, and it's
actively making it harder to refactor `test-tool fast-rebase` into `git
replay`, which we are going to do in following commits.  Dispense with
these bits.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/helper/test-fast-rebase.c              | 9 +--------
 t/t6429-merge-sequence-rename-caching.sh | 9 +++++++--
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/t/helper/test-fast-rebase.c b/t/helper/test-fast-rebase.c
index cac20a72b3..2bfab66b1b 100644
--- a/t/helper/test-fast-rebase.c
+++ b/t/helper/test-fast-rebase.c
@@ -194,7 +194,7 @@ int cmd__fast_rebase(int argc, const char **argv)
 		last_commit = create_commit(result.tree, commit, last_commit);
 	}
 
-	merge_switch_to_result(&merge_opt, head_tree, &result, 1, !result.clean);
+	merge_finalize(&merge_opt, &result);
 
 	if (result.clean < 0)
 		exit(128);
@@ -213,9 +213,6 @@ int cmd__fast_rebase(int argc, const char **argv)
 		}
 		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
 			die(_("unable to update HEAD"));
-
-		prime_cache_tree(the_repository, the_repository->index,
-				 result.tree);
 	} else {
 		fprintf(stderr, "\nAborting: Hit a conflict.\n");
 		strbuf_addf(&reflog_msg, "rebase progress up to %s",
@@ -228,10 +225,6 @@ int cmd__fast_rebase(int argc, const char **argv)
 			die("Failed to update %s", argv[4]);
 		}
 	}
-	if (write_locked_index(&the_index, &lock,
-			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
-		die(_("unable to write %s"), get_index_file());
-
 	ret = (result.clean == 0);
 cleanup:
 	strbuf_release(&reflog_msg);
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index d02fa16614..75d3fd2dba 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -72,6 +72,7 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 		git switch upstream &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream~1..topic
 
 		git ls-files >tracked-files &&
@@ -200,6 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream~1..topic &&
 
 		git ls-files >tracked &&
@@ -277,6 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream~1..topic &&
 
 		git ls-files >tracked &&
@@ -356,8 +359,6 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
 		test_must_fail test-tool fast-rebase --onto HEAD upstream~1 topic >output &&
 		#git cherry-pick upstream..topic &&
 
-		grep CONFLICT..rename/rename output &&
-
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls
 	)
@@ -456,6 +457,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
@@ -522,6 +524,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
@@ -624,6 +627,7 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
@@ -682,6 +686,7 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		export GIT_TRACE2_PERF &&
 
 		test-tool fast-rebase --onto HEAD upstream~1 topic &&
+		git reset --hard topic &&
 		#git cherry-pick upstream..topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
-- 
2.42.0.496.g529a7fda40


^ permalink raw reply related

* Re: why does git set X in LESS env var?
From: Thomas Guyot @ 2023-11-02 13:19 UTC (permalink / raw)
  To: Dragan Simic; +Cc: Junio C Hamano, Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <8022dae27797bf1e1770f099ed37f5d3@manjaro.org>

On 2023-11-02 02:48, Dragan Simic wrote:
> On 2023-11-02 06:48, Thomas Guyot wrote:
>>         -c or --clear-screen ( and backward compat. -C or
>> --CLEAR-SCREEN )
>>                Causes full screen repaints to be painted from the top
>> line down.  By default, full screen repaints are done by scrolling
>> from the  bottom  of the screen.
> AFAIK, the "-c" option is about the way screen contents is updated when
> scrolled, and it exists to aid in resolving possible issues with some
> terminal emulators.  To make sure, I just tested it, and "-c" doesn't
> replace "-X".
That's correct, you need both and also -y0

>>         -yn or --max-forw-scroll=n
>>                Specifies  a  maximum  number of lines to scroll
>> forward.  If it is necessary to scroll forward more than n lines, the
>> screen is repainted in‐
>>                stead.  The -c or -C option may be used to repaint from
>> the top of the screen if desired.  By default, any forward movement
>> causes scrolling.
> This option is, I'd guess, also about aiding in resolving possible
> issues with some terminal emulators.  Or maybe even with some actual
> terminals as pieces of hardware, who knows, which may be too slow to
> scroll many lines at once.

With a value of 0, it effectively redraw the screen on scroll. This 
could have a potential impact on slow connections.

>> I actually have one major issue with it, it's that displaying anything
>> less than a full page will fill the screen with ~ on the bottom, just
>> like when scrolling up on a partial page  without -F. I can see this
>> being a major annoyance when using for ex. git log -1, git show --stat
>> or --name-only, etc. as I  usually do it to keep the latest history
>> within the current screen (and there's likely even commands that I
>> never seen using the pager because I never exceeded the page height).
> Huh, this confuses me a bit, quite frankly.  Isn't the "-F" option used
> specifically to make pagination invisible in case fewer lines than one
> full screen are displayed?

Indeed, but when less update from the bottom, it can add new lines and 
let the overflow lines scroll up into the scrollback buffer.

Then updating it from the top, it draws the whole page, top to bottom. 
That's fine for a full page but not desired for a partial one. Also note 
that on my terminal (rxvt-unicode) when less clears the screen to draw 
the first page the current screen is rolled up into scrollback - iirc 
that's a configurable option, it would be worth testing other terminal's 
behavior on that. IIRC it may also erase it when using the wrong termcap 
file.

I haven't looked at the code, but I think it could be possibly to start 
the -c behavior only after a full page is drawn, after exiting on 
partial pages, which would give us the best of both worlds.

>> OTOH by repainting from the top, the scrollback buffer is never
>> affected. only the last displayed page remains on the terminal.
> Just to clarify, it's the "-X" option that creates all the issues, and
> the "--redraw-on-quit" option is already there to replace it with no
> associated issues, but the trouble is that only newer versions of
> less(1) support the "--redraw-on-quit" option.  IOW, it's all about
> improving less(1) to avoid complex workarounds required to handle
> different versions, such as the workarounds used in bat(1).

TBH I haven't tested --redraw-on-quit, even on Debian Bookworm which was 
just released a couple months ago this option isn't available. I suspect 
that the issue isn't -X, but the scrolling behavior controlled by -y and 
the full redraw controlled by -c.Actually I just tested my solution on 
xfce4-terminal and it doesn't work, the terminal still push up stuff 
above on redraw (noteworthy is with rxvt-unicode the first draw pushes 
the current screen contents up but no other redraw does, which is what 
makes it work so well - I haven't tried to find out what is being done 
exactly... OTOH the redraw on scroll down is slightly noticeable there, 
while impossible to see on xfce4-terminal. I'll install the latest less 
and see what happens with --redraw on
>> If less could only enable this behavior after the first full page
>> draw, that would be perfect!
> Could you, please, elaborate a bit on that?

I mentioned it slightly above, to be clear it would mean that:

1. less starts by just writing lined down as usual, making any lines 
above scroll up and overflow into the scrollback buffer as usual
2.  If less draws less than a page, exits as before - the effective 
result is as if pager was cat
3. If less reaches a full page and still has lines to write, it turns on 
-c's behavior and further updates happen from the top of the screen, 
preventing scroll up (at least on rxvt-unicode)

Now, if all other terms misbehave here, that's an issue, making this 
suggestion mostly useless. And considering the number of Windows users 
we absolutely need to test Windows Terminal, and should probably test 
MacOS's term too (whatever that is).
>> Dragan, that may be useful if you're discussing with less
>> developers...
> We've basically reached some kind of an agreement about the need for a
> good solution, which turned out to be rather complex as a result of
> being quite universal and extensible, which was required for it to,
> hopefully, be accepted into less(1).  Also, the author of less(1) seems
> to be quite busy with some other things, and he prefers to implement new
> features himself.
>
> We've also agreed on another new feature for less(1), hopefully, which
> isn't exactly related, but should be quite useful.  It's about the
> secure mode for less(1).

Feel free to cc me on your next correspondence. If there are mailing 
lists archives for the thread I'll fetch them as needed. We have at 
least one working term/switch combination, which IMO is a better start 
than nothing :)

Regards,

--
Thomas

^ permalink raw reply

* mailing list question
From: Rus Cristian @ 2023-11-02 12:23 UTC (permalink / raw)
  To: git

Hello,

I have never used a mailing list before.
I have a question (a feature) for git scm for windows.
I'm using git scm for a few years. I have updated it whenever there was 
an update.
But the git command line command does not recognize the key loaded in 
pagent and I have to enter the key password every time.
Is there a special setup for this or the git scm does not support pagent 
yet?

Thanks!


^ permalink raw reply

* Re: [PATCH v3 1/2] git-merge-file doc: drop "-file" from argument placeholders
From: Martin Ågren @ 2023-11-02  9:29 UTC (permalink / raw)
  To: brian m. carlson, Martin Ågren, Junio C Hamano, git,
	Elijah Newren, Phillip Wood, Eric Sunshine, Taylor Blau
In-Reply-To: <ZUNpVyQYUT8TA6An@tapette.crustytoothpaste.net>

On Thu, 2 Nov 2023 at 10:18, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2023-11-02 at 08:53:36, Martin Ågren wrote:

> > Hmm, yeah. Or just "three non-filename arguments". I do wonder: doesn't
> > this mean that the second patch could/should possibly move away from the
> > notion of "object ID"/`--object-id`? (That's not trying to shift any
> > blame from one patch to the other, that's my honest reaction.)
>
> Not specifying an option would make this ambiguous.  What if I have a
> file named "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"?  Is that the
> empty blob, or is it that file?  Normally we have ways to disambiguate
> this, but those don't work here because of the positional arguments.

I didn't intend to suggest dropping the new option altogether, sorry for
being unclear. I meant moving from `--object-id` to something else, such
as `--blob` that you mention. Completely agreed that something explicit
is needed here and that heuristics aren't possible.

> I think the name is fine.  We don't typically use the phrase "blob ID"
> anywhere, but we do say "object ID".  We'd need to say "--blob", but
> I'm not sure that's an improvement, and I fear it may be less
> understandable.

Agreed. Thanks.

Martin

^ permalink raw reply

* Re: [PATCH v3 1/2] git-merge-file doc: drop "-file" from argument placeholders
From: brian m. carlson @ 2023-11-02  9:18 UTC (permalink / raw)
  To: Martin Ågren
  Cc: Junio C Hamano, git, Elijah Newren, Phillip Wood, Eric Sunshine,
	Taylor Blau
In-Reply-To: <CAN0heSrv7MPcEwkq4uEtv9uBbqm4FLKQLE3gdsEbqKkxPXOj5A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2013 bytes --]

On 2023-11-02 at 08:53:36, Martin Ågren wrote:
> On Thu, 2 Nov 2023 at 00:53, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> >
> > > From: Martin Ågren <martin.agren@gmail.com>
> > >
> > > `git merge-file` takes three positional arguments. Each of them is
> > > documented as `<foo-file>`. In preparation for teaching this command to
> > > alternatively take three object IDs, make these placeholders a bit more
> >
> > Minor nit.  Don't we want to say "three blob object names"?  Unless
> > we plan to grow this feature into accepting three tree object names,
> > that is.
> 
> Hmm, yeah. Or just "three non-filename arguments". I do wonder: doesn't
> this mean that the second patch could/should possibly move away from the
> notion of "object ID"/`--object-id`? (That's not trying to shift any
> blame from one patch to the other, that's my honest reaction.)

Not specifying an option would make this ambiguous.  What if I have a
file named "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"?  Is that the
empty blob, or is it that file?  Normally we have ways to disambiguate
this, but those don't work here because of the positional arguments.

> Ah, yes, I thought I recognized this. Quoting your response [1] to v2:
> 
> > I briefly thought about suggesting --blob-id instead of --object-id
> > simply because you'd never want to feed it trees and commits, but
> > the error message from read_mmblob() the users would get mentions
> > 'blob' to signal that non-blob objects are unwelcome, so the name of
> > the optionwould be OK as-is.
> 
> Maybe you having a similar reaction a second time makes this smell a bit
> more?

I think the name is fine.  We don't typically use the phrase "blob ID"
anywhere, but we do say "object ID".  We'd need to say "--blob", but
I'm not sure that's an improvement, and I fear it may be less
understandable.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox