commit/reset: try to clean up sequencer state
When cherry-picking or reverting a sequence of commits and if the final pick/revert has conflicts and the user uses `git commit` to commit the conflict resolution and does not run `git cherry-pick --continue` then the sequencer state is left behind. This can cause problems later. In my case I cherry-picked a sequence of commits the last one of which I committed with `git commit` after resolving some conflicts, then a while later, on a different branch I aborted a revert which rewound my HEAD to the end of the cherry-pick sequence on the previous branch. Avoid this potential problem by removing the sequencer state if we're committing or resetting the final pick in a sequence. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Apr 16, 2019 at 11:18 UTC
b07d9bfd171d881a02d110b68673c58cb63e9f62
5 files changed
+94
-4
branch.c
+2
-2
@@ -5,6 +5,7 @@
5
#include "refs.h"
6
#include "refspec.h"
7
#include "remote.h"
8
+#include "sequencer.h"
9
#include "commit.h"
10
#include "worktree.h"
11
@@ -339,8 +340,7 @@ void create_branch(struct repository *r,
340
341
void remove_branch_state(struct repository *r)
342
{
342
- unlink(git_path_cherry_pick_head(r));
343
- unlink(git_path_revert_head(r));
343
+ sequencer_post_commit_cleanup(r);
344
unlink(git_path_merge_head(r));
345
unlink(git_path_merge_rr(r));
346
unlink(git_path_merge_msg(r));
builtin/commit.c
+1
-2
@@ -1657,8 +1657,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1657
die("%s", err.buf);
1658
}
1659
1660
- unlink(git_path_cherry_pick_head(the_repository));
1661
- unlink(git_path_revert_head(the_repository));
1660
+ sequencer_post_commit_cleanup(the_repository);
1661
unlink(git_path_merge_head(the_repository));
1662
unlink(git_path_merge_msg(the_repository));
1663
unlink(git_path_merge_mode(the_repository));
sequencer.c
+51
@@ -2220,6 +2220,57 @@ static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
2220
return len;
2221
}
2222
2223
+static int have_finished_the_last_pick(void)
2224
+{
2225
+ struct strbuf buf = STRBUF_INIT;
2226
+ const char *eol;
2227
+ const char *todo_path = git_path_todo_file();
2228
+ int ret = 0;
2229
+
2230
+ if (strbuf_read_file(&buf, todo_path, 0) < 0) {
2231
+ if (errno == ENOENT) {
2232
+ return 0;
2233
+ } else {
2234
+ error_errno("unable to open '%s'", todo_path);
2235
+ return 0;
2236
+ }
2237
+ }
2238
+ /* If there is only one line then we are done */
2239
+ eol = strchr(buf.buf, '\n');
2240
+ if (!eol || !eol[1])
2241
+ ret = 1;
2242
+
2243
+ strbuf_release(&buf);
2244
+
2245
+ return ret;
2246
+}
2247
+
2248
+void sequencer_post_commit_cleanup(struct repository *r)
2249
+{
2250
+ struct replay_opts opts = REPLAY_OPTS_INIT;
2251
+ int need_cleanup = 0;
2252
+
2253
+ if (file_exists(git_path_cherry_pick_head(r))) {
2254
+ unlink(git_path_cherry_pick_head(r));
2255
+ opts.action = REPLAY_PICK;
2256
+ need_cleanup = 1;
2257
+ }
2258
+
2259
+ if (file_exists(git_path_revert_head(r))) {
2260
+ unlink(git_path_revert_head(r));
2261
+ opts.action = REPLAY_REVERT;
2262
+ need_cleanup = 1;
2263
+ }
2264
+
2265
+ if (!need_cleanup)
2266
+ return;
2267
+
2268
+ if (!have_finished_the_last_pick())
2269
+ return;
2270
+
2271
+ sequencer_remove_state(&opts);
2272
+}
2273
+
2274
static int read_populate_todo(struct repository *r,
2275
struct todo_list *todo_list,
2276
struct replay_opts *opts)
sequencer.h
+1
@@ -144,3 +144,4 @@ int read_author_script(const char *path, char **name, char **email, char **date,
144
void parse_strategy_opts(struct replay_opts *opts, char *raw_opts);
145
int write_basic_state(struct replay_opts *opts, const char *head_name,
146
const char *onto, const char *orig_head);
147
+void sequencer_post_commit_cleanup(struct repository *r);
t/t3507-cherry-pick-conflict.sh
+39
@@ -156,6 +156,25 @@ test_expect_success 'successful commit clears CHERRY_PICK_HEAD' '
156
157
test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
158
'
159
+test_expect_success 'successful final commit clears cherry-pick state' '
160
+ pristine_detach initial &&
161
+
162
+ test_must_fail git cherry-pick base picked-signed &&
163
+ echo resolved >foo &&
164
+ test_path_is_file .git/sequencer/todo &&
165
+ git commit -a &&
166
+ test_must_fail test_path_exists .git/sequencer
167
+'
168
+
169
+test_expect_success 'reset after final pick clears cherry-pick state' '
170
+ pristine_detach initial &&
171
+
172
+ test_must_fail git cherry-pick base picked-signed &&
173
+ echo resolved >foo &&
174
+ test_path_is_file .git/sequencer/todo &&
175
+ git reset &&
176
+ test_must_fail test_path_exists .git/sequencer
177
+'
178
179
test_expect_success 'failed cherry-pick produces dirty index' '
180
pristine_detach initial &&
@@ -316,6 +335,26 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' '
335
test_cmp_rev picked REVERT_HEAD
336
'
337
338
+test_expect_success 'successful final commit clears revert state' '
339
+ pristine_detach picked-signed &&
340
+
341
+ test_must_fail git revert picked-signed base &&
342
+ echo resolved >foo &&
343
+ test_path_is_file .git/sequencer/todo &&
344
+ git commit -a &&
345
+ test_must_fail test_path_exists .git/sequencer
346
+'
347
+
348
+test_expect_success 'reset after final pick clears revert state' '
349
+ pristine_detach picked-signed &&
350
+
351
+ test_must_fail git revert picked-signed base &&
352
+ echo resolved >foo &&
353
+ test_path_is_file .git/sequencer/todo &&
354
+ git reset &&
355
+ test_must_fail test_path_exists .git/sequencer
356
+'
357
+
358
test_expect_success 'revert conflict, diff3 -m style' '
359
pristine_detach initial &&
360
git config merge.conflictstyle diff3 &&