[GSoC] [PATCH 0/7] builtin rebase actions

[GSoC] [PATCH 0/7] builtin rebase actions

[Pratik Karki]

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/20180808134830.19949-1-predatoramigo@gmail.com/).

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(-)

-- 
2.18.0

[PATCH 1/7] builtin rebase: support --continue

[Pratik Karki]

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.

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 115 +++++++++++++++++++++++++++++++++++++++++++++--
 strbuf.c         |   9 ++++
 strbuf.h         |   3 ++
 3 files changed, 123 insertions(+), 4 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b2ddfa8dbf..10da4c978b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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:
diff --git a/strbuf.c b/strbuf.c
index 030556111d..fdc0ffbafb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -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;
diff --git a/strbuf.h b/strbuf.h
index 60a35aef16..b7aea8a966 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -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.
-- 
2.18.0

[PATCH 2/7] builtin rebase: support --skip

[Pratik Karki]

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

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 10da4c978b..7a903838b1 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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");
 	}
-- 
2.18.0

[PATCH 3/7] builtin rebase: support --abort

[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.

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 7a903838b1..8e94f6cc85 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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");
 	}
-- 
2.18.0

[PATCH 4/7] builtin rebase: support --quit

[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).

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 8e94f6cc85..0d805480a1 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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");
 	}
-- 
2.18.0

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

[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).

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0d805480a1..e3dd2f511e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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 */
-- 
2.18.0

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

[Pratik Karki]

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

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index e3dd2f511e..1344e071f3 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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."));
-- 
2.18.0

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

[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.

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
 builtin/rebase.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 1344e071f3..75c4ac66e0 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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();
-- 
2.18.0