Skip to contentSkip to conversation

builtin rebase actions

PATCH New Pratik Karki 1 participant 8 messages Aug 8, 2018

lore
Share & tools
lore
#0 lore raw

cover builtin rebase actions

The previous patch series implemented essential options and neccessary configurations. This patch series teaches all the rebase actions like --continue, --skip, --abort, et al. to builtin rebase.

These actions are important to operation of git rebase hence, are kept in a patch series of their own.

It adds the rebase actions to the previous patch series,
(https://public-inbox.org/git/[email protected]/).

This is the third patch series that brings us more closer to a builtin "git rebase".

If you like to view the development branch, you can view
(https://github.com/git/git/pull/505), where I have kept my commits up to date and leveraged Travis(there is sporadic failures in t5520 for macos gcc and isn't due to my patches) for extra testing other than my system.

The next in line patch series which I'll be sending out today are:

builtin rebase options: The builtin rebase will add all the options supported by original rebase.
builtin rebase rest: The builtin rebase will convert all the remaining shell scripts from the original rebase to C.
default to builtin rebase: This will turn on the feature-complete builtin rebase to on.

These patch series are built on top of each other, i.e. they depend on this order.

Pratik Karki (7):
builtin rebase: support --continue
builtin rebase: support --skip
builtin rebase: support --abort
builtin rebase: support --quit
builtin rebase: support --edit-todo and --show-current-patch builtin rebase: actions require a rebase in progress builtin rebase: stop if git am is in progress

 builtin/rebase.c | 193 ++++++++++++++++++++++++++++++++++++++++++++++-
 strbuf.c         |   9 +++
 strbuf.h         |   3 +
 3 files changed, 201 insertions(+), 4 deletions(-)
Signature · 1 line
2.18.0
#1 lore raw

PATCH 1/7 builtin rebase: support --continue

This commit adds the option --continue which is used to resume rebase after merge conflicts. The code tries to stay as close to the equivalent shell scripts found in git-legacy-rebase.sh as possible.

When continuing a rebase, the state variables are read from state_dir. Some of the state variables are not actually stored there, such as upstream. The shell script version simply does not set them, but for consistency, we unset them in the builtin version.

---
 builtin/rebase.c | 115 +++++++++++++++++++++++++++++++++++++++++++++--
 strbuf.c         |   9 ++++
 strbuf.h         |   3 ++
 3 files changed, 123 insertions(+), 4 deletions(-)
builtin/rebase.c +111 -4
index b2ddfa8dbf..10da4c978b 100644
@@ -91,6 +91,7 @@ struct rebase_options {
 		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
 	} flags;
 	struct strbuf git_am_opt;
+	const char *action;
 };
 
 static int is_interactive(struct rebase_options *opts)
@@ -115,6 +116,62 @@ static const char *state_dir_path(const char *filename, struct rebase_options *o
 	return path.buf;
 }
 
+/* Read one file, then strip line endings */
+static int read_one(const char *path, struct strbuf *buf)
+{
+	if (strbuf_read_file(buf, path, 0) < 0)
+		return error_errno(_("could not read '%s'"), path);
+	strbuf_trim_trailing_newline(buf);
+	return 0;
+}
+
+/* Initialize the rebase options from the state directory. */
+static int read_basic_state(struct rebase_options *opts)
+{
+	struct strbuf head_name = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct object_id oid;
+
+	if (read_one(state_dir_path("head-name", opts), &head_name) ||
+	    read_one(state_dir_path("onto", opts), &buf))
+		return -1;
+	opts->head_name = starts_with(head_name.buf, "refs/") ?
+		xstrdup(head_name.buf) : NULL;
+	strbuf_release(&head_name);
+	if (get_oid(buf.buf, &oid))
+		return error(_("could not get 'onto': '%s'"), buf.buf);
+	opts->onto = lookup_commit_or_die(&oid, buf.buf);
+
+	/*
+	 * We always write to orig-head, but interactive rebase used to write to
+	 * head. Fall back to reading from head to cover for the case that the
+	 * user upgraded git with an ongoing interactive rebase.
+	 */
+	strbuf_reset(&buf);
+	if (file_exists(state_dir_path("orig-head", opts))) {
+		if (read_one(state_dir_path("orig-head", opts), &buf))
+			return -1;
+	} else if (read_one(state_dir_path("head", opts), &buf))
+		return -1;
+	if (get_oid(buf.buf, &opts->orig_head))
+		return error(_("invalid orig-head: '%s'"), buf.buf);
+
+	strbuf_reset(&buf);
+	if (read_one(state_dir_path("quiet", opts), &buf))
+		return -1;
+	if (buf.len)
+		opts->flags &= ~REBASE_NO_QUIET;
+	else
+		opts->flags |= REBASE_NO_QUIET;
+
+	if (file_exists(state_dir_path("verbose", opts)))
+		opts->flags |= REBASE_VERBOSE;
+
+	strbuf_release(&buf);
+
+	return 0;
+}
+
 static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
@@ -168,12 +225,13 @@ static int run_specific_rebase(struct rebase_options *opts)
 	add_var(&script_snippet, "state_dir", opts->state_dir);
 
 	add_var(&script_snippet, "upstream_name", opts->upstream_name);
-	add_var(&script_snippet, "upstream",
-				 oid_to_hex(&opts->upstream->object.oid));
+	add_var(&script_snippet, "upstream", opts->upstream ?
+		oid_to_hex(&opts->upstream->object.oid) : NULL);
 	add_var(&script_snippet, "head_name",
 		opts->head_name ? opts->head_name : "detached HEAD");
 	add_var(&script_snippet, "orig_head", oid_to_hex(&opts->orig_head));
-	add_var(&script_snippet, "onto", oid_to_hex(&opts->onto->object.oid));
+	add_var(&script_snippet, "onto", opts->onto ?
+		oid_to_hex(&opts->onto->object.oid) : NULL);
 	add_var(&script_snippet, "onto_name", opts->onto_name);
 	add_var(&script_snippet, "revisions", opts->revisions);
 	add_var(&script_snippet, "restrict_revision", opts->restrict_revision ?
@@ -189,6 +247,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 		opts->flags & REBASE_FORCE ? "t" : "");
 	if (opts->switch_to)
 		add_var(&script_snippet, "switch_to", opts->switch_to);
+	add_var(&script_snippet, "action", opts->action ? opts->action : "");
 
 	switch (opts->type) {
 	case REBASE_AM:
@@ -400,12 +459,16 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		.git_am_opt = STRBUF_INIT,
 	};
 	const char *branch_name;
-	int ret, flags, in_progress = 0;
+	int ret, flags, total_argc, in_progress = 0;
 	int ok_to_skip_pre_rebase = 0;
 	struct strbuf msg = STRBUF_INIT;
 	struct strbuf revisions = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	struct object_id merge_base;
+	enum {
+		NO_ACTION,
+		ACTION_CONTINUE,
+	} action = NO_ACTION;
 	struct option builtin_rebase_options[] = {
 		OPT_STRING(0, "onto", &options.onto_name,
 			   N_("revision"),
@@ -427,6 +490,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "no-ff", &options.flags,
 			N_("cherry-pick all commits, even if unchanged"),
 			REBASE_FORCE),
+		OPT_CMDMODE(0, "continue", &action, N_("continue"),
+			    ACTION_CONTINUE),
 		OPT_END(),
 	};
 
@@ -480,14 +545,55 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.type != REBASE_UNSPECIFIED)
 		in_progress = 1;
 
