From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Christian Couder <chriscool@tuxfamily.org>,
Daniel Barkalow <barkalow@iabervon.org>
Subject: [PATCH 16/17] revert: Introduce --reset to remove sequencer state
Date: Mon, 11 Jul 2011 14:54:07 +0000 [thread overview]
Message-ID: <1310396048-24925-17-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1310396048-24925-1-git-send-email-artagnon@gmail.com>
Protect the sequencer state from accidentally being stomped by a new
cherry-pick or revert invocation by ensuring that an existing one
isn't in progress. While this patch would normally be expected to
break many tests, the earlier patches "reset: Make hard reset remove
the sequencer state" and "revert: Remove sequencer state when no
commits are pending" make sure that they don't.
To explicitly remove the sequencer state for a fresh cherry-pick or
revert invocation, introduce a new subcommand called '--reset' which
really removes the sequencer state on the very first invocation.
Ensure that the "rebase -i" script which invokes cherry-pick or revert
doesn't change its behavior by using '--reset' to to clear the state
after every failed pick. Effectively, the script will remain unaware
and unaffected by introduction of further subcommands like
'--continue'.
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/git-cherry-pick.txt | 5 +++
Documentation/git-revert.txt | 5 +++
Documentation/sequencer.txt | 4 ++
builtin/reset.c | 2 +-
builtin/revert.c | 66 ++++++++++++++++++++++++++++++------
git-rebase--interactive.sh | 25 +++++++++++---
sequencer.c | 4 ++-
sequencer.h | 6 +++-
t/t3510-cherry-pick-sequence.sh | 17 ++++++++-
9 files changed, 113 insertions(+), 21 deletions(-)
create mode 100644 Documentation/sequencer.txt
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 9d8fe0d..138a292 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -8,6 +8,7 @@ git-cherry-pick - Apply the changes introduced by some existing commits
SYNOPSIS
--------
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
+'git cherry-pick' --reset
DESCRIPTION
-----------
@@ -109,6 +110,10 @@ effect to your index in a row.
Pass the merge strategy-specific option through to the
merge strategy. See linkgit:git-merge[1] for details.
+SEQUENCER SUBCOMMANDS
+---------------------
+include::sequencer.txt[]
+
EXAMPLES
--------
git cherry-pick master::
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index ac10cfb..783ffb4 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -8,6 +8,7 @@ git-revert - Revert some existing commits
SYNOPSIS
--------
'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
+'git revert' --reset
DESCRIPTION
-----------
@@ -90,6 +91,10 @@ effect to your index in a row.
Pass the merge strategy-specific option through to the
merge strategy. See linkgit:git-merge[1] for details.
+SEQUENCER SUBCOMMANDS
+---------------------
+include::sequencer.txt[]
+
EXAMPLES
--------
git revert HEAD~3::
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
new file mode 100644
index 0000000..16ce88c
--- /dev/null
+++ b/Documentation/sequencer.txt
@@ -0,0 +1,4 @@
+--reset::
+ Forget about the current operation in progress. Can be used
+ to clear the sequencer state after a failed cherry-pick or
+ revert.
diff --git a/builtin/reset.c b/builtin/reset.c
index 899e250..ec0e5d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -368,7 +368,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
switch (reset_type) {
case HARD:
- remove_sequencer_state();
+ remove_sequencer_state(0);
if (!update_ref_status && !quiet)
print_new_head_line(commit);
break;
diff --git a/builtin/revert.c b/builtin/revert.c
index 3936516..409d88f 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -29,19 +29,23 @@
static const char * const revert_usage[] = {
"git revert [options] <commit-ish>",
+ "git revert <subcommand>",
NULL
};
static const char * const cherry_pick_usage[] = {
"git cherry-pick [options] <commit-ish>",
+ "git cherry-pick <subcommand>",
NULL
};
static const char *me;
enum replay_action { REVERT, CHERRY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET };
struct replay_opts {
enum replay_action action;
+ enum replay_subcommand subcommand;
/* Boolean options */
int edit;
@@ -104,7 +108,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
int noop;
+ int reset = 0;
struct option options[] = {
+ OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
OPT_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
@@ -133,7 +139,27 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN);
- if (opts->commit_argc < 2)
+
+ /* Set the subcommand */
+ if (reset)
+ opts->subcommand = REPLAY_RESET;
+ else
+ opts->subcommand = REPLAY_NONE;
+
+ /* Check for incompatible command line arguments */
+ if (opts->subcommand == REPLAY_RESET) {
+ verify_opt_compatible(me, "--reset",
+ "--no-commit", opts->no_commit,
+ "--signoff", opts->signoff,
+ "--mainline", opts->mainline,
+ "--strategy", opts->strategy ? 1 : 0,
+ "--strategy-option", opts->xopts ? 1 : 0,
+ "-x", opts->record_origin,
+ "--ff", opts->allow_ff,
+ NULL);
+ }
+
+ else if (opts->commit_argc < 2)
usage_with_options(usage_str, options);
if (opts->allow_ff)
@@ -916,8 +942,11 @@ static void walk_revs_populate_todo(struct commit_list **todo_list,
static void create_seq_dir(void)
{
if (file_exists(git_path(SEQ_DIR))) {
- if (!is_directory(git_path(SEQ_DIR)) && remove_path(git_path(SEQ_DIR)) < 0)
- die(_("Could not remove %s"), git_path(SEQ_DIR));
+ error(_("%s already exists."), git_path(SEQ_DIR));
+ advise(_("This usually means that a %s operation is in progress."), me);
+ advise(_("Use %s --continue to continue the operation"), me);
+ advise(_("or use %s --reset to forget about it"), me);
+ die(_("%s failed"), me);
} else if (mkdir(git_path(SEQ_DIR), 0777) < 0)
die_errno(_("Could not create sequencer directory '%s'."), git_path(SEQ_DIR));
}
@@ -999,7 +1028,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* the user simply needs to resolve
* the conflict and commit
*/
- remove_sequencer_state();
+ remove_sequencer_state(0);
return res;
}
}
@@ -1008,7 +1037,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- remove_sequencer_state();
+ remove_sequencer_state(1);
return 0;
}
@@ -1019,13 +1048,28 @@ static int process_subcommand(struct replay_opts *opts)
read_and_refresh_cache(me, opts);
- walk_revs_populate_todo(&todo_list, opts);
- create_seq_dir();
- if (!get_sha1("HEAD", sha1))
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
- save_todo(todo_list, opts);
+ if (opts->subcommand == REPLAY_RESET) {
+ remove_sequencer_state(1);
+ return 0;
+ } else {
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+ if (file_exists(git_path(SEQ_TODO_FILE))) {
+ error(_("A %s is already in progress"), me);
+ advise(_("Use %s --reset to forget about it"), me);
+ return -1;
+ }
+ walk_revs_populate_todo(&todo_list, opts);
+ create_seq_dir();
+ if (!get_sha1("HEAD", sha1))
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
+ save_todo(todo_list, opts);
+ }
return pick_commits(todo_list, opts);
}
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 65690af..14c57dd 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -136,6 +136,10 @@ make_patch () {
get_author_ident_from_commit "$1" > "$author_script"
}
+clear_cherry_pick_state () {
+ git cherry-pick --reset
+}
+
die_with_patch () {
echo "$1" > "$state_dir"/stopped-sha
make_patch "$1"
@@ -279,8 +283,10 @@ pick_one_preserving_merges () {
echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
;;
*)
- output git cherry-pick "$@" ||
+ output git cherry-pick "$@" || {
+ clear_cherry_pick_state
die_with_patch $sha1 "Could not pick $sha1"
+ }
;;
esac
;;
@@ -385,16 +391,20 @@ do_next () {
comment_for_reflog pick
mark_action_done
- pick_one $sha1 ||
+ pick_one $sha1 || {
+ clear_cherry_pick_state
die_with_patch $sha1 "Could not apply $sha1... $rest"
+ }
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- pick_one $sha1 ||
+ pick_one $sha1 || {
+ clear_cherry_pick_state
die_with_patch $sha1 "Could not apply $sha1... $rest"
+ }
git commit --amend --no-post-rewrite
record_in_rewritten $sha1
;;
@@ -402,8 +412,10 @@ do_next () {
comment_for_reflog edit
mark_action_done
- pick_one $sha1 ||
+ pick_one $sha1 || {
+ clear_cherry_pick_state
die_with_patch $sha1 "Could not apply $sha1... $rest"
+ }
echo "$sha1" > "$state_dir"/stopped-sha
make_patch $sha1
git rev-parse --verify HEAD > "$amend"
@@ -438,7 +450,10 @@ do_next () {
echo "$author_script_content" > "$author_script"
eval "$author_script_content"
output git reset --soft HEAD^
- pick_one -n $sha1 || die_failed_squash $sha1 "$rest"
+ pick_one -n $sha1 || {
+ clear_cherry_pick_state
+ die_failed_squash $sha1 "$rest"
+ }
case "$(peek_next_command)" in
squash|s|fixup|f)
# This is an intermediate commit; its message will only be
diff --git a/sequencer.c b/sequencer.c
index 8c1de63..bc2c046 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3,7 +3,7 @@
#include "strbuf.h"
#include "dir.h"
-void remove_sequencer_state(void)
+void remove_sequencer_state(int aggressive)
{
struct strbuf seq_dir = STRBUF_INIT;
struct strbuf seq_old_dir = STRBUF_INIT;
@@ -12,6 +12,8 @@ void remove_sequencer_state(void)
strbuf_addf(&seq_old_dir, "%s", git_path(SEQ_OLD_DIR));
remove_dir_recursively(&seq_old_dir, 0);
rename(git_path(SEQ_DIR), git_path(SEQ_OLD_DIR));
+ if (aggressive)
+ remove_dir_recursively(&seq_old_dir, 0);
strbuf_release(&seq_dir);
strbuf_release(&seq_old_dir);
}
diff --git a/sequencer.h b/sequencer.h
index dcb51b1..e1951cc 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -9,7 +9,11 @@
/* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset --hard'.
+ *
+ * With the aggressive flag, it additionally removes SEQ_OLD_DIR,
+ * ignoring any errors. Inteded to be used by the sequencer's
+ * '--reset' subcommand.
*/
-void remove_sequencer_state(void);
+void remove_sequencer_state(int aggressive);
#endif
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 0a8b093..c21b345 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -37,7 +37,7 @@ test_expect_success 'cherry-pick persists data on failure' '
test_path_is_file .git/sequencer/head &&
test_path_is_file .git/sequencer/todo &&
test_path_is_file .git/sequencer/opts &&
- rm -rf .git/sequencer
+ git cherry-pick --reset
'
test_expect_success 'cherry-pick persists opts correctly' '
@@ -54,7 +54,7 @@ test_expect_success 'cherry-pick persists opts correctly' '
strategy-option = patience | ours
EOF
test_cmp expect .git/sequencer/opts &&
- rm -rf .git/sequencer
+ git cherry-pick --reset
'
test_expect_success 'cherry-pick cleans up sequencer state upon success' '
@@ -88,4 +88,17 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
test_cmp expect actual
'
+test_expect_success '--reset does not complain when no cherry-pick is in progress' '
+ pristine_detach initial &&
+ git cherry-pick --reset
+'
+
+test_expect_success '--reset cleans up sequencer state' '
+ pristine_detach initial &&
+ head=$(git rev-parse HEAD) &&
+ test_must_fail git cherry-pick base..picked &&
+ git cherry-pick --reset &&
+ test_path_is_missing .git/sequencer
+'
+
test_done
--
1.7.5.GIT
next prev parent reply other threads:[~2011-07-11 14:55 UTC|newest]
Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-11 14:53 [GSoC update] Sequencer for inclusion Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 01/17] advice: Introduce error_resolve_conflict Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 02/17] revert: Inline add_message_to_msg function Ramkumar Ramachandra
2011-07-12 16:53 ` Jonathan Nieder
2011-07-13 6:00 ` Ramkumar Ramachandra
2011-07-13 6:42 ` Jonathan Nieder
2011-07-19 16:36 ` Ramkumar Ramachandra
2011-07-19 16:52 ` Junio C Hamano
2011-07-19 17:08 ` Ramkumar Ramachandra
2011-07-19 19:36 ` Jonathan Nieder
2011-07-20 5:32 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 03/17] revert: Don't check lone argument in get_encoding Ramkumar Ramachandra
2011-07-12 16:59 ` Jonathan Nieder
2011-07-13 6:14 ` Ramkumar Ramachandra
2011-07-13 6:30 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 04/17] revert: Rename no_replay to record_origin Ramkumar Ramachandra
2011-07-12 17:02 ` Jonathan Nieder
2011-07-13 7:35 ` Ramkumar Ramachandra
2011-07-17 19:36 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 05/17] revert: Propogate errors upwards from do_pick_commit Ramkumar Ramachandra
2011-07-12 17:32 ` Jonathan Nieder
2011-07-17 10:46 ` Ramkumar Ramachandra
2011-07-17 16:59 ` Jonathan Nieder
2011-07-19 10:09 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 06/17] revert: Eliminate global "commit" variable Ramkumar Ramachandra
2011-07-12 17:45 ` Jonathan Nieder
2011-07-13 6:57 ` Ramkumar Ramachandra
2011-07-13 7:10 ` Jonathan Nieder
2011-07-13 8:33 ` Ramkumar Ramachandra
2011-07-13 16:40 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 07/17] revert: Introduce struct to keep command-line options Ramkumar Ramachandra
2011-07-12 18:05 ` Jonathan Nieder
2011-07-13 7:56 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 08/17] revert: Separate cmdline parsing from functional code Ramkumar Ramachandra
2011-07-12 18:20 ` Jonathan Nieder
2011-07-18 20:53 ` Ramkumar Ramachandra
2011-07-18 21:03 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 09/17] revert: Don't create invalid replay_opts in parse_args Ramkumar Ramachandra
2011-07-11 20:44 ` Junio C Hamano
2011-07-12 5:57 ` Ramkumar Ramachandra
2011-07-12 18:29 ` Jonathan Nieder
2011-07-17 11:56 ` Ramkumar Ramachandra
2011-07-17 18:43 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 10/17] sequencer: Announce sequencer state location Ramkumar Ramachandra
2011-07-12 18:56 ` Jonathan Nieder
2011-07-13 12:10 ` Sverre Rabbelier
2011-07-17 16:23 ` Ramkumar Ramachandra
2011-07-17 19:19 ` Jonathan Nieder
2011-07-18 19:44 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 11/17] revert: Save data for continuing after conflict resolution Ramkumar Ramachandra
2011-07-11 21:01 ` Junio C Hamano
2011-07-11 21:31 ` Junio C Hamano
2011-07-12 5:43 ` Ramkumar Ramachandra
2011-07-12 19:37 ` Jonathan Nieder
2011-07-17 11:48 ` Ramkumar Ramachandra
2011-07-17 18:40 ` Jonathan Nieder
2011-07-18 19:31 ` Ramkumar Ramachandra
2011-07-19 12:21 ` Jonathan Nieder
2011-07-19 12:34 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 12/17] revert: Save command-line options for continuing operation Ramkumar Ramachandra
2011-07-11 21:15 ` Junio C Hamano
2011-07-12 5:56 ` Ramkumar Ramachandra
2011-07-12 19:52 ` Jonathan Nieder
2011-07-18 20:18 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 13/17] revert: Introduce a layer of indirection over pick_commits Ramkumar Ramachandra
2011-07-12 20:03 ` Jonathan Nieder
2011-07-18 21:24 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 14/17] reset: Make hard reset remove the sequencer state Ramkumar Ramachandra
2011-07-12 20:15 ` Jonathan Nieder
2011-07-17 16:40 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 15/17] revert: Remove sequencer state when no commits are pending Ramkumar Ramachandra
2011-07-11 19:58 ` Junio C Hamano
2011-07-12 6:26 ` Ramkumar Ramachandra
2011-07-11 14:54 ` Ramkumar Ramachandra [this message]
2011-07-12 20:30 ` [PATCH 16/17] revert: Introduce --reset to remove sequencer state Jonathan Nieder
2011-07-17 17:10 ` Ramkumar Ramachandra
2011-07-17 19:25 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 17/17] revert: Introduce --continue to continue the operation Ramkumar Ramachandra
2011-07-12 20:46 ` Jonathan Nieder
2011-07-17 16:11 ` Ramkumar Ramachandra
2011-07-17 18:32 ` Jonathan Nieder
2011-07-18 20:00 ` Ramkumar Ramachandra
2011-07-18 20:09 ` Jonathan Nieder
2011-07-11 17:17 ` [GSoC update] Sequencer for inclusion Jonathan Nieder
2011-07-11 17:57 ` Ramkumar Ramachandra
2011-07-11 20:05 ` Ramkumar Ramachandra
2011-07-11 20:11 ` Jonathan Nieder
2011-07-12 7:05 ` Ramkumar Ramachandra
2011-07-11 20:07 ` Junio C Hamano
2011-07-11 22:14 ` Jeff King
2011-07-12 6:41 ` Ramkumar Ramachandra
2011-07-12 6:47 ` Jeff King
2011-07-13 9:41 ` Ramkumar Ramachandra
2011-07-13 19:07 ` Jeff King
2011-07-18 21:37 ` [RFC PATCH] config: Introduce functions to write non-standard file Ramkumar Ramachandra
2011-07-18 23:54 ` Junio C Hamano
2011-07-19 8:52 ` Ramkumar Ramachandra
2011-07-12 5:58 ` [GSoC update] Sequencer for inclusion Jonathan Nieder
2011-07-12 6:28 ` Miles Bader
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1310396048-24925-17-git-send-email-artagnon@gmail.com \
--to=artagnon@gmail.com \
--cc=barkalow@iabervon.org \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).