git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[GSoC][PATCH v8 4/5] cherry-pick/revert: add --skip option

From
Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Date
Jul 2, 2019, 09:11 UTC
Message-ID
<20190702091129.7531-5-rohit.ashiwal265@gmail.com>
In-Reply-To
<20190702091129.7531-1-rohit.ashiwal265@gmail.com>

git am or rebase have a --skip flag to skip the current commit if the user wishes to do so. During a cherry-pick or revert a user could likewise skip a commit, but needs to use 'git reset' (or in the case of conflicts 'git reset --merge'), followed by 'git (cherry-pick | revert) --continue' to skip the commit. This is more annoying and sometimes confusing on the users' part. Add a `--skip` option to make skipping commits easier for the user and to make the commands more consistent.

In the next commit, we will change the advice messages hence finishing the process of teaching revert and cherry-pick "how to skip commits".

Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
---
 Documentation/git-cherry-pick.txt |   4 +-
 Documentation/git-revert.txt      |   4 +-
 Documentation/sequencer.txt       |   4 ++
 builtin/revert.c                  |   5 ++
 sequencer.c                       |  73 +++++++++++++++++++++
 sequencer.h                       |   1 +
 t/t3510-cherry-pick-sequence.sh   | 102 ++++++++++++++++++++++++++++++
 7 files changed, 187 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 754b16ce0c..83ce51aedf 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,9 +10,7 @@ SYNOPSIS
 [verse]
 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
 		  [-S[<keyid>]] <commit>...
-'git cherry-pick' --continue
-'git cherry-pick' --quit
-'git cherry-pick' --abort
+'git cherry-pick' (--continue | --skip | --abort | --quit)
 
 DESCRIPTION
 -----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 0c82ca5bc0..665e065ee3 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -9,9 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<keyid>]] <commit>...
-'git revert' --continue
-'git revert' --quit
-'git revert' --abort
+'git revert' (--continue | --skip | --abort | --quit)
 
 DESCRIPTION
 -----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 5a57c4a407..3bceb56474 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -3,6 +3,10 @@
 	`.git/sequencer`.  Can be used to continue after resolving
 	conflicts in a failed cherry-pick or revert.
 
+--skip::
+	Skip the current commit and continue with the rest of the
+	sequence.
+
 --quit::
 	Forget about the current operation in progress.  Can be used
 	to clear the sequencer state after a failed cherry-pick or
diff --git a/builtin/revert.c b/builtin/revert.c
index d4dcedbdc6..5dc5891ea2 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
 		OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
 		OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
+		OPT_CMDMODE(0, "skip", &cmd, N_("skip current commit and continue"), 's'),
 		OPT_CLEANUP(&cleanup_arg),
 		OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
 		OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
@@ -151,6 +152,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 			this_operation = "--quit";
 		else if (cmd == 'c')
 			this_operation = "--continue";
+		else if (cmd == 's')
+			this_operation = "--skip";
 		else {
 			assert(cmd == 'a');
 			this_operation = "--abort";
@@ -210,6 +213,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 		return sequencer_continue(the_repository, opts);
 	if (cmd == 'a')
 		return sequencer_rollback(the_repository, opts);
+	if (cmd == 's')
+		return sequencer_skip(the_repository, opts);
 	return sequencer_pick_revisions(the_repository, opts);
 }
 
diff --git a/sequencer.c b/sequencer.c
index 70efe36ee8..f5e3d60878 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2762,6 +2762,15 @@ static int rollback_single_pick(struct repository *r)
 	return reset_merge(&head_oid);
 }
 
+static int skip_single_pick(void)
+{
+	struct object_id head;
+
+	if (read_ref_full("HEAD", 0, &head, NULL))
+		return error(_("cannot resolve HEAD"));
+	return reset_merge(&head);
+}
+
 int sequencer_rollback(struct repository *r, struct replay_opts *opts)
 {
 	FILE *f;
@@ -2811,6 +2820,70 @@ int sequencer_rollback(struct repository *r, struct replay_opts *opts)
 	return -1;
 }
 