+	total_argc = argc;
 	argc = parse_options(argc, argv, prefix,
 			     builtin_rebase_options,
 			     builtin_rebase_usage, 0);
 
+	if (action != NO_ACTION && total_argc != 2) {
+		usage_with_options(builtin_rebase_usage,
+				   builtin_rebase_options);
+	}
+
 	if (argc > 2)
 		usage_with_options(builtin_rebase_usage,
 				   builtin_rebase_options);
 
+	switch (action) {
+	case ACTION_CONTINUE: {
+		struct object_id head;
+		struct lock_file lock_file = LOCK_INIT;
+		int fd;
+
+		options.action = "continue";
+
+		/* Sanity check */
+		if (get_oid("HEAD", &head))
+			die(_("Cannot read HEAD"));
+
+		fd = hold_locked_index(&lock_file, 0);
+		if (read_index(the_repository->index) < 0)
+			die(_("could not read index"));
+		refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL,
+			      NULL);
+		if (0 <= fd)
+			update_index_if_able(the_repository->index,
+					     &lock_file);
+		rollback_lock_file(&lock_file);
+
+		if (has_unstaged_changes(1)) {
+			puts(_("You must edit all merge conflicts and then\n"
+			       "mark them as resolved using git add"));
+			exit(1);
+		}
+		if (read_basic_state(&options))
+			exit(1);
+		goto run_rebase;
+	}
+	default:
+		die("TODO");
+	}
+
 	/* Make sure no rebase is in progress */
 	if (in_progress) {
 		const char *last_slash = strrchr(options.state_dir, '/');
@@ -719,6 +825,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	options.revisions = revisions.buf;
 
+run_rebase:
 	ret = !!run_specific_rebase(&options);
 
 cleanup:
strbuf.c +9 -0
index 030556111d..fdc0ffbafb 100644
@@ -120,6 +120,15 @@ void strbuf_trim_trailing_dir_sep(struct strbuf *sb)
 	sb->buf[sb->len] = '\0';
 }
 
