diff mbox series

[v3,11/15] replay: use standard revision ranges

Message ID 20230602102533.876905-12-christian.couder@gmail.com (mailing list archive)
State New, archived
Headers show
Series Introduce new `git replay` command | expand

Commit Message

Christian Couder June 2, 2023, 10:25 a.m. UTC
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.

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

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             | 88 ++++++++++++++++++++++++
 builtin/replay.c                         | 21 ++----
 t/t3650-replay-basics.sh                 | 83 ++++++++++++++++++++++
 t/t6429-merge-sequence-rename-caching.sh | 18 ++---
 4 files changed, 184 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/git-replay.txt
 create mode 100755 t/t3650-replay-basics.sh

Comments

Toon claes June 22, 2023, 10:03 a.m. UTC | #1
Christian Couder <christian.couder@gmail.com> writes:

> +DESCRIPTION
> +-----------
> +
> +Takes a range of commits, and replays them onto a new location.  Does
> +not touch the working tree or index, and does not update any
> +references.  However, the output of this command is meant to be used

Small suggestion here:

Takes a range of commits, and replays them onto a new location.  Does
neither touch the working tree nor index, and does not update any
references.
Christian Couder Sept. 7, 2023, 8:32 a.m. UTC | #2
On Thu, Jun 22, 2023 at 12:03 PM Toon Claes <toon@iotcl.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > +DESCRIPTION
> > +-----------
> > +
> > +Takes a range of commits, and replays them onto a new location.  Does
> > +not touch the working tree or index, and does not update any
> > +references.  However, the output of this command is meant to be used
>
> Small suggestion here:
>
> Takes a range of commits, and replays them onto a new location.  Does
> neither touch the working tree nor index, and does not update any
> references.

I am not a native speaker, so I am not sure what's best here. I find
your suggestion a bit less clear though, so until a native speaker
agrees with it or maybe finds something even better, I prefer to leave
it as-is.
Dragan Simic Sept. 7, 2023, 9:02 p.m. UTC | #3
On 2023-09-07 10:32, Christian Couder wrote:
> On Thu, Jun 22, 2023 at 12:03 PM Toon Claes <toon@iotcl.com> wrote:
>> 
>> Christian Couder <christian.couder@gmail.com> writes:
>> 
>> > +DESCRIPTION
>> > +-----------
>> > +
>> > +Takes a range of commits, and replays them onto a new location.  Does
>> > +not touch the working tree or index, and does not update any
>> > +references.  However, the output of this command is meant to be used
>> 
>> Small suggestion here:
>> 
>> Takes a range of commits, and replays them onto a new location.  Does
>> neither touch the working tree nor index, and does not update any
>> references.
> 
> I am not a native speaker, so I am not sure what's best here. I find
> your suggestion a bit less clear though, so until a native speaker
> agrees with it or maybe finds something even better, I prefer to leave
> it as-is.

I'm also not a native English speaker, but I spent about 2.5 years 
contributing a whole lot to English Wikipedia, so I'd dare to say I've 
honed my English skills rather well.  Thus, here's my take on this:

     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 to be used...

This is written in a concise yet slightly imperative way, which should 
be suitable for the purpose.  I hope you agree.
Christian Couder Oct. 10, 2023, 12:44 p.m. UTC | #4
On Thu, Sep 7, 2023 at 11:02 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> On 2023-09-07 10:32, Christian Couder wrote:
> > On Thu, Jun 22, 2023 at 12:03 PM Toon Claes <toon@iotcl.com> wrote:
> >>
> >> Christian Couder <christian.couder@gmail.com> writes:
> >>
> >> > +DESCRIPTION
> >> > +-----------
> >> > +
> >> > +Takes a range of commits, and replays them onto a new location.  Does
> >> > +not touch the working tree or index, and does not update any
> >> > +references.  However, the output of this command is meant to be used
> >>
> >> Small suggestion here:
> >>
> >> Takes a range of commits, and replays them onto a new location.  Does
> >> neither touch the working tree nor index, and does not update any
> >> references.
> >
> > I am not a native speaker, so I am not sure what's best here. I find
> > your suggestion a bit less clear though, so until a native speaker
> > agrees with it or maybe finds something even better, I prefer to leave
> > it as-is.
>
> I'm also not a native English speaker, but I spent about 2.5 years
> contributing a whole lot to English Wikipedia, so I'd dare to say I've
> honed my English skills rather well.  Thus, here's my take on this:
>
>      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 to be used...
>
> This is written in a concise yet slightly imperative way, which should
> be suitable for the purpose.  I hope you agree.

