2017-04-26 19:17:40 +00:00
|
|
|
#!/bin/sh
|
|
|
|
|
|
|
|
|
|
test_description='rebase should reread the todo file if an exec modifies it'
|
|
|
|
|
|
|
|
|
|
. ./test-lib.sh
|
2019-08-19 09:18:22 +00:00
|
|
|
. "$TEST_DIRECTORY"/lib-rebase.sh
|
|
|
|
|
|
|
|
|
|
test_expect_success 'setup' '
|
|
|
|
|
test_commit first file &&
|
|
|
|
|
test_commit second file &&
|
|
|
|
|
test_commit third file
|
|
|
|
|
'
|
2017-04-26 19:17:40 +00:00
|
|
|
|
|
|
|
|
test_expect_success 'rebase exec modifies rebase-todo' '
|
|
|
|
|
todo=.git/rebase-merge/git-rebase-todo &&
|
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 54fd3243da ("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 <nikitabobko@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-30 03:58:39 +00:00
|
|
|
git rebase HEAD~1 -x "echo exec touch F >>$todo" &&
|
2017-04-26 19:17:40 +00:00
|
|
|
test -e F
|
|
|
|
|
'
|
|
|
|
|
|
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 54fd3243da ("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 <nikitabobko@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-30 03:58:39 +00:00
|
|
|
test_expect_success 'rebase exec with an empty list does not exec anything' '
|
|
|
|
|
git rebase HEAD -x "true" 2>output &&
|
|
|
|
|
! grep "Executing: true" output
|
|
|
|
|
'
|
|
|
|
|
|
2019-10-28 00:58:57 +00:00
|
|
|
test_expect_success 'loose object cache vs re-reading todo list' '
|
rebase -i: demonstrate obscure loose object cache bug
We specifically support `exec` commands in `git rebase -i`'s todo lists
to rewrite the very same todo list. Of course, we need to validate that
todo list when re-reading it.
It is also totally legitimate to extend the todo list by `pick` lines
using short names of commits that were created only after the rebase
started.
And this is where the loose object cache interferes with this feature:
if *some* loose object was read whose hash shares the same first two
digits with a commit that was not yet created when that loose object was
created, then we fail to find that new commit by its short name in
`get_oid()`, and the interactive rebase fails with an obscure error
message like:
error: invalid line 1: pick 6568fef
error: please fix this using 'git rebase --edit-todo'.
Let's first demonstrate that this is actually a bug in a new regression
test, in a separate commit so that other developers who do not believe
me can cherry-pick it to confirm the problem.
This new regression test generates two commits whose hashes share the
first two hex digits (so that their corresponding loose objects live in
the same subdirectory of .git/objects/, and are therefore supposed to be
in the same loose object cache bin).
It then picks the first, to make sure that the loose object cache is
initialized and cached that object directory, then generates the second
commit and picks it, too. Since the commit was generated in a different
process than the sequencer that wants to pick it, the loose object cache
had no chance of being updated in the meantime.
Technically, we would need only one `exec` command in this regression
test case, but for ease of implementation, it uses a pseudo-recursive
call to the same script.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-13 10:16:31 +00:00
|
|
|
GIT_REBASE_TODO=.git/rebase-merge/git-rebase-todo &&
|
|
|
|
|
export GIT_REBASE_TODO &&
|
|
|
|
|
write_script append-todo.sh <<-\EOS &&
|
|
|
|
|
# For values 5 and 6, this yields SHA-1s with the same first two digits
|
|
|
|
|
echo "pick $(git rev-parse --short \
|
|
|
|
|
$(printf "%s\\n" \
|
|
|
|
|
"tree $EMPTY_TREE" \
|
|
|
|
|
"author A U Thor <author@example.org> $1 +0000" \
|
|
|
|
|
"committer A U Thor <author@example.org> $1 +0000" \
|
|
|
|
|
"" \
|
|
|
|
|
"$1" |
|
|
|
|
|
git hash-object -t commit -w --stdin))" >>$GIT_REBASE_TODO
|
|
|
|
|
|
|
|
|
|
shift
|
|
|
|
|
test -z "$*" ||
|
|
|
|
|
echo "exec $0 $*" >>$GIT_REBASE_TODO
|
|
|
|
|
EOS
|
|
|
|
|
|
|
|
|
|
git rebase HEAD -x "./append-todo.sh 5 6"
|
|
|
|
|
'
|
|
|
|
|
|
2019-08-19 09:18:22 +00:00
|
|
|
test_expect_success 'todo is re-read after reword and squash' '
|
|
|
|
|
write_script reword-editor.sh <<-\EOS &&
|
|
|
|
|
GIT_SEQUENCE_EDITOR="echo \"exec echo $(cat file) >>actual\" >>" \
|
|
|
|
|
git rebase --edit-todo
|
|
|
|
|
EOS
|
|
|
|
|
|
|
|
|
|
test_write_lines first third >expected &&
|
|
|
|
|
set_fake_editor &&
|
|
|
|
|
GIT_SEQUENCE_EDITOR="$EDITOR" FAKE_LINES="reword 1 squash 2 fixup 3" \
|
|
|
|
|
GIT_EDITOR=./reword-editor.sh git rebase -i --root third &&
|
|
|
|
|
test_cmp expected actual
|
|
|
|
|
'
|
|
|
|
|
|
sequencer: don't re-read todo for revert and cherry-pick
When 'git revert' or 'git cherry-pick --edit' is invoked with multiple
commits, then after editing the first commit message is finished both
these commands should continue with processing the second commit and
launch another editor for its commit message, assuming there are
no conflicts, of course.
Alas, this inadvertently changed with commit a47ba3c777 (rebase -i:
check for updated todo after squash and reword, 2019-08-19): after
editing the first commit message is finished, both 'git revert' and
'git cherry-pick --edit' exit with error, claiming that "nothing to
commit, working tree clean".
The reason for the changed behaviour is twofold:
- Prior to a47ba3c777 the up-to-dateness of the todo list file was
only checked after 'exec' instructions, and that commit moved
those checks to the common code path. The intention was that this
check should be performed after instructions spawning an editor
('squash' and 'reword') as well, so the ongoing 'rebase -i'
notices when the user runs a 'git rebase --edit-todo' while
squashing/rewording a commit message.
However, as it happened that check is now performed even after
'revert' and 'pick' instructions when they involved editing the
commit message. And 'revert' by default while 'pick' optionally
(with 'git cherry-pick --edit') involves editing the commit
message.
- When invoking 'git revert' or 'git cherry-pick --edit' with
multiple commits they don't read a todo list file but assemble the
todo list in memory, thus the associated stat data used to check
whether the file has been updated is all zeroed out initially.
Then the sequencer writes all instructions (including the very
first) to the todo file, executes the first 'revert/pick'
instruction, and after the user finished editing the commit
message the changes of a47ba3c777 kick in, and it checks whether
the todo file has been modified. The initial all-zero stat data
obviously differs from the todo file's current stat data, so the
sequencer concludes that the file has been modified. Technically
it is not wrong, of course, because the file just has been written
indeed by the sequencer itself, though the file's contents still
match what the sequencer was invoked with in the beginning.
Consequently, after re-reading the todo file the sequencer
executes the same first instruction _again_, thus ending up in
that "nothing to commit" situation.
The todo list was never meant to be edited during multi-commit 'git
revert' or 'cherry-pick' operations, so perform that "has the todo
file been modified" check only when the sequencer was invoked as part
of an interactive rebase.
Reported-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-23 17:20:46 +00:00
|
|
|
test_expect_success 're-reading todo doesnt interfere with revert --edit' '
|
|
|
|
|
git reset --hard third &&
|
|
|
|
|
|
|
|
|
|
git revert --edit third second &&
|
|
|
|
|
|
|
|
|
|
cat >expect <<-\EOF &&
|
|
|
|
|
Revert "second"
|
|
|
|
|
Revert "third"
|
|
|
|
|
third
|
|
|
|
|
second
|
|
|
|
|
first
|
|
|
|
|
EOF
|
|
|
|
|
git log --format="%s" >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 're-reading todo doesnt interfere with cherry-pick --edit' '
|
|
|
|
|
git reset --hard first &&
|
|
|
|
|
|
|
|
|
|
git cherry-pick --edit second third &&
|
|
|
|
|
|
|
|
|
|
cat >expect <<-\EOF &&
|
|
|
|
|
third
|
|
|
|
|
second
|
|
|
|
|
first
|
|
|
|
|
EOF
|
|
|
|
|
git log --format="%s" >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2017-04-26 19:17:40 +00:00
|
|
|
test_done
|