Skip to content

Commit cc9dcde

Browse files
newrengitster
authored andcommitted
sequencer: avoid adding exec commands for non-commit creating commands
The `--exec <cmd>` is documented as Append "exec <cmd>" after each line creating a commit in the final history. ... If --autosquash is used, "exec" lines will not be appended for the intermediate commits, and will only appear at the end of each squash/fixup series. Unfortunately, it would also add exec commands after non-pick operations, such as 'no-op', which could be seen for example with git rebase -i --exec true HEAD todo_list_add_exec_commands() intent was to insert exec commands after each logical pick, while trying to consider a chains of fixup and squash commits to be part of the pick before it. So it would keep an 'insert' boolean tracking if it had seen a pick or merge, but not write the exec command until it saw the next non-fixup/squash command. Since that would make it miss the final exec command, it had some code that would check whether it still needed to insert one at the end, but instead of a simple if (insert) it had a if (insert || <condition that is always true>) That's buggy; as per the docs, we should only add exec commands for lines that create commits, i.e. only if insert is true. Fix the conditional. There was one testcase in the testsuite that we tweak for this change; it was introduced in 54fd324 ("rebase -i: reread the todo list if `exec` touched it", 2017-04-26), and was merely testing that after an exec had fired that the todo list would be re-read. The test at the time would have worked given any revision at all, though it would only work with 'HEAD' as a side-effect of this bug. Since we're fixing this bug, choose something other than 'HEAD' for that test. Finally, add a testcase that verifies when we have no commits to pick, that we get no exec lines in the generated todo list. Reported-by: Nikita Bobko <[email protected]> Signed-off-by: Elijah Newren <[email protected]> Acked-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e9d7761 commit cc9dcde

File tree

2 files changed

+7
-2
lines changed

2 files changed

+7
-2
lines changed

sequencer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5496,7 +5496,7 @@ static void todo_list_add_exec_commands(struct todo_list *todo_list,
54965496
}
54975497

54985498
/* insert or append final <commands> */
5499-
if (insert || nr == todo_list->nr) {
5499+
if (insert) {
55005500
ALLOC_GROW(items, nr + commands->nr, alloc);
55015501
COPY_ARRAY(items + nr, base_items, commands->nr);
55025502
nr += commands->nr;

t/t3429-rebase-edit-todo.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,15 @@ test_expect_success 'setup' '
1313

1414
test_expect_success 'rebase exec modifies rebase-todo' '
1515
todo=.git/rebase-merge/git-rebase-todo &&
16-
git rebase HEAD -x "echo exec touch F >>$todo" &&
16+
git rebase HEAD~1 -x "echo exec touch F >>$todo" &&
1717
test -e F
1818
'
1919

20+
test_expect_success 'rebase exec with an empty list does not exec anything' '
21+
git rebase HEAD -x "true" 2>output &&
22+
! grep "Executing: true" output
23+
'
24+
2025
test_expect_success 'loose object cache vs re-reading todo list' '
2126
GIT_REBASE_TODO=.git/rebase-merge/git-rebase-todo &&
2227
export GIT_REBASE_TODO &&

0 commit comments

Comments
 (0)