I agree and I like it, so I have changed it to the above in version 5
I just sent.

Thanks!
Dragan Simic Oct. 10, 2023, 2:02 p.m. UTC | #5
On 2023-10-10 14:44, Christian Couder wrote:
> On Thu, Sep 7, 2023 at 11:02 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> 
>> On 2023-09-07 10:32, Christian Couder wrote:
>> > On Thu, Jun 22, 2023 at 12:03 PM Toon Claes <toon@iotcl.com> wrote:
>> >>
>> >> Christian Couder <christian.couder@gmail.com> writes:
>> >>
>> >> > +DESCRIPTION
>> >> > +-----------
>> >> > +
>> >> > +Takes a range of commits, and replays them onto a new location.  Does
>> >> > +not touch the working tree or index, and does not update any
>> >> > +references.  However, the output of this command is meant to be used
>> >>
>> >> Small suggestion here:
>> >>
>> >> Takes a range of commits, and replays them onto a new location.  Does
>> >> neither touch the working tree nor index, and does not update any
>> >> references.
>> >
>> > I am not a native speaker, so I am not sure what's best here. I find
>> > your suggestion a bit less clear though, so until a native speaker
>> > agrees with it or maybe finds something even better, I prefer to leave
>> > it as-is.
>> 
>> I'm also not a native English speaker, but I spent about 2.5 years
>> contributing a whole lot to English Wikipedia, so I'd dare to say I've
>> honed my English skills rather well.  Thus, here's my take on this:
>> 
>>      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 to be used...
>> 
>> This is written in a concise yet slightly imperative way, which should
>> be suitable for the purpose.  I hope you agree.
> 
> I agree and I like it, so I have changed it to the above in version 5
> I just sent.

Great, thanks.
diff mbox series

Patch

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
new file mode 100644
index 0000000000..394d7b0050
--- /dev/null
+++ b/Documentation/git-replay.txt
@@ -0,0 +1,88 @@ 
+git-replay(1)
+=============
+
+NAME
+----
+git-replay - Replay commits on a different base, without touching working tree
+
+
+SYNOPSIS
+--------
+[verse]
+'git replay' --onto <newbase> <revision-range>...
+
+DESCRIPTION
+-----------
+
+Takes a range of commits, and replays them onto a new location.  Does
+not touch the working tree or index, and does not update any
+references.  However, 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.
+
+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 commands in the output will update the branch(es)
+in the revision range to point at the new commits (in other
+words, this mimics a rebase operation).
+
+<revision-range>::
+	Range of commits to replay; see "Specifying Ranges" in
+	linkgit:git-rev-parse.
+
+OUTPUT
+------
+
+When there are no conflicts, the output of this command is usable as
+input to `git update-ref --stdin`.  It is basically 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 depend on the arguments passed.
+
+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.
+
+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 9385973ffc..c1bd72c0e5 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 ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay --onto <newbase> <oldbase> <branch>"),
+		N_("git replay --onto <newbase> <revision-range>..."),
 		NULL
 	};
 	struct option replay_options[] = {
@@ -145,20 +142,13 @@  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);
-
-	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;
 	}
 
@@ -168,8 +158,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;
@@ -212,7 +200,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
new file mode 100755
index 0000000000..a1da4f9ef9
--- /dev/null
+++ b/t/t3650-replay-basics.sh
@@ -0,0 +1,83 @@ 
+#!/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' '
+	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' '
+	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 &&