+void strbuf_trim_trailing_newline(struct strbuf *sb)
+{
+	if (sb->len > 0 && sb->buf[sb->len - 1] == '\n') {
+		if (--sb->len > 0 && sb->buf[sb->len - 1] == '\r')
+			--sb->len;
+		sb->buf[sb->len] = '\0';
+	}
+}
+
 void strbuf_ltrim(struct strbuf *sb)
 {
 	char *b = sb->buf;
strbuf.h +3 -0
index 60a35aef16..b7aea8a966 100644
@@ -190,6 +190,9 @@ extern void strbuf_ltrim(struct strbuf *);
 /* Strip trailing directory separators */
 extern void strbuf_trim_trailing_dir_sep(struct strbuf *);
 
+/* Strip trailing LF or CR/LF */
+extern void strbuf_trim_trailing_newline(struct strbuf *sb);
+
 /**
  * Replace the contents of the strbuf with a reencoded form.  Returns -1
  * on error, 0 on success.
Signature · 1 line
2.18.0
#2 lore raw

PATCH 2/7 builtin rebase: support --skip

In reply to Pratik Karki

This commit adds the option --skip which is used to restart rebase after skipping the current patch.

---
 builtin/rebase.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
builtin/rebase.c +18 -0
index 10da4c978b..7a903838b1 100644
@@ -21,6 +21,7 @@
 #include "diff.h"
 #include "wt-status.h"
 #include "revision.h"
+#include "rerere.h"
 
 static char const * const builtin_rebase_usage[] = {
 	N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] "
@@ -468,6 +469,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	enum {
 		NO_ACTION,
 		ACTION_CONTINUE,
+		ACTION_SKIP,
 	} action = NO_ACTION;
 	struct option builtin_rebase_options[] = {
 		OPT_STRING(0, "onto", &options.onto_name,
@@ -492,6 +494,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			REBASE_FORCE),
 		OPT_CMDMODE(0, "continue", &action, N_("continue"),
 			    ACTION_CONTINUE),
+		OPT_CMDMODE(0, "skip", &action,
+			    N_("skip current patch and continue"), ACTION_SKIP),
 		OPT_END(),
 	};
 
@@ -590,6 +594,20 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			exit(1);
 		goto run_rebase;
 	}
+	case ACTION_SKIP: {
+		struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+		options.action = "skip";
+
+		rerere_clear(&merge_rr);
+		string_list_clear(&merge_rr, 1);
+
+		if (reset_head(NULL, "reset", NULL, 0) < 0)
+			die(_("could not discard worktree changes"));
+		if (read_basic_state(&options))
+			exit(1);
+		goto run_rebase;
+	}
 	default:
 		die("TODO");
 	}
Signature · 1 line
2.18.0
#3 lore raw

PATCH 3/7 builtin rebase: support --abort

In reply to Pratik Karki

This commit teaches the builtin rebase the "abort" action, which a user can call to roll back a rebase that is in progress.

---
 builtin/rebase.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
builtin/rebase.c +20 -0
index 7a903838b1..8e94f6cc85 100644
@@ -470,6 +470,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		NO_ACTION,
 		ACTION_CONTINUE,
 		ACTION_SKIP,
+		ACTION_ABORT,
 	} action = NO_ACTION;
 	struct option builtin_rebase_options[] = {
 		OPT_STRING(0, "onto", &options.onto_name,
@@ -496,6 +497,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			    ACTION_CONTINUE),
 		OPT_CMDMODE(0, "skip", &action,
 			    N_("skip current patch and continue"), ACTION_SKIP),
+		OPT_CMDMODE(0, "abort", &action,
+			    N_("abort and check out the original branch"),
+			    ACTION_ABORT),
 		OPT_END(),
 	};
 
@@ -608,6 +612,22 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			exit(1);
 		goto run_rebase;
 	}
+	case ACTION_ABORT: {
+		struct string_list merge_rr = STRING_LIST_INIT_DUP;
+		options.action = "abort";
+
+		rerere_clear(&merge_rr);
+		string_list_clear(&merge_rr, 1);
+
+		if (read_basic_state(&options))
+			exit(1);
+		if (reset_head(&options.orig_head, "reset",
+			       options.head_name, 0) < 0)
+			die(_("could not move back to %s"),
+			    oid_to_hex(&options.orig_head));
+		ret = finish_rebase(&options);
+		goto cleanup;
+	}
 	default:
 		die("TODO");
 	}
Signature · 1 line
2.18.0
#4 lore raw

PATCH 4/7 builtin rebase: support --quit

In reply to Pratik Karki

With this patch, the builtin rebase handles the --quit action which can be used to abort a rebase without rolling back any changes performed during the rebase (this is useful when a user forgot that they were in the middle of a rebase and continued working normally).

---
 builtin/rebase.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
builtin/rebase.c +11 -0
index 8e94f6cc85..0d805480a1 100644
@@ -471,6 +471,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		ACTION_CONTINUE,
 		ACTION_SKIP,
 		ACTION_ABORT,
+		ACTION_QUIT,
 	} action = NO_ACTION;
 	struct option builtin_rebase_options[] = {
 		OPT_STRING(0, "onto", &options.onto_name,
@@ -500,6 +501,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &action,
 			    N_("abort and check out the original branch"),
 			    ACTION_ABORT),
+		OPT_CMDMODE(0, "quit", &action,
+			    N_("abort but keep HEAD where it is"), ACTION_QUIT),
 		OPT_END(),
 	};
 
@@ -628,6 +631,14 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		ret = finish_rebase(&options);
 		goto cleanup;
 	}
+	case ACTION_QUIT: {
+		strbuf_reset(&buf);
+		strbuf_addstr(&buf, options.state_dir);
+		ret = !!remove_dir_recursively(&buf, 0);
+		if (ret)
+			die(_("could not remove '%s'"), options.state_dir);
+		goto cleanup;
+	}
 	default:
 		die("TODO");
 	}
Signature · 1 line
2.18.0
#5 lore raw

PATCH 5/7 builtin rebase: support --edit-todo and --show-current-patch

In reply to Pratik Karki

While these sub-commands are very different in spirit, their implementation is almost identical, so we convert them in one go.

And since those are the last sub-commands that needed to be converted, now we can also turn that default: case into a bug (because we should now handle all the actions).

---
 builtin/rebase.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
builtin/rebase.c +22 -1
index 0d805480a1..e3dd2f511e 100644
@@ -472,6 +472,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		ACTION_SKIP,
 		ACTION_ABORT,
 		ACTION_QUIT,
+		ACTION_EDIT_TODO,
+		ACTION_SHOW_CURRENT_PATCH,
 	} action = NO_ACTION;
 	struct option builtin_rebase_options[] = {
 		OPT_STRING(0, "onto", &options.onto_name,
@@ -503,6 +505,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			    ACTION_ABORT),
 		OPT_CMDMODE(0, "quit", &action,
 			    N_("abort but keep HEAD where it is"), ACTION_QUIT),
+		OPT_CMDMODE(0, "edit-todo", &action, N_("edit the todo list "
+			    "during an interactive rebase"), ACTION_EDIT_TODO),
+		OPT_CMDMODE(0, "show-current-patch", &action,
+			    N_("show the patch file being applied or merged"),
+			    ACTION_SHOW_CURRENT_PATCH),
 		OPT_END(),
 	};
 
@@ -570,6 +577,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_rebase_usage,
 				   builtin_rebase_options);
 
+	if (action == ACTION_EDIT_TODO && !is_interactive(&options))
+		die(_("The --edit-todo action can only be used during "
+		      "interactive rebase."));
+
 	switch (action) {
 	case ACTION_CONTINUE: {
 		struct object_id head;
@@ -639,8 +650,18 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			die(_("could not remove '%s'"), options.state_dir);
 		goto cleanup;
 	}
+	case ACTION_EDIT_TODO:
+		options.action = "edit-todo";
+		options.dont_finish_rebase = 1;
+		goto run_rebase;
+	case ACTION_SHOW_CURRENT_PATCH:
+		options.action = "show-current-patch";
+		options.dont_finish_rebase = 1;
+		goto run_rebase;
+	case NO_ACTION:
+		break;
 	default:
-		die("TODO");
+		BUG("action: %d", action);
 	}
 
 	/* Make sure no rebase is in progress */
Signature · 1 line
2.18.0
#6 lore raw

PATCH 6/7 builtin rebase: actions require a rebase in progress

In reply to Pratik Karki

This commit prevents actions (such as --continue, --skip) from running when there is no rebase in progress.

---
 builtin/rebase.c | 3 +++
 1 file changed, 3 insertions(+)
builtin/rebase.c +3 -0
index e3dd2f511e..1344e071f3 100644
@@ -577,6 +577,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_rebase_usage,
 				   builtin_rebase_options);
 
+	if (action != NO_ACTION && !in_progress)
+		die(_("No rebase in progress?"));
+
 	if (action == ACTION_EDIT_TODO && !is_interactive(&options))
 		die(_("The --edit-todo action can only be used during "
 		      "interactive rebase."));
Signature · 1 line
2.18.0
#7 lore raw

PATCH 7/7 builtin rebase: stop if `git am` is in progress

In reply to Pratik Karki

This commit checks for the file applying used by git am in rebase-apply/ and if the file is present it means git am is in progress so it errors out.

---
 builtin/rebase.c | 5 +++++
 1 file changed, 5 insertions(+)
builtin/rebase.c +5 -0
index 1344e071f3..75c4ac66e0 100644
@@ -539,6 +539,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	git_config(rebase_config, &options);
 
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "%s/applying", apply_dir());
+	if(file_exists(buf.buf))
+		die(_("It looks like 'git am' is in progress. Cannot rebase."));
+
 	if (is_directory(apply_dir())) {
 		options.type = REBASE_AM;
 		options.state_dir = apply_dir();
Signature · 1 line
2.18.0