+int sequencer_skip(struct repository *r, struct replay_opts *opts)
+{
+	enum replay_action action = -1;
+	sequencer_get_last_command(r, &action);
+
+	/*
+	 * Check whether the subcommand requested to skip the commit is actually
+	 * in progress and that it's safe to skip the commit.
+	 *
+	 * opts->action tells us which subcommand requested to skip the commit.
+	 * If the corresponding .git/<ACTION>_HEAD exists, we know that the
+	 * action is in progress and we can skip the commit.
+	 *
+	 * Otherwise we check that the last instruction was related to the
+	 * particular subcommand we're trying to execute and barf if that's not
+	 * the case.
+	 *
+	 * Finally we check that the rollback is "safe", i.e., has the HEAD
+	 * moved? In this case, it doesn't make sense to "reset the merge" and
+	 * "skip the commit" as the user already handled this by committing. But
+	 * we'd not want to barf here, instead give advice on how to proceed. We
+	 * only need to check that when .git/<ACTION>_HEAD doesn't exist because
+	 * it gets removed when the user commits, so if it still exists we're
+	 * sure the user can't have committed before.
+	 */
+	switch (opts->action) {
+	case REPLAY_REVERT:
+		if (!file_exists(git_path_revert_head(r))) {
+			if (action != REPLAY_REVERT)
+				return error(_("no revert in progress"));
+			if (!rollback_is_safe())
+				goto give_advice;
+		}
+		break;
+	case REPLAY_PICK:
+		if (!file_exists(git_path_cherry_pick_head(r))) {
+			if (action != REPLAY_PICK)
+				return error(_("no cherry-pick in progress"));
+			if (!rollback_is_safe())
+				goto give_advice;
+		}
+		break;
+	default:
+		BUG("unexpected action in sequencer_skip");
+	}
+
+	if (skip_single_pick())
+		return error(_("failed to skip the commit"));
+	if (!is_directory(git_path_seq_dir()))
+		return 0;
+
+	return sequencer_continue(r, opts);
+
+give_advice:
+	error(_("there is nothing to skip"));
+
+	if (advice_resolve_conflict) {
+		advise(_("have you committed already?\n"
+			 "try \"git %s --continue\""),
+			 action == REPLAY_REVERT ? "revert" : "cherry-pick");
+	}
+	return -1;
+}
+
 static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
 {
 	struct lock_file todo_lock = LOCK_INIT;
diff --git a/sequencer.h b/sequencer.h
index 0c494b83d4..731b9853eb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -129,6 +129,7 @@ int sequencer_pick_revisions(struct repository *repo,
 			     struct replay_opts *opts);
 int sequencer_continue(struct repository *repo, struct replay_opts *opts);
 int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
+int sequencer_skip(struct repository *repo, struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
 #define TODO_LIST_KEEP_EMPTY (1U << 0)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 941d5026da..20515ea37b 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -93,6 +93,108 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '
 	test_path_is_missing .git/sequencer
 '
 
+test_expect_success 'cherry-pick --skip requires cherry-pick in progress' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick --skip
+'
+
+test_expect_success 'revert --skip requires revert in progress' '
+	pristine_detach initial &&
+	test_must_fail git revert --skip
+'
+
+test_expect_success 'cherry-pick --skip to skip commit' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick anotherpick &&
+	test_must_fail git revert --skip &&
+	git cherry-pick --skip &&
+	test_cmp_rev initial HEAD &&
+	test_path_is_missing .git/CHERRY_PICK_HEAD
+'
+
+test_expect_success 'revert --skip to skip commit' '
+	pristine_detach anotherpick &&
+	test_must_fail git revert anotherpick~1 &&
+	test_must_fail git cherry-pick --skip &&
+	git revert --skip &&
+	test_cmp_rev anotherpick HEAD
+'
+
+test_expect_success 'skip "empty" commit' '
+	pristine_detach picked &&
+	test_commit dummy foo d &&
+	test_must_fail git cherry-pick anotherpick &&
+	git cherry-pick --skip &&
+	test_cmp_rev dummy HEAD
+'
+
+test_expect_success 'skip a commit and check if rest of sequence is correct' '
+	pristine_detach initial &&
+	echo e >expect &&
+	cat >expect.log <<-EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_must_fail git cherry-pick base..yetanotherpick &&
+	test_must_fail git cherry-pick --skip &&
+	echo d >foo &&
+	git add foo &&
+	git cherry-pick --continue &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$OID_REGEX/OBJID/g"
+	} >actual.log &&
+	test_cmp expect foo &&
+	test_cmp expect.log actual.log
+'
+
+test_expect_success 'check advice when we move HEAD by committing' '
+	pristine_detach initial &&
+	cat >expect <<-EOF &&
+	error: there is nothing to skip
+	hint: have you committed already?
+	hint: try "git cherry-pick --continue"
+	fatal: cherry-pick failed
+	EOF
+	test_must_fail git cherry-pick base..yetanotherpick &&
+	echo c >foo &&
+	git commit -a &&
+	test_path_is_missing .git/CHERRY_PICK_HEAD &&
+	test_must_fail git cherry-pick --skip 2>advice &&
+	test_i18ncmp expect advice
+'
+
+test_expect_success 'allow skipping commit but not abort for a new history' '
+	pristine_detach initial &&
+	cat >expect <<-EOF &&
+	error: cannot abort from a branch yet to be born
+	fatal: cherry-pick failed
+	EOF
+	git checkout --orphan new_disconnected &&
+	git reset --hard &&
+	test_must_fail git cherry-pick anotherpick &&
+	test_must_fail git cherry-pick --abort 2>advice &&
+	git cherry-pick --skip &&
+	test_i18ncmp expect advice
+'
+
+test_expect_success 'allow skipping stopped cherry-pick because of untracked file modifications' '
+	pristine_detach initial &&
+	git rm --cached unrelated &&
+	git commit -m "untrack unrelated" &&
+	test_must_fail git cherry-pick initial base &&
+	test_path_is_missing .git/CHERRY_PICK_HEAD &&
+	git cherry-pick --skip
+'
+
 test_expect_success '--quit does not complain when no cherry-pick is in progress' '
 	pristine_detach initial &&
 	git cherry-pick --quit

-- 
2.21.0
Previous: Rohit AshiwalNext: Rohit Ashiwal
Message 93 of 95 in “Teach cherry-pick/revert to skip commits”
  1. Rohit AshiwalJun 8, 2019
  2. [GSoC][PATCH 1/3] sequencer: add advice for revertRohit Ashiwal, Jun 8, 2019
  3. Phillip WoodJun 9, 2019
  4. Rohit AshiwalJun 10, 2019
  5. Phillip WoodJun 10, 2019
  6. Rohit AshiwalJun 10, 2019
  7. Phillip WoodJun 10, 2019
  8. Junio C HamanoJun 10, 2019
  9. [GSoC][PATCH 2/3] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 8, 2019
  10. Thomas GummererJun 9, 2019
  11. Phillip WoodJun 9, 2019
  12. Rohit AshiwalJun 10, 2019
  13. Phillip WoodJun 10, 2019
  14. Rohit AshiwalJun 10, 2019
  15. Phillip WoodJun 10, 2019
  16. [GSoC][PATCH 3/3] cherry-pick/revert: update hintsRohit Ashiwal, Jun 8, 2019
  17. Thomas GummererJun 9, 2019
  18. Phillip WoodJun 9, 2019
  19. Rohit AshiwalJun 10, 2019
  20. Phillip WoodJun 10, 2019
  21. Rohit AshiwalJun 10, 2019
  22. Phillip WoodJun 10, 2019
  23. Thomas GummererJun 9, 2019
  24. Rohit AshiwalJun 9, 2019
  25. Thomas GummererJun 9, 2019
  26. [GSoC][PATCH v2 0/3] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 11, 2019
  27. [GSoC][PATCH v2 1/3] sequencer: add advice for revertRohit Ashiwal, Jun 11, 2019
  28. Junio C HamanoJun 11, 2019
  29. [GSoC][PATCH v2 3/3] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 11, 2019
  30. Phillip WoodJun 12, 2019
  31. [GSoC][PATCH v2 2/3] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 11, 2019
  32. Phillip WoodJun 12, 2019
  33. Junio C HamanoJun 12, 2019
  34. Phillip WoodJun 12, 2019
  35. [GSoC][PATCH v3 0/3] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 13, 2019
  36. [GSoC][PATCH v3 3/3] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 13, 2019
  37. [GSoC][PATCH v3 2/3] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 13, 2019
  38. Junio C HamanoJun 13, 2019
  39. Junio C HamanoJun 13, 2019
  40. Rohit AshiwalJun 14, 2019
  41. Rohit AshiwalJun 14, 2019
  42. Junio C HamanoJun 14, 2019
  43. Rohit AshiwalJun 16, 2019
  44. Phillip WoodJun 13, 2019
  45. [GSoC][PATCH v3 1/3] sequencer: add advice for revertRohit Ashiwal, Jun 13, 2019
  46. Phillip WoodJun 13, 2019
  47. Martin ÅgrenJun 13, 2019
  48. Junio C HamanoJun 13, 2019
  49. Rohit AshiwalJun 14, 2019
  50. Rohit AshiwalJun 14, 2019
  51. [GSoC][PATCH v4 0/4] [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 16, 2019
  52. [GSoC][PATCH v4 1/4] sequencer: add advice for revertRohit Ashiwal, Jun 16, 2019
  53. Thomas GummererJun 17, 2019
  54. [GSoC][PATCH v4 2/4] sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal, Jun 16, 2019
  55. [GSoC][PATCH v4 3/4] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 16, 2019
  56. Thomas GummererJun 17, 2019
  57. [GSoC][PATCH v4 4/4] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 16, 2019
  58. Thomas GummererJun 17, 2019
  59. [GSoC][PATCH v5 0/5] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 18, 2019
  60. [GSoC][PATCH v5 1/5] sequencer: add advice for revertRohit Ashiwal, Jun 18, 2019
  61. [GSoC][PATCH v5 2/5] sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal, Jun 18, 2019
  62. [GSoC][PATCH v5 3/5] sequencer: use argv_array in reset_mergeRohit Ashiwal, Jun 18, 2019
  63. [GSoC][PATCH v5 4/5] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 18, 2019
  64. Junio C HamanoJun 20, 2019
  65. Rohit AshiwalJun 20, 2019
  66. Phillip WoodJun 20, 2019
  67. Junio C HamanoJun 20, 2019
  68. Phillip WoodJun 20, 2019
  69. Rohit AshiwalJun 20, 2019
  70. Phillip WoodJun 20, 2019
  71. Rohit AshiwalJun 21, 2019
  72. [GSoC][PATCH v5 5/5] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 18, 2019
  73. [GSoC][PATCH v6 0/5] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 21, 2019
  74. [GSoC][PATCH v6 1/5] sequencer: add advice for revertRohit Ashiwal, Jun 21, 2019
  75. [GSoC][PATCH v6 2/5] sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal, Jun 21, 2019
  76. [GSoC][PATCH v6 3/5] sequencer: use argv_array in reset_mergeRohit Ashiwal, Jun 21, 2019
  77. [GSoC][PATCH v6 4/5] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 21, 2019
  78. [GSoC][PATCH v6 5/5] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 21, 2019
  79. Junio C HamanoJun 21, 2019
  80. [GSoC][PATCH v7 0/6] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jun 23, 2019
  81. [GSoC][PATCH v7 1/6] advice: add sequencerInUse config variableRohit Ashiwal, Jun 23, 2019
  82. Thomas GummererJun 25, 2019
  83. [GSoC][PATCH v7 4/6] sequencer: use argv_array in reset_mergeRohit Ashiwal, Jun 23, 2019
  84. [GSoC][PATCH v7 2/6] sequencer: add advice for revertRohit Ashiwal, Jun 23, 2019
  85. Phillip WoodJun 29, 2019
  86. [GSoC][PATCH v7 3/6] sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal, Jun 23, 2019
  87. [GSoC][PATCH v7 5/6] cherry-pick/revert: add --skip optionRohit Ashiwal, Jun 23, 2019
  88. [GSoC][PATCH v7 6/6] cherry-pick/revert: advise using --skipRohit Ashiwal, Jun 23, 2019
  89. [GSoC][PATCH v8 0/5] Teach cherry-pick/revert to skip commitsRohit Ashiwal, Jul 2, 2019
  90. [GSoC][PATCH v8 1/5] sequencer: add advice for revertRohit Ashiwal, Jul 2, 2019
  91. [GSoC][PATCH v8 2/5] sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal, Jul 2, 2019
  92. [GSoC][PATCH v8 3/5] sequencer: use argv_array in reset_mergeRohit Ashiwal, Jul 2, 2019
  93. [GSoC][PATCH v8 4/5] cherry-pick/revert: add --skip optionRohit Ashiwal, Jul 2, 2019
  94. [GSoC][PATCH v8 5/5] cherry-pick/revert: advise using --skipRohit Ashiwal, Jul 2, 2019
  95. Phillip WoodJul 2, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.