Merge branch 'tk/cherry-pick-sequence-requires-clean-worktree' into jch

"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.

* tk/cherry-pick-sequence-requires-clean-worktree:
  cherry-pick: refuse cherry-pick sequence if index is dirty

Author: gitster

sequencer.c

@@ -3199,38 +3199,48 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
 	return 0;
 }
 
-static int create_seq_dir(struct repository *r)
+static const char *cherry_pick_action_name(enum replay_action action) {
+	switch (action) {
+	case REPLAY_REVERT:
+		return "revert";
+		break;
+	case REPLAY_PICK:
+		return "cherry-pick";
+		break;
+	default:
+		BUG("unexpected action in cherry_pick_action_name");
+	}
+}
+
+static int create_seq_dir(struct repository *r, enum replay_action requested_action)
 {
-	enum replay_action action;
+	enum replay_action in_progress_action;
+	const char *in_progress_action_name = NULL;
 	const char *in_progress_error = NULL;
 	const char *in_progress_advice = NULL;
+	const char *requested_action_name = NULL;
 	unsigned int advise_skip =
 		refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD") ||
 		refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD");
 
-	if (!sequencer_get_last_command(r, &action)) {
-		switch (action) {
-		case REPLAY_REVERT:
-			in_progress_error = _("revert is already in progress");
-			in_progress_advice =
-			_("try \"git revert (--continue | %s--abort | --quit)\"");
-			break;
-		case REPLAY_PICK:
-			in_progress_error = _("cherry-pick is already in progress");
-			in_progress_advice =
-			_("try \"git cherry-pick (--continue | %s--abort | --quit)\"");
-			break;
-		default:
-			BUG("unexpected action in create_seq_dir");
-		}
+	if (!sequencer_get_last_command(r, &in_progress_action)) {
+		in_progress_action_name = cherry_pick_action_name(in_progress_action);
+		in_progress_error = _("%s is already in progress");
+		in_progress_advice =
+		_("try \"git %s (--continue | %s--abort | --quit)\"");
 	}
 	if (in_progress_error) {
-		error("%s", in_progress_error);
+		error(in_progress_error, in_progress_action_name);
 		if (advice_enabled(ADVICE_SEQUENCER_IN_USE))
 			advise(in_progress_advice,
+				in_progress_action_name,
 				advise_skip ? "--skip | " : "");
 		return -1;
 	}
+	requested_action_name = cherry_pick_action_name(requested_action);
+	if (require_clean_index(r, requested_action_name,
+				    _("Please commit or stash them."), 1, 1))
+		return -1;
 	if (mkdir(git_path_seq_dir(), 0777) < 0)
 		return error_errno(_("could not create sequencer directory '%s'"),
 				   git_path_seq_dir());
@@ -5279,12 +5289,11 @@ int sequencer_pick_revisions(struct repository *r,
 
 	/*
 	 * Start a new cherry-pick/ revert sequence; but
-	 * first, make sure that an existing one isn't in
-	 * progress
+	 * first, make sure that the index is clean and that
+	 * an existing one isn't in progress.
 	 */
-
 	if (walk_revs_populate_todo(&todo_list, opts) ||
-			create_seq_dir(r) < 0)
+			create_seq_dir(r, opts->action) < 0)
 		return -1;
 	if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT))
 		return error(_("can't revert as initial commit"));

t/t3510-cherry-pick-sequence.sh

@@ -47,6 +47,16 @@ test_expect_success 'cherry-pick persists data on failure' '
 	test_path_is_file .git/sequencer/opts
 '
 
+test_expect_success 'cherry-pick sequence refuses to run on dirty index' '
+	pristine_detach initial &&
+	touch localindexchange &&
+	git add localindexchange &&
+	echo picking &&
+	test_must_fail git cherry-pick initial..picked &&
+	test_path_is_missing .git/sequencer &&
+	test_must_fail git cherry-pick --abort
+'
+
 test_expect_success 'cherry-pick mid-cherry-pick-sequence' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick base..anotherpick &&

wt-status.c

@@ -2620,15 +2620,12 @@ int has_uncommitted_changes(struct repository *r,
 	return result;
 }
 
-/**
- * If the work tree has unstaged or uncommitted changes, dies with the
- * appropriate message.
- */
-int require_clean_work_tree(struct repository *r,
-			    const char *action,
-			    const char *hint,
-			    int ignore_submodules,
-			    int gently)
+static int require_clean_index_or_work_tree(struct repository *r,
+				     const char *action,
+				     const char *hint,
+				     int ignore_submodules,
+				     int check_index_only,
+				     int gently)
 {
 	struct lock_file lock_file = LOCK_INIT;
 	int err = 0, fd;
@@ -2639,10 +2636,12 @@ int require_clean_work_tree(struct repository *r,
 		repo_update_index_if_able(r, &lock_file);
 	rollback_lock_file(&lock_file);
 
-	if (has_unstaged_changes(r, ignore_submodules)) {
-		/* TRANSLATORS: the action is e.g. "pull with rebase" */
-		error(_("cannot %s: You have unstaged changes."), _(action));
-		err = 1;
+	if (!check_index_only) {
+		if (has_unstaged_changes(r, ignore_submodules)) {
+			/* TRANSLATORS: the action is e.g. "pull with rebase" */
+			error(_("cannot %s: You have unstaged changes."), _(action));
+			err = 1;
+		}
 	}
 
 	if (has_uncommitted_changes(r, ignore_submodules)) {
@@ -2667,3 +2666,39 @@ int require_clean_work_tree(struct repository *r,
 
 	return err;
 }
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+int require_clean_work_tree(struct repository *r,
+			    const char *action,
+			    const char *hint,
+			    int ignore_submodules,
+			    int gently)
+{
+	return require_clean_index_or_work_tree(r,
+						action,
+						hint,
+						ignore_submodules,
+						0,
+						gently);
+}
+
+/**
+ * If the work tree has uncommitted changes, dies with the appropriate
+ * message.
+ */
+int require_clean_index(struct repository *r,
+			const char *action,
+			const char *hint,
+			int ignore_submodules,
+			int gently)
+{
+	return require_clean_index_or_work_tree(r,
+						action,
+						hint,
+						ignore_submodules,
+						1,
+						gently);
+}

wt-status.h

@@ -181,5 +181,10 @@ int require_clean_work_tree(struct repository *repo,
 			    const char *hint,
 			    int ignore_submodules,
 			    int gently);
+int require_clean_index(struct repository *repo,
+			    const char *action,
+			    const char *hint,
+			    int ignore_submodules,
+			    int gently);
 
 #endif /* STATUS_H */