Skip to content

Commit 4dbeecb

Browse files
gitstergitster
committed
Merge branch 'ag/edit-todo-drop-check'
Allow the rebase.missingCommitsCheck configuration to kick in when "rebase --edit-todo" and "rebase --continue" restarts the procedure. * ag/edit-todo-drop-check: rebase-interactive: warn if commit is dropped with `rebase --edit-todo' sequencer: move check_todo_list_from_file() to rebase-interactive.c
2 parents f7f43af + 5a5445d commit 4dbeecb

5 files changed

+214
-49
lines changed

‎rebase-interactive.c

+76-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
#include "strbuf.h"
66
#include "commit-slab.h"
77
#include "config.h"
8+
#include "dir.h"
9+
10+
static const char edit_todo_list_advice[] =
11+
N_("You can fix this with 'git rebase --edit-todo' "
12+
"and then run 'git rebase --continue'.\n"
13+
"Or you can abort the rebase with 'git rebase"
14+
" --abort'.\n");
815

916
enum missing_commit_check_level {
1017
MISSING_COMMIT_CHECK_IGNORE = 0,
@@ -91,21 +98,24 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
9198
struct todo_list *new_todo, const char *shortrevisions,
9299
const char *shortonto, unsigned flags)
93100
{
94-
const char *todo_file = rebase_path_todo();
101+
const char *todo_file = rebase_path_todo(),
102+
*todo_backup = rebase_path_todo_backup();
95103
unsigned initial = shortrevisions && shortonto;
104+
int incorrect = 0;
96105

97106
/* If the user is editing the todo list, we first try to parse
98107
* it. If there is an error, we do not return, because the user
99108
* might want to fix it in the first place. */
100109
if (!initial)
101-
todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list);
110+
incorrect = todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list) |
111+
file_exists(rebase_path_dropped());
102112

103113
if (todo_list_write_to_file(r, todo_list, todo_file, shortrevisions, shortonto,
104114
-1, flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP))
105115
return error_errno(_("could not write '%s'"), todo_file);
106116

107-
if (initial &&
108-
todo_list_write_to_file(r, todo_list, rebase_path_todo_backup(),
117+
if (!incorrect &&
118+
todo_list_write_to_file(r, todo_list, todo_backup,
109119
shortrevisions, shortonto, -1,
110120
(flags | TODO_LIST_APPEND_TODO_HELP) & ~TODO_LIST_SHORTEN_IDS) < 0)
111121
return error(_("could not write '%s'."), rebase_path_todo_backup());
@@ -117,10 +127,23 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
117127
if (initial && new_todo->buf.len == 0)
118128
return -3;
119129

120-
/* For the initial edit, the todo list gets parsed in
121-
* complete_action(). */
122-
if (!initial)
123-
return todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo);
130+
if (todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo)) {
131+
fprintf(stderr, _(edit_todo_list_advice));
132+
return -4;
133+
}
134+
135+
if (incorrect) {
136+
if (todo_list_check_against_backup(r, new_todo)) {
137+
write_file(rebase_path_dropped(), "");
138+
return -4;
139+
}
140+
141+
if (incorrect > 0)
142+
unlink(rebase_path_dropped());
143+
} else if (todo_list_check(todo_list, new_todo)) {
144+
write_file(rebase_path_dropped(), "");
145+
return -4;
146+
}
124147

125148
return 0;
126149
}
@@ -185,7 +208,52 @@ int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
185208
"the level of warnings.\n"
186209
"The possible behaviours are: ignore, warn, error.\n\n"));
187210

211+
fprintf(stderr, _(edit_todo_list_advice));
212+
188213
leave_check:
189214
clear_commit_seen(&commit_seen);
190215
return res;
191216
}
217+
218+
int todo_list_check_against_backup(struct repository *r, struct todo_list *todo_list)
219+
{
220+
struct todo_list backup = TODO_LIST_INIT;
221+
int res = 0;
222+
223+
if (strbuf_read_file(&backup.buf, rebase_path_todo_backup(), 0) > 0) {
224+
todo_list_parse_insn_buffer(r, backup.buf.buf, &backup);
225+
res = todo_list_check(&backup, todo_list);
226+
}
227+
228+
todo_list_release(&backup);
229+
return res;
230+
}
231+
232+
int check_todo_list_from_file(struct repository *r)
233+
{
234+
struct todo_list old_todo = TODO_LIST_INIT, new_todo = TODO_LIST_INIT;
235+
int res = 0;
236+
237+
if (strbuf_read_file(&new_todo.buf, rebase_path_todo(), 0) < 0) {
238+
res = error(_("could not read '%s'."), rebase_path_todo());
239+
goto out;
240+
}
241+
242+
if (strbuf_read_file(&old_todo.buf, rebase_path_todo_backup(), 0) < 0) {
243+
res = error(_("could not read '%s'."), rebase_path_todo_backup());
244+
goto out;
245+
}
246+
247+
res = todo_list_parse_insn_buffer(r, old_todo.buf.buf, &old_todo);
248+
if (!res)
249+
res = todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo);
250+
if (res)
251+
fprintf(stderr, _(edit_todo_list_advice));
252+
if (!res)
253+
res = todo_list_check(&old_todo, &new_todo);
254+
out:
255+
todo_list_release(&old_todo);
256+
todo_list_release(&new_todo);
257+
258+
return res;
259+
}

‎rebase-interactive.h

+5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ void append_todo_help(unsigned keep_empty, int command_count,
1111
int edit_todo_list(struct repository *r, struct todo_list *todo_list,
1212
struct todo_list *new_todo, const char *shortrevisions,
1313
const char *shortonto, unsigned flags);
14+
1415
int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
16+
int todo_list_check_against_backup(struct repository *r,
17+
struct todo_list *todo_list);
18+
19+
int check_todo_list_from_file(struct repository *r);
1520

1621
#endif

‎sequencer.c

+11-40
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
5757
GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
5858
GIT_PATH_FUNC(rebase_path_todo_backup, "rebase-merge/git-rebase-todo.backup")
5959

60+
GIT_PATH_FUNC(rebase_path_dropped, "rebase-merge/dropped")
61+
6062
/*
6163
* The rebase command lines that have already been processed. A line
6264
* is moved here when it is first handled, before any associated user
@@ -4225,6 +4227,14 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
42254227
if (is_rebase_i(opts)) {
42264228
if ((res = read_populate_todo(r, &todo_list, opts)))
42274229
goto release_todo_list;
4230+
4231+
if (file_exists(rebase_path_dropped())) {
4232+
if ((res = todo_list_check_against_backup(r, &todo_list)))
4233+
goto release_todo_list;
4234+
4235+
unlink(rebase_path_dropped());
4236+
}
4237+
42284238
if (commit_staged_changes(r, opts, &todo_list)) {
42294239
res = -1;
42304240
goto release_todo_list;
@@ -4971,41 +4981,6 @@ int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
49714981
return res;
49724982
}
49734983

4974-
static const char edit_todo_list_advice[] =
4975-
N_("You can fix this with 'git rebase --edit-todo' "
4976-
"and then run 'git rebase --continue'.\n"
4977-
"Or you can abort the rebase with 'git rebase"
4978-
" --abort'.\n");
4979-
4980-
int check_todo_list_from_file(struct repository *r)
4981-
{
4982-
struct todo_list old_todo = TODO_LIST_INIT, new_todo = TODO_LIST_INIT;
4983-
int res = 0;
4984-
4985-
if (strbuf_read_file_or_whine(&new_todo.buf, rebase_path_todo()) < 0) {
4986-
res = -1;
4987-
goto out;
4988-
}
4989-
4990-
if (strbuf_read_file_or_whine(&old_todo.buf, rebase_path_todo_backup()) < 0) {
4991-
res = -1;
4992-
goto out;
4993-
}
4994-
4995-
res = todo_list_parse_insn_buffer(r, old_todo.buf.buf, &old_todo);
4996-
if (!res)
4997-
res = todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo);
4998-
if (!res)
4999-
res = todo_list_check(&old_todo, &new_todo);
5000-
if (res)
5001-
fprintf(stderr, _(edit_todo_list_advice));
5002-
out:
5003-
todo_list_release(&old_todo);
5004-
todo_list_release(&new_todo);
5005-
5006-
return res;
5007-
}
5008-
50094984
/* skip picking commits whose parents are unchanged */
50104985
static int skip_unnecessary_picks(struct repository *r,
50114986
struct todo_list *todo_list,
@@ -5103,11 +5078,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
51035078
todo_list_release(&new_todo);
51045079

51055080
return error(_("nothing to do"));
5106-
}
5107-
5108-
if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) ||
5109-
todo_list_check(todo_list, &new_todo)) {
5110-
fprintf(stderr, _(edit_todo_list_advice));
5081+
} else if (res == -4) {
51115082
checkout_onto(r, opts, onto_name, &onto->object.oid, orig_head);
51125083
todo_list_release(&new_todo);
51135084

‎sequencer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const char *git_path_commit_editmsg(void);
1111
const char *git_path_seq_dir(void);
1212
const char *rebase_path_todo(void);
1313
const char *rebase_path_todo_backup(void);
14+
const char *rebase_path_dropped(void);
1415

1516
#define APPEND_SIGNOFF_DEDUP (1u << 0)
1617

@@ -155,7 +156,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
155156

156157
void todo_list_add_exec_commands(struct todo_list *todo_list,
157158
struct string_list *commands);
158-
int check_todo_list_from_file(struct repository *r);
159159
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
160160
const char *shortrevisions, const char *onto_name,
161161
struct commit *onto, const char *orig_head, struct string_list *commands,

‎t/t3404-rebase-interactive.sh

+121
Original file line numberDiff line numberDiff line change
@@ -1463,6 +1463,127 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
14631463
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
14641464
'
14651465

1466+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
1467+
test_config rebase.missingCommitsCheck ignore &&
1468+
rebase_setup_and_clean missing-commit &&
1469+
(
1470+
set_fake_editor &&
1471+
FAKE_LINES="break 1 2 3 4 5" git rebase -i --root &&
1472+
FAKE_LINES="1 2 3 4" git rebase --edit-todo &&
1473+
git rebase --continue 2>actual
1474+
) &&
1475+
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
1476+
test_i18ngrep \
1477+
"Successfully rebased and updated refs/heads/missing-commit" \
1478+
actual
1479+
'
1480+
1481+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = warn' '
1482+
cat >expect <<-EOF &&
1483+
error: invalid line 1: badcmd $(git rev-list --pretty=oneline --abbrev-commit -1 master~4)
1484+
Warning: some commits may have been dropped accidentally.
1485+
Dropped commits (newer to older):
1486+
- $(git rev-list --pretty=oneline --abbrev-commit -1 master)
1487+
- $(git rev-list --pretty=oneline --abbrev-commit -1 master~4)
1488+
To avoid this message, use "drop" to explicitly remove a commit.
1489+
EOF
1490+
head -n4 expect >expect.2 &&
1491+
tail -n1 expect >>expect.2 &&
1492+
tail -n4 expect.2 >expect.3 &&
1493+
test_config rebase.missingCommitsCheck warn &&
1494+
rebase_setup_and_clean missing-commit &&
1495+
(
1496+
set_fake_editor &&
1497+
test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
1498+
git rebase -i --root &&
1499+
cp .git/rebase-merge/git-rebase-todo.backup orig &&
1500+
FAKE_LINES="2 3 4" git rebase --edit-todo 2>actual.2 &&
1501+
head -n6 actual.2 >actual &&
1502+
test_i18ncmp expect actual &&
1503+
cp orig .git/rebase-merge/git-rebase-todo &&
1504+
FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual.2 &&
1505+
head -n4 actual.2 >actual &&
1506+
test_i18ncmp expect.3 actual &&
1507+
git rebase --continue 2>actual
1508+
) &&
1509+
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
1510+
test_i18ngrep \
1511+
"Successfully rebased and updated refs/heads/missing-commit" \
1512+
actual
1513+
'
1514+
1515+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = error' '
1516+
cat >expect <<-EOF &&
1517+
error: invalid line 1: badcmd $(git rev-list --pretty=oneline --abbrev-commit -1 master~4)
1518+
Warning: some commits may have been dropped accidentally.
1519+
Dropped commits (newer to older):
1520+
- $(git rev-list --pretty=oneline --abbrev-commit -1 master)
1521+
- $(git rev-list --pretty=oneline --abbrev-commit -1 master~4)
1522+
To avoid this message, use "drop" to explicitly remove a commit.
1523+
1524+
Use '\''git config rebase.missingCommitsCheck'\'' to change the level of warnings.
1525+
The possible behaviours are: ignore, warn, error.
1526+
1527+
You can fix this with '\''git rebase --edit-todo'\'' and then run '\''git rebase --continue'\''.
1528+
Or you can abort the rebase with '\''git rebase --abort'\''.
1529+
EOF
1530+
tail -n11 expect >expect.2 &&
1531+
head -n3 expect.2 >expect.3 &&
1532+
tail -n7 expect.2 >>expect.3 &&
1533+
test_config rebase.missingCommitsCheck error &&
1534+
rebase_setup_and_clean missing-commit &&
1535+
(
1536+
set_fake_editor &&
1537+
test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
1538+
git rebase -i --root &&
1539+
cp .git/rebase-merge/git-rebase-todo.backup orig &&
1540+
test_must_fail env FAKE_LINES="2 3 4" \
1541+
git rebase --edit-todo 2>actual &&
1542+
test_i18ncmp expect actual &&
1543+
test_must_fail git rebase --continue 2>actual &&
1544+
test_i18ncmp expect.2 actual &&
1545+
test_must_fail git rebase --edit-todo &&
1546+
cp orig .git/rebase-merge/git-rebase-todo &&
1547+
test_must_fail env FAKE_LINES="1 2 3 4" \
1548+
git rebase --edit-todo 2>actual &&
1549+
test_i18ncmp expect.3 actual &&
1550+
test_must_fail git rebase --continue 2>actual &&
1551+
test_i18ncmp expect.3 actual &&
1552+
cp orig .git/rebase-merge/git-rebase-todo &&
1553+
FAKE_LINES="1 2 3 4 drop 5" git rebase --edit-todo &&
1554+
git rebase --continue 2>actual
1555+
) &&
1556+
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
1557+
test_i18ngrep \
1558+
"Successfully rebased and updated refs/heads/missing-commit" \
1559+
actual
1560+
'
1561+
1562+
test_expect_success 'rebase.missingCommitsCheck = error after resolving conflicts' '
1563+
test_config rebase.missingCommitsCheck error &&
1564+
(
1565+
set_fake_editor &&
1566+
FAKE_LINES="drop 1 break 2 3 4" git rebase -i A E
1567+
) &&
1568+
git rebase --edit-todo &&
1569+
test_must_fail git rebase --continue &&
1570+
echo x >file1 &&
1571+
git add file1 &&
1572+
git rebase --continue
1573+
'
1574+
1575+
test_expect_success 'rebase.missingCommitsCheck = error when editing for a second time' '
1576+
test_config rebase.missingCommitsCheck error &&
1577+
(
1578+
set_fake_editor &&
1579+
FAKE_LINES="1 break 2 3" git rebase -i A D &&
1580+
cp .git/rebase-merge/git-rebase-todo todo &&
1581+
test_must_fail env FAKE_LINES=2 git rebase --edit-todo &&
1582+
GIT_SEQUENCE_EDITOR="cp todo" git rebase --edit-todo &&
1583+
git rebase --continue
1584+
)
1585+
'
1586+
14661587
test_expect_success 'respects rebase.abbreviateCommands with fixup, squash and exec' '
14671588
rebase_setup_and_clean abbrevcmd &&
14681589
test_commit "first" file1.txt "first line" first &&

0 commit comments

Comments
 (0)