rebase -r: make 'merge -c' behave like reword
If the user runs git log while rewording a commit it is confusing if sometimes we're amending the commit that's being reworded and at other times we're creating a new commit depending on whether we could fast-forward or not[1]. For this reason the reword command ensures that there are no uncommitted changes when rewording. The reword command also allows the user to edit the todo list while the rebase is paused. As 'merge -c' also rewords commits make it behave like reword and add a test. [1] https://lore.kernel.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Aug 20, 2021 at 15:40 UTC
2be6b6f411136c6773a90d2cfdacf3cccb79d3e6
3 files changed
+75
-19
sequencer.c
+14
-10
@@ -3739,10 +3739,9 @@ static struct commit *lookup_label(const char *label, int len,
3739
static int do_merge(struct repository *r,
3740
struct commit *commit,
3741
const char *arg, int arg_len,
3742
- int flags, struct replay_opts *opts)
3742
+ int flags, int *check_todo, struct replay_opts *opts)
3743
{
3744
- int run_commit_flags = (flags & TODO_EDIT_MERGE_MSG) ?
3745
- EDIT_MSG | VERIFY_MSG : 0;
3744
+ int run_commit_flags = 0;
3745
struct strbuf ref_name = STRBUF_INIT;
3746
struct commit *head_commit, *merge_commit, *i;
3747
struct commit_list *bases, *j, *reversed = NULL;
@@ -3898,10 +3897,9 @@ static int do_merge(struct repository *r,
3897
rollback_lock_file(&lock);
3898
ret = fast_forward_to(r, &commit->object.oid,
3899
&head_commit->object.oid, 0, opts);
3901
- if (flags & TODO_EDIT_MERGE_MSG) {
3902
- run_commit_flags |= AMEND_MSG;
3900
+ if (flags & TODO_EDIT_MERGE_MSG)
3901
goto fast_forward_edit;
3904
- }
3902
+
3903
goto leave_merge;
3904
}
3905
@@ -4035,10 +4033,17 @@ static int do_merge(struct repository *r,
4033
* value (a negative one would indicate that the `merge`
4034
* command needs to be rescheduled).
4035
*/
4038
- fast_forward_edit:
4036
ret = !!run_git_commit(git_path_merge_msg(r), opts,
4037
run_commit_flags);
4038
4039
+ if (!ret && flags & TODO_EDIT_MERGE_MSG) {
4040
+ fast_forward_edit:
4041
+ *check_todo = 1;
4042
+ run_commit_flags |= AMEND_MSG | EDIT_MSG | VERIFY_MSG;
4043
+ ret = !!run_git_commit(NULL, opts, run_commit_flags);
4044
+ }
4045
+
4046
+
4047
leave_merge:
4048
strbuf_release(&ref_name);
4049
rollback_lock_file(&lock);
@@ -4405,9 +4410,8 @@ static int pick_commits(struct repository *r,
4410
if ((res = do_reset(r, arg, item->arg_len, opts)))
4411
reschedule = 1;
4412
} else if (item->command == TODO_MERGE) {
4408
- if ((res = do_merge(r, item->commit,
4409
- arg, item->arg_len,
4410
- item->flags, opts)) < 0)
4413
+ if ((res = do_merge(r, item->commit, arg, item->arg_len,
4414
+ item->flags, &check_todo, opts)) < 0)
4415
reschedule = 1;
4416
else if (item->commit)
4417
record_in_rewritten(&item->commit->object.oid,
t/lib-rebase.sh
+50
@@ -151,3 +151,53 @@ test_editor_unchanged () {
151
EOF
152
test_cmp expect actual
153
}
154
+
155
+# Set up an editor for testing reword commands
156
+# Checks that there are no uncommitted changes when rewording and that the
157
+# todo-list is reread after each
158
+set_reword_editor () {
159
+ >reword-actual &&
160
+ >reword-oid &&
161
+
162
+ # Check rewording keeps the original authorship
163
+ GIT_AUTHOR_NAME="Reword Author"
164
+ GIT_AUTHOR_EMAIL="reword.author@example.com"
165
+ GIT_AUTHOR_DATE=@123456
166
+
167
+ write_script reword-sequence-editor.sh <<-\EOF &&
168
+ todo="$(cat "$1")" &&
169
+ echo "exec git log -1 --pretty=format:'%an <%ae> %at%n%B%n' \
170
+ >>reword-actual" >"$1" &&
171
+ printf "%s\n" "$todo" >>"$1"
172
+ EOF
173
+
174
+ write_script reword-editor.sh <<-EOF &&
175
+ # Save the oid of the first reworded commit so we can check rebase
176
+ # fast-forwards to it
177
+ if ! test -s reword-oid
178
+ then
179
+ git rev-parse HEAD >reword-oid
180
+ fi &&
181
+ # There should be no uncommited changes
182
+ git diff --exit-code HEAD &&
183
+ # The todo-list should be re-read after a reword
184
+ GIT_SEQUENCE_EDITOR="\"$PWD/reword-sequence-editor.sh\"" \
185
+ git rebase --edit-todo &&
186
+ echo edited >>"\$1"
187
+ EOF
188
+
189
+ test_set_editor "$PWD/reword-editor.sh"
190
+}
191
+
192
+# Check the results of a rebase after calling set_reword_editor
193
+# Pass the commits that were reworded in the order that they were picked
194
+# Expects the first pick to be a fast-forward
195
+check_reworded_commits () {
196
+ test_cmp_rev "$(cat reword-oid)" "$1^{commit}" &&
197
+ git log --format="%an <%ae> %at%n%B%nedited%n" --no-walk=unsorted "$@" \
198
+ >reword-expected &&
199
+ test_cmp reword-expected reword-actual &&
200
+ git log --format="%an <%ae> %at%n%B" -n $# --first-parent --reverse \
201
+ >reword-log &&
202
+ test_cmp reword-expected reword-log
203
+}
t/t3430-rebase-merges.sh
+11
-9
@@ -172,17 +172,19 @@ test_expect_success 'failed `merge <branch>` does not crash' '
172
grep "^Merge branch ${SQ}G${SQ}$" .git/rebase-merge/message
173
'
174
175
-test_expect_success 'fast-forward merge -c still rewords' '
176
- git checkout -b fast-forward-merge-c H &&
175
+test_expect_success 'merge -c commits before rewording and reloads todo-list' '
176
+ cat >script-from-scratch <<-\EOF &&
177
+ merge -c E B
178
+ merge -c H G
179
+ EOF
180
+
181
+ git checkout -b merge-c H &&
182
(
178
- set_fake_editor &&
179
- FAKE_COMMIT_MESSAGE=edited \
180
- GIT_SEQUENCE_EDITOR="echo merge -c H G >" \
181
- git rebase -ir @^
183
+ set_reword_editor &&
184
+ GIT_SEQUENCE_EDITOR="\"$PWD/replace-editor.sh\"" \
185
+ git rebase -i -r D
186
) &&
183
- echo edited >expected &&
184
- git log --pretty=format:%B -1 >actual &&
185
- test_cmp expected actual
187
+ check_reworded_commits E H
188
'
189
190
test_expect_success 'with a branch tip that was cherry-picked already' '