Thread (287 messages) 287 messages, 11 authors, 2018-10-08

Re: [PATCH 6/8] sequencer: handle autosquash and post-rewrite for merge commands

From: Johannes Schindelin <hidden>
Date: 2018-01-18 21:29:37
Subsystem: the rest · Maintainer: Linus Torvalds

Hi Jake,

On Thu, 18 Jan 2018, Jacob Keller wrote:
On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
[off-list ref] wrote:
quoted
In the previous patches, we implemented the basic functionality of the
`git rebase -i --recreate-merges` command, in particular the `merge`
command to create merge commits in the sequencer.

The interactive rebase is a lot more these days, though, than a simple
cherry-pick in a loop. For example, it calls the post-rewrite hook (if
any) after rebasing with a mapping of the old->new commits. And the
interactive rebase also supports the autosquash mode, where commits
whose oneline is of the form `fixup! <oneline>` or `squash! <oneline>`
are rearranged to amend commits whose oneline they match.

This patch implements the post-rewrite and autosquash handling for the
`merge` command we just introduced. The other commands that were added
recently (`label`, `reset` and `bud`) do not create new commits,
therefore post-rewrite & autosquash do not need to handle them.

Signed-off-by: Johannes Schindelin <redacted>
---
 refs.c                            |  3 ++-
 sequencer.c                       | 10 +++++++---
 t/t3430-rebase-recreate-merges.sh | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 20ba82b4343..e8b84c189ff 100644
--- a/refs.c
+++ b/refs.c
@@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
        return !strcmp(refname, "HEAD") ||
-               starts_with(refname, "refs/bisect/");
+               starts_with(refname, "refs/bisect/") ||
+               starts_with(refname, "refs/rewritten/");
 }
Would this part make more sense to move into the commit that
introduces writing these refs, or does it only matter once you start
this step here?
quoted
 static int is_pseudoref_syntax(const char *refname)
diff --git a/sequencer.c b/sequencer.c
index 1bef16647b4..b63bfb9a141 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2413,10 +2413,13 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
                        res = do_reset(item->arg, item->arg_len);
                else if (item->command == TODO_BUD)
                        res = do_reset("onto", 4);
-               else if (item->command == TODO_MERGE)
+               else if (item->command == TODO_MERGE) {
                        res = do_merge(item->commit,
                                       item->arg, item->arg_len, opts);
-               else if (!is_noop(item->command))
+                       if (item->commit)
+                               record_in_rewritten(&item->commit->object.oid,
+                                                   peek_command(todo_list, 1));
+               } else if (!is_noop(item->command))
                        return error(_("unknown command %d"), item->command);

                todo_list->current++;
@@ -3556,7 +3559,8 @@ int rearrange_squash(void)
                struct subject2item_entry *entry;

                next[i] = tail[i] = -1;
-               if (item->command >= TODO_EXEC) {
+               if (item->command >= TODO_EXEC &&
+                   (item->command != TODO_MERGE || !item->commit)) {
                        subjects[i] = NULL;
                        continue;
                }
diff --git a/t/t3430-rebase-recreate-merges.sh b/t/t3430-rebase-recreate-merges.sh
index 46ae52f88b3..76e615bd7c1 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -143,4 +143,43 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
        EOF
 '

+test_expect_success 'refs/rewritten/* is worktree-local' '
+       git worktree add wt &&
+       cat >wt/script-from-scratch <<-\EOF &&
+       label xyz
+       exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a || :
+       exec git rev-parse --verify refs/rewritten/xyz >b
+       EOF
+
+       test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+       git -C wt rebase -i HEAD &&
+       test_must_be_empty wt/a &&
+       test_cmp_rev HEAD "$(cat wt/b)"
+'
+
Same for the test here, I can't figure out why this is necessary in
this patch as opposed to the patch which first introduced the
refs/rewritten/<label> refs.
Woops. This was its own commit, and must have been accidentally squashed
during one of my rebases. Will re-introduce it; this is roughly what it
will look like:

-- snipsnap --
Author: Johannes Schindelin [off-list ref]
Subject: sequencer: make refs generated by the `label` command worktree-local

This allows for rebases to be run in parallel in separate worktrees
(think: interrupted in the middle of one rebase, being asked to perform
a different rebase, adding a separate worktree just for that job).

Signed-off-by: Johannes Schindelin <redacted>
diff --git a/refs.c b/refs.c
index 20ba82b4343..e8b84c189ff 100644
--- a/refs.c
+++ b/refs.c
@@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
 	return !strcmp(refname, "HEAD") ||
-		starts_with(refname, "refs/bisect/");
+		starts_with(refname, "refs/bisect/") ||
+		starts_with(refname, "refs/rewritten/");
 }
 
 static int is_pseudoref_syntax(const char *refname)
diff --git a/t/t3430-rebase-recreate-merges.sh b/t/t3430-rebase-recreate-merges.sh
index f5e476d948b..50a8eefc81e 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -115,4 +115,18 @@ test_expect_success 'generate correct todo list' '
 	test_cmp expect output
 '
 
+test_expect_success 'refs/rewritten/* is worktree-local' '
+	git worktree add wt &&
+	cat >wt/script-from-scratch <<-\EOF &&
+	label xyz
+	exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a || :
+	exec git rev-parse --verify refs/rewritten/xyz >b
+	EOF
+
+	test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+	git -C wt rebase -i HEAD &&
+	test_must_be_empty wt/a &&
+	test_cmp_rev HEAD "$(cat wt/b)"
+'
+
 test_done
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help