Git development
 help / color / mirror / Atom feed
* Re: [RFC/PATCH] Makefile: add cppcheck target
From: Jeff King @ 2016-12-13 12:28 UTC (permalink / raw)
  To: Chris Packham; +Cc: git, gitter.spiros
In-Reply-To: <20161213121510.5o5axuwzztbxcvfd@sigill.intra.peff.net>

On Tue, Dec 13, 2016 at 07:15:10AM -0500, Jeff King wrote:

> I think these last two are a good sign that we need to be feeding the
> list of source files to cppcheck. I tried your patch and it also started
> looking in t/perf/build, which are old versions of git built to serve
> the performance-testing suite.
> 
> See the way that the "tags" target is handled for a possible approach.

Maybe something like this:

diff --git a/Makefile b/Makefile
index 8b5976d88..e7684ae63 100644
--- a/Makefile
+++ b/Makefile
@@ -2638,4 +2638,6 @@ cover_db_html: cover_db
 .PHONY: cppcheck
 
 cppcheck:
-	cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) .
+	$(FIND_SOURCE_FILES) |\
+	grep -v ^t/t |\
+	xargs cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD)

> My main complaint with any static checker is how we can handle false
> positives. [...]

Here's what that generates on my machine, with annotations:

[builtin/am.c:766]: (error) Resource leak: in

  Correct.

[builtin/notes.c:260]: (error) Memory pointed to by 'buf' is freed twice.
[builtin/notes.c:264]: (error) Memory pointed to by 'buf' is freed twice.

  Nope. It appears not to understand that die() does not return.

[builtin/rev-list.c:391]: (error) Uninitialized variable: reaches
[builtin/rev-list.c:391]: (error) Uninitialized variable: all

  These are "int foo = foo" (which is ugly, and maybe we can get rid of
  if compilers have gotten smart enough to figure it out).

[compat/nedmalloc/malloc.c.h:4646]: (error) Memory leak: mem

  Hard to tell, but I think this is wrong and is confused by pointer
  arithmetic on the malloc chunks.

[compat/regex/regcomp.c:3086]: (error) Memory leak: sbcset

  Nope, this return is hit only when sbcset is NULL. The tool is
  presumably confused by a conditional hidden inside a macro.

[compat/regex/regcomp.c:3634]: (error) Memory leak: sbcset
[compat/regex/regcomp.c:3086]: (error) Memory leak: mbcset
[compat/regex/regcomp.c:3634]: (error) Memory leak: mbcset

  I didn't look at these, but presumably similar.

[compat/regex/regcomp.c:2802]: (error) Uninitialized variable: table_size
[compat/regex/regcomp.c:2805]: (error) Uninitialized variable: table_size

  Not sure, but it looks like this function declares another inline
  function inside its scope, and that confuses the tool.

[compat/regex/regcomp.c:532]: (error) Memory leak: fastmap

  Nope. Tool seems confused by hiding free() in a macro.

[contrib/examples/builtin-fetch--tool.c:420]: (error) Uninitialized variable: lrr_count
[contrib/examples/builtin-fetch--tool.c:427]: (error) Uninitialized variable: lrr_list

  More "int foo = foo". Might be worth omitting contrib/examples (or
  possibly contrib/ entirely) from the check.

[t/helper/test-hashmap.c:125]: (error) Memory leak: entries
[t/helper/test-hashmap.c:125]: (error) Memory leak: hashes

  Correct (but unimportant).

So I think it is capable of finding real problems, but I think we'd need
some way of squelching false positives, preferably in a way that carries
forward as the code changes (so not just saying "foo.c:1234 is a false
positive", which will break when it becomes "foo.c:1235").

-Peff

^ permalink raw reply related

* Re: [PATCH 1/4] doc: add articles (grammar)
From: Kristoffer Haugsbakk @ 2016-12-13 13:05 UTC (permalink / raw)
  To: Philip Oakley; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <88A192B34B3D4DDDA47628687AE458D3@PhilipOakley>


Thank you for reviewing this series, Philip.

Philip Oakley writes:

> This looks good to me.

I'll add this header:

    Acked-by: Philip Oakley <philipoakley@iee.org>

To the commit message of this patch in the next review round (version 2
of the patch series).

Let me know if I should add another header, or do something different
than this.

-- 
Kristoffer Haugsbakk

^ permalink raw reply

* Re: [PATCH 3/4] doc: make the intent of sentence clearer
From: Kristoffer Haugsbakk @ 2016-12-13 13:23 UTC (permalink / raw)
  To: Philip Oakley; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <34DFA15DE05248069167FE613B66CD70@PhilipOakley>


Philip Oakley writes:

> Looks like a reasonable emphasis to me

I'll add this header:

    Acked-by: Philip Oakley <philipoakley@iee.org>

To the commit message of this patch in the next review round (version 2
of the patch series).

Let me know if I should add another header, or do something different
than this.

-- 
Kristoffer Haugsbakk

^ permalink raw reply

* [PATCH] builtin/commit.c: convert trivial snprintf calls to xsnprintf
From: Elia Pinto @ 2016-12-13 13:27 UTC (permalink / raw)
  To: git; +Cc: Elia Pinto

With the commits f2f02675 and 5096d490 we have been converted in some files the call from snprintf/sprintf/strcpy to xsnprintf.  This patch supersedes the
snprintf with several calls that make use of the heap rather than fixed
length buffers (eg. PATH_MAX) that may be incorrect on some systems.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/commit.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 0ed634b26..37228330c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -960,15 +960,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		return 0;
 
 	if (use_editor) {
-		char index[PATH_MAX];
-		const char *env[2] = { NULL };
-		env[0] =  index;
-		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-		if (launch_editor(git_path_commit_editmsg(), NULL, env)) {
+		struct argv_array env = ARGV_ARRAY_INIT;
+
+		argv_array_pushf(&env, "GIT_INDEX_FILE=%s", index_file);
+		if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) {
 			fprintf(stderr,
 			_("Please supply the message using either -m or -F option.\n"));
+			argv_array_clear(&env);
 			exit(1);
 		}
+		argv_array_clear(&env);
 	}
 
 	if (!no_verify &&
@@ -1525,12 +1526,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 static int run_rewrite_hook(const unsigned char *oldsha1,
 			    const unsigned char *newsha1)
 {
-	/* oldsha1 SP newsha1 LF NUL */
-	static char buf[2*40 + 3];
+	char *buf;
 	struct child_process proc = CHILD_PROCESS_INIT;
 	const char *argv[3];
 	int code;
-	size_t n;
 
 	argv[0] = find_hook("post-rewrite");
 	if (!argv[0])
@@ -1546,34 +1545,33 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 	code = start_command(&proc);
 	if (code)
 		return code;
-	n = snprintf(buf, sizeof(buf), "%s %s\n",
-		     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+	buf = xstrfmt("%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
 	sigchain_push(SIGPIPE, SIG_IGN);
-	write_in_full(proc.in, buf, n);
+	write_in_full(proc.in, buf, strlen(buf));
 	close(proc.in);
+	free(buf);
 	sigchain_pop(SIGPIPE);
 	return finish_command(&proc);
 }
 
 int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
 {
-	const char *hook_env[3] =  { NULL };
-	char index[PATH_MAX];
+	struct argv_array hook_env = ARGV_ARRAY_INIT;
 	va_list args;
 	int ret;
 
-	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-	hook_env[0] = index;
+	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
 
 	/*
 	 * Let the hook know that no editor will be launched.
 	 */
 	if (!editor_is_used)
-		hook_env[1] = "GIT_EDITOR=:";
+		argv_array_push(&hook_env, "GIT_EDITOR=:");
 
 	va_start(args, name);
-	ret = run_hook_ve(hook_env, name, args);
+	ret = run_hook_ve(hook_env.argv,name, args);
 	va_end(args);
+	argv_array_clear(&hook_env);
 
 	return ret;
 }
-- 
2.11.0.153.gacc9cba


^ permalink raw reply related

* Re: [PATCH] builtin/commit.c: convert trivial snprintf calls to xsnprintf
From: Jeff King @ 2016-12-13 13:55 UTC (permalink / raw)
  To: Elia Pinto; +Cc: git
In-Reply-To: <20161213132717.42965-1-gitter.spiros@gmail.com>

On Tue, Dec 13, 2016 at 01:27:17PM +0000, Elia Pinto wrote:

> With the commits f2f02675 and 5096d490 we have been converted in some files the call from snprintf/sprintf/strcpy to xsnprintf.  This patch supersedes the
> snprintf with several calls that make use of the heap rather than fixed
> length buffers (eg. PATH_MAX) that may be incorrect on some systems.

I had trouble parsing this, because it leads with mention of commits
that _aren't_ doing the same thing we are doing here. I think the
argument you want to make is basically:

  1. snprintf is bad because it may silently truncate results if we're
     wrong.

  2. In cases where we use PATH_MAX, we'd want to handle larger paths
     anyway, so we should switch to dynamic allocation.

  3. In other cases where we know the input is bounded to a certain
     length we could use xsnprintf(), which asserts that we don't
     truncate. But by switching to dynamic allocation, we can avoid
     dealing with magic numbers in the code.

I'd actually consider splitting cases (2) and (3) into separate patches.
Even though the end result is the same, the reasoning is quite
different.

As far as the patch itself goes, they all look like improvements to me.
The extra allocations shouldn't matter because these are all heavy
operations already.

A few minor nits:

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 0ed634b26..37228330c 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -960,15 +960,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		return 0;
>  
>  	if (use_editor) {
> -		char index[PATH_MAX];
> -		const char *env[2] = { NULL };
> -		env[0] =  index;
> -		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> -		if (launch_editor(git_path_commit_editmsg(), NULL, env)) {
> +		struct argv_array env = ARGV_ARRAY_INIT;
> +
> +		argv_array_pushf(&env, "GIT_INDEX_FILE=%s", index_file);
> +		if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) {
>  			fprintf(stderr,
>  			_("Please supply the message using either -m or -F option.\n"));
> +			argv_array_clear(&env);
>  			exit(1);
>  		}
> +		argv_array_clear(&env);

I'd generally skip the clear() right before exiting, though I saw
somebody disagree with me recently on that.  I wonder if we should
decide as a project on whether it is worth doing or not.

> @@ -1525,12 +1526,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
>  static int run_rewrite_hook(const unsigned char *oldsha1,
>  			    const unsigned char *newsha1)
>  {
> -	/* oldsha1 SP newsha1 LF NUL */
> -	static char buf[2*40 + 3];
> +	char *buf;
>  	struct child_process proc = CHILD_PROCESS_INIT;
>  	const char *argv[3];
>  	int code;
> -	size_t n;
>  
>  	argv[0] = find_hook("post-rewrite");
>  	if (!argv[0])
> @@ -1546,34 +1545,33 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
>  	code = start_command(&proc);
>  	if (code)
>  		return code;
> -	n = snprintf(buf, sizeof(buf), "%s %s\n",
> -		     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
> +	buf = xstrfmt("%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
>  	sigchain_push(SIGPIPE, SIG_IGN);
> -	write_in_full(proc.in, buf, n);
> +	write_in_full(proc.in, buf, strlen(buf));
>  	close(proc.in);
> +	free(buf);

Any time you care about the length of the result, I'd generally use an
actual strbuf instead of xstrfmt. The extra strlen isn't a big deal
here, but it somehow seems simpler to me. It probably doesn't matter
much either way, though.

>  int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
>  {
> -	const char *hook_env[3] =  { NULL };
> -	char index[PATH_MAX];
> +	struct argv_array hook_env = ARGV_ARRAY_INIT;
>  	va_list args;
>  	int ret;
>  
> -	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> -	hook_env[0] = index;
> +	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
>  
>  	/*
>  	 * Let the hook know that no editor will be launched.
>  	 */
>  	if (!editor_is_used)
> -		hook_env[1] = "GIT_EDITOR=:";
> +		argv_array_push(&hook_env, "GIT_EDITOR=:");
>  
>  	va_start(args, name);
> -	ret = run_hook_ve(hook_env, name, args);
> +	ret = run_hook_ve(hook_env.argv,name, args);
>  	va_end(args);
> +	argv_array_clear(&hook_env);

Missing whitespace between arguments in the run_hook_ve() call.

-Peff

^ permalink raw reply

* [PATCH v2 04/34] sequencer (rebase -i): implement the 'exec' command
From: Johannes Schindelin @ 2016-12-13 15:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

The 'exec' command is a little special among rebase -i's commands, as it
does *not* have a SHA-1 as first parameter. Instead, everything after the
`exec` command is treated as command-line to execute.

Let's reuse the arg/arg_len fields of the todo_item structure (which hold
the oneline for pick/edit commands) to point to the command-line.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 68e2c84803..700b7575ed 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -17,6 +17,7 @@
 #include "argv-array.h"
 #include "quote.h"
 #include "log-tree.h"
+#include "wt-status.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -664,6 +665,8 @@ enum todo_command {
 	TODO_PICK = 0,
 	TODO_REVERT,
 	TODO_EDIT,
+	/* commands that do something else than handling a single commit */
+	TODO_EXEC,
 	/* commands that do nothing but are counted for reporting progress */
 	TODO_NOOP
 };
@@ -672,6 +675,7 @@ static const char *todo_command_strings[] = {
 	"pick",
 	"revert",
 	"edit",
+	"exec",
 	"noop"
 };
 
@@ -971,6 +975,12 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 		return -1;
 	bol += padding;
 
+	if (item->command == TODO_EXEC) {
+		item->arg = bol;
+		item->arg_len = (int)(eol - bol);
+		return 0;
+	}
+
 	end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
 	saved = *end_of_object_name;
 	*end_of_object_name = '\0';
@@ -1396,6 +1406,47 @@ static int error_with_patch(struct commit *commit,
 	return exit_code;
 }
 
+static int do_exec(const char *command_line)
+{
+	const char *child_argv[] = { NULL, NULL };
+	int dirty, status;
+
+	fprintf(stderr, "Executing: %s\n", command_line);
+	child_argv[0] = command_line;
+	status = run_command_v_opt(child_argv, RUN_USING_SHELL);
+
+	/* force re-reading of the cache */
+	if (discard_cache() < 0 || read_cache() < 0)
+		return error(_("could not read index"));
+
+	dirty = require_clean_work_tree("rebase", NULL, 1, 1);
+
+	if (status) {
+		warning(_("execution failed: %s\n%s"
+			  "You can fix the problem, and then run\n"
+			  "\n"
+			  "  git rebase --continue\n"
+			  "\n"),
+			command_line,
+			dirty ? N_("and made changes to the index and/or the "
+				"working tree\n") : "");
+		if (status == 127)
+			/* command not found */
+			status = 1;
+	}
+	else if (dirty) {
+		warning(_("execution succeeded: %s\nbut "
+			  "left changes to the index and/or the working tree\n"
+			  "Commit or stash your changes, and then run\n"
+			  "\n"
+			  "  git rebase --continue\n"
+			  "\n"), command_line);
+		status = 1;
+	}
+
+	return status;
+}
+
 static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 {
 	int res = 0;
@@ -1425,6 +1476,14 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 					!res);
 			}
 		}
+		else if (item->command == TODO_EXEC) {
+			char *end_of_arg = (char *)(item->arg + item->arg_len);
+			int saved = *end_of_arg;
+
+			*end_of_arg = '\0';
+			res = do_exec(item->arg);
+			*end_of_arg = saved;
+		}
 		else if (item->command != TODO_NOOP)
 			return error(_("unknown command %d"), item->command);
 
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 03/34] sequencer (rebase -i): implement the 'edit' command
From: Johannes Schindelin @ 2016-12-13 15:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

This patch is a straight-forward reimplementation of the `edit`
operation of the interactive rebase command.

Well, not *quite* straight-forward: when stopping, the `edit`
command wants to write the `patch` file (which is not only the
patch, but includes the commit message and author information). To
that end, this patch requires the earlier work that taught the
log-tree machinery to respect the `file` setting of
rev_info->diffopt to write to a file stream different than stdout.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1224799286..68e2c84803 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -16,6 +16,7 @@
 #include "refs.h"
 #include "argv-array.h"
 #include "quote.h"
+#include "log-tree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -43,6 +44,20 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
  */
 static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
 /*
+ * When an "edit" rebase command is being processed, the SHA1 of the
+ * commit to be edited is recorded in this file.  When "git rebase
+ * --continue" is executed, if there are any staged changes then they
+ * will be amended to the HEAD commit, but only provided the HEAD
+ * commit is still the commit to be edited.  When any other rebase
+ * command is processed, this file is deleted.
+ */
+static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
+/*
+ * When we stop at a given patch via the "edit" command, this file contains
+ * the long commit name of the corresponding patch.
+ */
+static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
+/*
  * The following files are written by git-rebase just after parsing the
  * command-line (and are only consumed, not modified, by the sequencer).
  */
@@ -648,6 +663,7 @@ enum todo_command {
 	/* commands that handle commits */
 	TODO_PICK = 0,
 	TODO_REVERT,
+	TODO_EDIT,
 	/* commands that do nothing but are counted for reporting progress */
 	TODO_NOOP
 };
@@ -655,6 +671,7 @@ enum todo_command {
 static const char *todo_command_strings[] = {
 	"pick",
 	"revert",
+	"edit",
 	"noop"
 };
 
@@ -1301,9 +1318,87 @@ static int save_opts(struct replay_opts *opts)
 	return res;
 }
 
+static int make_patch(struct commit *commit, struct replay_opts *opts)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct rev_info log_tree_opt;
+	const char *commit_buffer = get_commit_buffer(commit, NULL), *subject, *p;
+	int res = 0;
+
+	p = short_commit_name(commit);
+	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
+		return -1;
+
+	strbuf_addf(&buf, "%s/patch", get_dir(opts));
+	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
+	init_revisions(&log_tree_opt, NULL);
+	log_tree_opt.abbrev = 0;
+	log_tree_opt.diff = 1;
+	log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
+	log_tree_opt.disable_stdin = 1;
+	log_tree_opt.no_commit_id = 1;
+	log_tree_opt.diffopt.file = fopen(buf.buf, "w");
+	log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
+	if (!log_tree_opt.diffopt.file)
+		res |= error_errno(_("could not open '%s'"), buf.buf);
+	else {
+		res |= log_tree_commit(&log_tree_opt, commit);
+		fclose(log_tree_opt.diffopt.file);
+	}
+	strbuf_reset(&buf);
+
+	strbuf_addf(&buf, "%s/message", get_dir(opts));
+	if (!file_exists(buf.buf)) {
+		find_commit_subject(commit_buffer, &subject);
+		res |= write_message(subject, strlen(subject), buf.buf, 1);
+		unuse_commit_buffer(commit, commit_buffer);
+	}
+	strbuf_release(&buf);
+
+	return res;
+}
+
+static int intend_to_amend(void)
+{
+	unsigned char head[20];
+	char *p;
+
+	if (get_sha1("HEAD", head))
+		return error(_("cannot read HEAD"));
+
+	p = sha1_to_hex(head);
+	return write_message(p, strlen(p), rebase_path_amend(), 1);
+}
+
+static int error_with_patch(struct commit *commit,
+	const char *subject, int subject_len,
+	struct replay_opts *opts, int exit_code, int to_amend)
+{
+	if (make_patch(commit, opts))
+		return -1;
+
+	if (to_amend) {
+		if (intend_to_amend())
+			return -1;
+
+		fprintf(stderr, "You can amend the commit now, with\n"
+			"\n"
+			"  git commit --amend %s\n"
+			"\n"
+			"Once you are satisfied with your changes, run\n"
+			"\n"
+			"  git rebase --continue\n", gpg_sign_opt_quoted(opts));
+	}
+	else if (exit_code)
+		fprintf(stderr, "Could not apply %s... %.*s\n",
+			short_commit_name(commit), subject_len, subject);
+
+	return exit_code;
+}
+
 static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 {
-	int res;
+	int res = 0;
 
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
 	if (opts->allow_ff)
@@ -1316,9 +1411,20 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 		struct todo_item *item = todo_list->items + todo_list->current;
 		if (save_todo(todo_list, opts))
 			return -1;
-		if (item->command <= TODO_REVERT)
+		if (item->command <= TODO_EDIT) {
 			res = do_pick_commit(item->command, item->commit,
 					opts);
+			if (item->command == TODO_EDIT) {
+				struct commit *commit = item->commit;
+				if (!res)
+					warning(_("stopped at %s... %.*s"),
+						short_commit_name(commit),
+						item->arg_len, item->arg);
+				return error_with_patch(commit,
+					item->arg, item->arg_len, opts, res,
+					!res);
+			}
+		}
 		else if (item->command != TODO_NOOP)
 			return error(_("unknown command %d"), item->command);
 
@@ -1327,6 +1433,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			return res;
 	}
 
+	if (is_rebase_i(opts)) {
+		/* Stopped in the middle, as planned? */
+		if (todo_list->current < todo_list->nr)
+			return 0;
+	}
+
 	/*
 	 * Sequence of picks finished successfully; cleanup by
 	 * removing the .git/sequencer directory
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 12/34] sequencer (rebase -i): skip some revert/cherry-pick specific code path
From: Johannes Schindelin @ 2016-12-13 15:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When a cherry-pick continues without a "todo script", the intention is
simply to pick a single commit.

However, when an interactive rebase is continued without a "todo
script", it means that the last command has been completed and that we
now need to clean up.

This commit guards the revert/cherry-pick specific steps so that they
are not executed in rebase -i mode.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index abffaf3b40..4ceb6f3ac5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1882,25 +1882,28 @@ int sequencer_continue(struct replay_opts *opts)
 		if (commit_staged_changes(opts))
 			return -1;
 	}
-	if (!file_exists(get_todo_path(opts)))
+	else if (!file_exists(get_todo_path(opts)))
 		return continue_single_pick();
 	if (read_populate_opts(opts))
 		return -1;
 	if ((res = read_populate_todo(&todo_list, opts)))
 		goto release_todo_list;
 
-	/* Verify that the conflict has been resolved */
-	if (file_exists(git_path_cherry_pick_head()) ||
-	    file_exists(git_path_revert_head())) {
-		res = continue_single_pick();
-		if (res)
+	if (!is_rebase_i(opts)) {
+		/* Verify that the conflict has been resolved */
+		if (file_exists(git_path_cherry_pick_head()) ||
+		    file_exists(git_path_revert_head())) {
+			res = continue_single_pick();
+			if (res)
+				goto release_todo_list;
+		}
+		if (index_differs_from("HEAD", 0, 0)) {
+			res = error_dirty_index(opts);
 			goto release_todo_list;
+		}
+		todo_list.current++;
 	}
-	if (index_differs_from("HEAD", 0, 0)) {
-		res = error_dirty_index(opts);
-		goto release_todo_list;
-	}
-	todo_list.current++;
+
 	res = pick_commits(&todo_list, opts);
 release_todo_list:
 	todo_list_release(&todo_list);
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 14/34] sequencer (rebase -i): update refs after a successful rebase
From: Johannes Schindelin @ 2016-12-13 15:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

An interactive rebase operates on a detached HEAD (to keep the reflog
of the original branch relatively clean), and updates the branch only
at the end.

Now that the sequencer learns to perform interactive rebases, it also
needs to learn the trick to update the branch before removing the
directory containing the state of the interactive rebase.

We introduce a new head_ref variable in a wider scope than necessary at
the moment, to allow for a later patch that prints out "Successfully
rebased and updated <ref>".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index a6625e765d..a4e9b326ba 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -100,6 +100,8 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
 static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
 static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
 static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
+static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
+static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
 
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
@@ -1793,12 +1795,39 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 	}
 
 	if (is_rebase_i(opts)) {
-		struct strbuf buf = STRBUF_INIT;
+		struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
 
 		/* Stopped in the middle, as planned? */
 		if (todo_list->current < todo_list->nr)
 			return 0;
 
+		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
+				starts_with(head_ref.buf, "refs/")) {
+			unsigned char head[20], orig[20];
+
+			if (get_sha1("HEAD", head))
+				return error(_("cannot read HEAD"));
+			if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
+					get_sha1_hex(buf.buf, orig))
+				return error(_("could not read orig-head"));
+			strbuf_addf(&buf, "rebase -i (finish): %s onto ",
+				head_ref.buf);
+			if (!read_oneliner(&buf, rebase_path_onto(), 0))
+				return error(_("could not read 'onto'"));
+			if (update_ref(buf.buf, head_ref.buf, head, orig,
+					REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
+				return error(_("could not update %s"),
+					head_ref.buf);
+			strbuf_reset(&buf);
+			strbuf_addf(&buf,
+				"rebase -i (finish): returning to %s",
+				head_ref.buf);
+			if (create_symref("HEAD", head_ref.buf, buf.buf))
+				return error(_("could not update HEAD to %s"),
+					head_ref.buf);
+			strbuf_reset(&buf);
+		}
+
 		if (opts->verbose) {
 			const char *argv[] = {
 				"diff-tree", "--stat", NULL, NULL
@@ -1813,6 +1842,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			strbuf_reset(&buf);
 		}
 		strbuf_release(&buf);
+		strbuf_release(&head_ref);
 	}
 
 	/*
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 27/34] sequencer (rebase -i): differentiate between comments and 'noop'
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

In the upcoming patch, we will support rebase -i's progress
reporting. The progress skips comments but counts 'noop's.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1f314b2743..63f6f25ced 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -770,7 +770,9 @@ enum todo_command {
 	TODO_EXEC,
 	/* commands that do nothing but are counted for reporting progress */
 	TODO_NOOP,
-	TODO_DROP
+	TODO_DROP,
+	/* comments (not counted for reporting progress) */
+	TODO_COMMENT
 };
 
 static struct {
@@ -785,12 +787,13 @@ static struct {
 	{ 's', "squash" },
 	{ 'x', "exec" },
 	{ 0,   "noop" },
-	{ 'd', "drop" }
+	{ 'd', "drop" },
+	{ 0,   NULL }
 };
 
 static const char *command_to_string(const enum todo_command command)
 {
-	if ((size_t)command < ARRAY_SIZE(todo_command_info))
+	if (command < TODO_COMMENT)
 		return todo_command_info[command].str;
 	die("Unknown command: %d", command);
 }
@@ -1237,14 +1240,14 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 	bol += strspn(bol, " \t");
 
 	if (bol == eol || *bol == '\r' || *bol == comment_line_char) {
-		item->command = TODO_NOOP;
+		item->command = TODO_COMMENT;
 		item->commit = NULL;
 		item->arg = bol;
 		item->arg_len = eol - bol;
 		return 0;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(todo_command_info); i++)
+	for (i = 0; i < TODO_COMMENT; i++)
 		if (skip_prefix(bol, todo_command_info[i].str, &bol)) {
 			item->command = i;
 			break;
@@ -1254,7 +1257,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 			item->command = i;
 			break;
 		}
-	if (i >= ARRAY_SIZE(todo_command_info))
+	if (i >= TODO_COMMENT)
 		return -1;
 
 	if (item->command == TODO_NOOP) {
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 06/34] sequencer (rebase -i): write the 'done' file
From: Johannes Schindelin @ 2016-12-13 15:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

In the interactive rebase, commands that were successfully processed are
not simply discarded, but appended to the 'done' file instead. This is
used e.g. to display the current state to the user in the output of
`git status` or the progress.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 1ab50884bd..f6e20b142a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -39,6 +39,12 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
  */
 static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
+ * The rebase command lines that have already been processed. A line
+ * is moved here when it is first handled, before any associated user
+ * actions.
+ */
+static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
+/*
  * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
  * GIT_AUTHOR_DATE that will be used for the commit that is currently
  * being rebased.
@@ -1295,6 +1301,23 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
 		return error_errno(_("could not write to '%s'"), todo_path);
 	if (commit_lock_file(&todo_lock) < 0)
 		return error(_("failed to finalize '%s'."), todo_path);
+
+	if (is_rebase_i(opts)) {
+		const char *done_path = rebase_path_done();
+		int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
+		int prev_offset = !next ? 0 :
+			todo_list->items[next - 1].offset_in_buf;
+
+		if (fd >= 0 && offset > prev_offset &&
+		    write_in_full(fd, todo_list->buf.buf + prev_offset,
+				  offset - prev_offset) < 0) {
+			close(fd);
+			return error_errno(_("could not write to '%s'"),
+					   done_path);
+		}
+		if (fd >= 0)
+			close(fd);
+	}
 	return 0;
 }
 
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 05/34] sequencer (rebase -i): learn about the 'verbose' mode
From: Johannes Schindelin @ 2016-12-13 15:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When calling `git rebase -i -v`, the user wants to see some statistics
after the commits were rebased. Let's show some.

The strbuf we use to perform that task will be used for other things
in subsequent commits, hence it is declared and initialized in a wider
scope than strictly needed here.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 22 ++++++++++++++++++++++
 sequencer.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 700b7575ed..1ab50884bd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -63,6 +63,8 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
  * command-line (and are only consumed, not modified, by the sequencer).
  */
 static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
+static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
+static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
 
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
@@ -1121,6 +1123,9 @@ static int read_populate_opts(struct replay_opts *opts)
 		}
 		strbuf_release(&buf);
 
+		if (file_exists(rebase_path_verbose()))
+			opts->verbose = 1;
+
 		return 0;
 	}
 
@@ -1493,9 +1498,26 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 	}
 
 	if (is_rebase_i(opts)) {
+		struct strbuf buf = STRBUF_INIT;
+
 		/* Stopped in the middle, as planned? */
 		if (todo_list->current < todo_list->nr)
 			return 0;
+
+		if (opts->verbose) {
+			const char *argv[] = {
+				"diff-tree", "--stat", NULL, NULL
+			};
+
+			if (!read_oneliner(&buf, rebase_path_orig_head(), 0))
+				return error(_("could not read '%s'"),
+					rebase_path_orig_head());
+			strbuf_addstr(&buf, "..HEAD");
+			argv[2] = buf.buf;
+			run_command_v_opt(argv, RUN_GIT_CMD);
+			strbuf_reset(&buf);
+		}
+		strbuf_release(&buf);
 	}
 
 	/*
diff --git a/sequencer.h b/sequencer.h
index cb21cfddee..f885b68395 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -24,6 +24,7 @@ struct replay_opts {
 	int allow_empty;
 	int allow_empty_message;
 	int keep_redundant_commits;
+	int verbose;
 
 	int mainline;
 
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-13 15:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

This is a huge patch, and at the same time a huge step forward to
execute the performance-critical parts of the interactive rebase in a
builtin command.

Since 'fixup' and 'squash' are not only similar, but also need to know
about each other (we want to reduce a series of fixups/squashes into a
single, final commit message edit, from the user's point of view), we
really have to implement them both at the same time.

Most of the actual work is done by the existing code path that already
handles the "pick" and the "edit" commands; We added support for other
features (e.g. to amend the commit message) in the patches leading up to
this one, yet there are still quite a few bits in this patch that simply
would not make sense as individual patches (such as: determining whether
there was anything to "fix up" in the "todo" script, etc).

In theory, it would be possible to reuse the fast-forward code path also
for the fixup and the squash code paths, but in practice this would make
the code less readable. The end result cannot be fast-forwarded anyway,
therefore let's just extend the cherry-picking code path for now.

Since the sequencer parses the entire `git-rebase-todo` script in one go,
fixup or squash commands without a preceding pick can be reported early
(in git-rebase--interactive, we could only report such errors just before
executing the fixup/squash).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 222 insertions(+), 10 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index f6e20b142a..271c21581d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -45,6 +45,35 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
  */
 static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
 /*
+ * The commit message that is planned to be used for any changes that
+ * need to be committed following a user interaction.
+ */
+static GIT_PATH_FUNC(rebase_path_message, "rebase-merge/message")
+/*
+ * The file into which is accumulated the suggested commit message for
+ * squash/fixup commands. When the first of a series of squash/fixups
+ * is seen, the file is created and the commit message from the
+ * previous commit and from the first squash/fixup commit are written
+ * to it. The commit message for each subsequent squash/fixup commit
+ * is appended to the file as it is processed.
+ *
+ * The first line of the file is of the form
+ *     # This is a combination of $count commits.
+ * where $count is the number of commits whose messages have been
+ * written to the file so far (including the initial "pick" commit).
+ * Each time that a commit message is processed, this line is read and
+ * updated. It is deleted just before the combined commit is made.
+ */
+static GIT_PATH_FUNC(rebase_path_squash_msg, "rebase-merge/message-squash")
+/*
+ * If the current series of squash/fixups has not yet included a squash
+ * command, then this file exists and holds the commit message of the
+ * original "pick" commit.  (If the series ends without a "squash"
+ * command, then this can be used as the commit message of the combined
+ * commit without opening the editor.)
+ */
+static GIT_PATH_FUNC(rebase_path_fixup_msg, "rebase-merge/message-fixup")
+/*
  * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
  * GIT_AUTHOR_DATE that will be used for the commit that is currently
  * being rebased.
@@ -673,6 +702,8 @@ enum todo_command {
 	TODO_PICK = 0,
 	TODO_REVERT,
 	TODO_EDIT,
+	TODO_FIXUP,
+	TODO_SQUASH,
 	/* commands that do something else than handling a single commit */
 	TODO_EXEC,
 	/* commands that do nothing but are counted for reporting progress */
@@ -683,6 +714,8 @@ static const char *todo_command_strings[] = {
 	"pick",
 	"revert",
 	"edit",
+	"fixup",
+	"squash",
 	"exec",
 	"noop"
 };
@@ -694,16 +727,119 @@ static const char *command_to_string(const enum todo_command command)
 	die("Unknown command: %d", command);
 }
 
+static int is_fixup(enum todo_command command)
+{
+	return command == TODO_FIXUP || command == TODO_SQUASH;
+}
+
+static int update_squash_messages(enum todo_command command,
+		struct commit *commit, struct replay_opts *opts)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int count, res;
+	const char *message, *body;
+
+	if (file_exists(rebase_path_squash_msg())) {
+		char *p, *p2;
+
+		if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
+			return error(_("could not read '%s'"),
+				rebase_path_squash_msg());
+
+		if (buf.buf[0] != comment_line_char ||
+		    !skip_prefix(buf.buf + 1, " This is a combination of ",
+				 (const char **)&p))
+			return error(_("unexpected 1st line of squash message:"
+				       "\n\n\t%.*s"),
+				     (int)(strchrnul(buf.buf, '\n') - buf.buf),
+				     buf.buf);
+		count = strtol(p, &p2, 10);
+
+		if (count < 1 || *p2 != ' ')
+			return error(_("invalid 1st line of squash message:\n"
+				       "\n\t%.*s"),
+				     (int)(strchrnul(buf.buf, '\n') - buf.buf),
+				     buf.buf);
+
+		sprintf((char *)p, "%d", ++count);
+		if (!*p2)
+			*p2 = ' ';
+		else {
+			*(++p2) = 'c';
+			strbuf_insert(&buf, p2 - buf.buf, " ", 1);
+		}
+	}
+	else {
+		unsigned char head[20];
+		struct commit *head_commit;
+		const char *head_message, *body;
+
+		if (get_sha1("HEAD", head))
+			return error(_("need a HEAD to fixup"));
+		if (!(head_commit = lookup_commit_reference(head)))
+			return error(_("could not read HEAD"));
+		if (!(head_message = get_commit_buffer(head_commit, NULL)))
+			return error(_("could not read HEAD's commit message"));
+
+		body = strstr(head_message, "\n\n");
+		if (!body)
+			body = "";
+		else
+			body = skip_blank_lines(body + 2);
+		if (write_message(body, strlen(body),
+				  rebase_path_fixup_msg(), 0))
+			return error(_("cannot write '%s'"),
+				     rebase_path_fixup_msg());
+
+		count = 2;
+		strbuf_addf(&buf, _("%c This is a combination of 2 commits.\n"
+				    "%c The first commit's message is:\n\n%s"),
+			    comment_line_char, comment_line_char, body);
+
+		unuse_commit_buffer(head_commit, head_message);
+	}
+
+	if (!(message = get_commit_buffer(commit, NULL)))
+		return error(_("could not read commit message of %s"),
+			     oid_to_hex(&commit->object.oid));
+	body = strstr(message, "\n\n");
+	if (!body)
+		body = "";
+	else
+		body = skip_blank_lines(body + 2);
+
+	if (command == TODO_SQUASH) {
+		unlink(rebase_path_fixup_msg());
+		strbuf_addf(&buf, _("\n%c This is the commit message #%d:\n"
+				    "\n%s"),
+			    comment_line_char, count, body);
+	}
+	else if (command == TODO_FIXUP) {
+		strbuf_addf(&buf, _("\n%c The commit message #%d "
+				    "will be skipped:\n\n"),
+			    comment_line_char, count);
+		strbuf_add_commented_lines(&buf, body, strlen(body));
+	}
+	else
+		return error(_("unknown command: %d"), command);
+	unuse_commit_buffer(commit, message);
+
+	res = write_message(buf.buf, buf.len, rebase_path_squash_msg(), 0);
+	strbuf_release(&buf);
+	return res;
+}
 
 static int do_pick_commit(enum todo_command command, struct commit *commit,
-		struct replay_opts *opts)
+		struct replay_opts *opts, int final_fixup)
 {
+	int edit = opts->edit, cleanup_commit_message = 0;
+	const char *msg_file = edit ? NULL : git_path_merge_msg();
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
 	const char *base_label, *next_label;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0, allow;
+	int res, unborn = 0, amend = 0, allow;
 
 	if (opts->no_commit) {
 		/*
@@ -749,7 +885,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 	else
 		parent = commit->parents->item;
 
-	if (opts->allow_ff &&
+	if (opts->allow_ff && !is_fixup(command) &&
 	    ((parent && !hashcmp(parent->object.oid.hash, head)) ||
 	     (!parent && unborn)))
 		return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
@@ -813,6 +949,28 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		}
 	}
 
+	if (is_fixup(command)) {
+		if (update_squash_messages(command, commit, opts))
+			return -1;
+		amend = 1;
+		if (!final_fixup)
+			msg_file = rebase_path_squash_msg();
+		else if (file_exists(rebase_path_fixup_msg())) {
+			cleanup_commit_message = 1;
+			msg_file = rebase_path_fixup_msg();
+		}
+		else {
+			const char *dest = git_path("SQUASH_MSG");
+			unlink(dest);
+			if (copy_file(dest, rebase_path_squash_msg(), 0666))
+				return error(_("could not rename '%s' to '%s'"),
+					     rebase_path_squash_msg(), dest);
+			unlink(git_path("MERGE_MSG"));
+			msg_file = dest;
+			edit = 1;
+		}
+	}
+
 	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf, opts);
@@ -868,8 +1026,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		goto leave;
 	}
 	if (!opts->no_commit)
-		res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
-				     opts, allow, opts->edit, 0, 0);
+		res = run_git_commit(msg_file, opts, allow, edit, amend,
+				     cleanup_commit_message);
+
+	if (!res && final_fixup) {
+		unlink(rebase_path_fixup_msg());
+		unlink(rebase_path_squash_msg());
+	}
 
 leave:
 	free_message(commit, &msg);
@@ -1009,7 +1172,7 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
 {
 	struct todo_item *item;
 	char *p = buf, *next_p;
-	int i, res = 0;
+	int i, res = 0, fixup_okay = file_exists(rebase_path_done());
 
 	for (i = 1; *p; i++, p = next_p) {
 		char *eol = strchrnul(p, '\n');
@@ -1024,8 +1187,16 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
 		if (parse_insn_line(item, p, eol)) {
 			res = error(_("invalid line %d: %.*s"),
 				i, (int)(eol - p), p);
-			item->command = -1;
+			item->command = TODO_NOOP;
 		}
+
+		if (fixup_okay)
+			; /* do nothing */
+		else if (is_fixup(item->command))
+			return error(_("cannot '%s' without a previous commit"),
+				command_to_string(item->command));
+		else if (item->command != TODO_NOOP)
+			fixup_okay = 1;
 	}
 	if (!todo_list->nr)
 		return error(_("no commits parsed."));
@@ -1434,6 +1605,20 @@ static int error_with_patch(struct commit *commit,
 	return exit_code;
 }
 
+static int error_failed_squash(struct commit *commit,
+	struct replay_opts *opts, int subject_len, const char *subject)
+{
+	if (rename(rebase_path_squash_msg(), rebase_path_message()))
+		return error(_("could not rename '%s' to '%s'"),
+			rebase_path_squash_msg(), rebase_path_message());
+	unlink(rebase_path_fixup_msg());
+	unlink(git_path("MERGE_MSG"));
+	if (copy_file(git_path("MERGE_MSG"), rebase_path_message(), 0666))
+		return error(_("could not copy '%s' to '%s'"),
+			     rebase_path_message(), git_path("MERGE_MSG"));
+	return error_with_patch(commit, subject, subject_len, opts, 1, 0);
+}
+
 static int do_exec(const char *command_line)
 {
 	const char *child_argv[] = { NULL, NULL };
@@ -1475,6 +1660,21 @@ static int do_exec(const char *command_line)
 	return status;
 }
 
+static int is_final_fixup(struct todo_list *todo_list)
+{
+	int i = todo_list->current;
+
+	if (!is_fixup(todo_list->items[i].command))
+		return 0;
+
+	while (++i < todo_list->nr)
+		if (is_fixup(todo_list->items[i].command))
+			return 0;
+		else if (todo_list->items[i].command < TODO_NOOP)
+			break;
+	return 1;
+}
+
 static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 {
 	int res = 0;
@@ -1490,9 +1690,15 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 		struct todo_item *item = todo_list->items + todo_list->current;
 		if (save_todo(todo_list, opts))
 			return -1;
-		if (item->command <= TODO_EDIT) {
+		if (is_rebase_i(opts)) {
+			unlink(rebase_path_message());
+			unlink(rebase_path_author_script());
+			unlink(rebase_path_stopped_sha());
+			unlink(rebase_path_amend());
+		}
+		if (item->command <= TODO_SQUASH) {
 			res = do_pick_commit(item->command, item->commit,
-					opts);
+					opts, is_final_fixup(todo_list));
 			if (item->command == TODO_EDIT) {
 				struct commit *commit = item->commit;
 				if (!res)
@@ -1503,6 +1709,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 					item->arg, item->arg_len, opts, res,
 					!res);
 			}
+			if (res && is_fixup(item->command)) {
+				if (res == 1)
+					intend_to_amend();
+				return error_failed_squash(item->commit, opts,
+					item->arg_len, item->arg);
+			}
 		}
 		else if (item->command == TODO_EXEC) {
 			char *end_of_arg = (char *)(item->arg + item->arg_len);
@@ -1597,7 +1809,7 @@ static int single_pick(struct commit *cmit, struct replay_opts *opts)
 {
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
 	return do_pick_commit(opts->action == REPLAY_PICK ?
-		TODO_PICK : TODO_REVERT, cmit, opts);
+		TODO_PICK : TODO_REVERT, cmit, opts, 0);
 }
 
 int sequencer_pick_revisions(struct replay_opts *opts)
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 13/34] sequencer (rebase -i): the todo can be empty when continuing
From: Johannes Schindelin @ 2016-12-13 15:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When the last command of an interactive rebase fails, the user needs to
resolve the problem and then continue the interactive rebase. Naturally,
the todo script is empty by then. So let's not complain about that!

To that end, let's move that test out of the function that parses the
todo script, and into the more high-level function read_populate_todo().
This is also necessary by now because the lower-level parse_insn_buffer()
has no idea whether we are performing an interactive rebase or not.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 4ceb6f3ac5..a6625e765d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1255,8 +1255,7 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
 		else if (item->command != TODO_NOOP)
 			fixup_okay = 1;
 	}
-	if (!todo_list->nr)
-		return error(_("no commits parsed."));
+
 	return res;
 }
 
@@ -1280,6 +1279,10 @@ static int read_populate_todo(struct todo_list *todo_list,
 	if (res)
 		return error(_("unusable instruction sheet: '%s'"), todo_file);
 
+	if (!todo_list->nr &&
+	    (!is_rebase_i(opts) || !file_exists(rebase_path_done())))
+		return error(_("no commits parsed."));
+
 	if (!is_rebase_i(opts)) {
 		enum todo_command valid =
 			opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT;
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds
From: Johannes Schindelin @ 2016-12-13 15:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

This will be needed to hide the output of `git commit` when the
sequencer handles an interactive rebase's script.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 run-command.c | 23 +++++++++++++++++++++++
 run-command.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/run-command.c b/run-command.c
index ca905a9e80..5bb957afdd 100644
--- a/run-command.c
+++ b/run-command.c
@@ -589,6 +589,29 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
 	cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
 	cmd.dir = dir;
 	cmd.env = env;
+
+	if (opt & RUN_HIDE_STDERR_ON_SUCCESS) {
+		struct strbuf buf = STRBUF_INIT;
+		int res;
+
+		cmd.err = -1;
+		if (start_command(&cmd) < 0)
+			return -1;
+
+		if (strbuf_read(&buf, cmd.err, 0) < 0) {
+			close(cmd.err);
+			finish_command(&cmd); /* throw away exit code */
+			return -1;
+		}
+
+		close(cmd.err);
+		res = finish_command(&cmd);
+		if (res)
+			fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		return res;
+	}
+
 	return run_command(&cmd);
 }
 
diff --git a/run-command.h b/run-command.h
index dd1c78c28d..65a21ddd4e 100644
--- a/run-command.h
+++ b/run-command.h
@@ -72,6 +72,7 @@ extern int run_hook_ve(const char *const *env, const char *name, va_list args);
 #define RUN_SILENT_EXEC_FAILURE 8
 #define RUN_USING_SHELL 16
 #define RUN_CLEAN_ON_EXIT 32
+#define RUN_HIDE_STDERR_ON_SUCCESS 64
 int run_command_v_opt(const char **argv, int opt);
 
 /*
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 26/34] sequencer (rebase -i): implement the 'drop' command
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

The parsing part of a 'drop' command is almost identical to parsing a
'pick', while the operation is the same as that of a 'noop'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 03256b5b1d..1f314b2743 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -769,7 +769,8 @@ enum todo_command {
 	/* commands that do something else than handling a single commit */
 	TODO_EXEC,
 	/* commands that do nothing but are counted for reporting progress */
-	TODO_NOOP
+	TODO_NOOP,
+	TODO_DROP
 };
 
 static struct {
@@ -783,7 +784,8 @@ static struct {
 	{ 'f', "fixup" },
 	{ 's', "squash" },
 	{ 'x', "exec" },
-	{ 0,   "noop" }
+	{ 0,   "noop" },
+	{ 'd', "drop" }
 };
 
 static const char *command_to_string(const enum todo_command command)
@@ -1317,7 +1319,7 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
 		else if (is_fixup(item->command))
 			return error(_("cannot '%s' without a previous commit"),
 				command_to_string(item->command));
-		else if (item->command != TODO_NOOP)
+		else if (item->command < TODO_NOOP)
 			fixup_okay = 1;
 	}
 
@@ -1827,7 +1829,7 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)
 	int i;
 
 	for (i = todo_list->current + offset; i < todo_list->nr; i++)
-		if (todo_list->items[i].command != TODO_NOOP)
+		if (todo_list->items[i].command < TODO_NOOP)
 			return todo_list->items[i].command;
 
 	return -1;
@@ -1960,7 +1962,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			res = do_exec(item->arg);
 			*end_of_arg = saved;
 		}
-		else if (item->command != TODO_NOOP)
+		else if (item->command < TODO_NOOP)
 			return error(_("unknown command %d"), item->command);
 
 		todo_list->current++;
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 15/34] sequencer (rebase -i): leave a patch upon error
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When doing an interactive rebase, we want to leave a 'patch' file for
further inspection by the user (even if we never tried to actually apply
that patch, since we're cherry-picking instead).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index a4e9b326ba..4361fe0e94 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1777,6 +1777,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 				return error_failed_squash(item->commit, opts,
 					item->arg_len, item->arg);
 			}
+			else if (res && is_rebase_i(opts))
+				return res | error_with_patch(item->commit,
+					item->arg, item->arg_len, opts, res, 0);
 		}
 		else if (item->command == TODO_EXEC) {
 			char *end_of_arg = (char *)(item->arg + item->arg_len);
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 25/34] sequencer (rebase -i): allow rescheduling commands
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

The interactive rebase has the very special magic that a cherry-pick
that exits with a status different from 0 and 1 signifies a failure to
even record that a cherry-pick was started.

This can happen e.g. when a fast-forward fails because it would
overwrite untracked files.

In that case, we must reschedule the command that we thought we already
had at least started successfully.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index a67ecec961..03256b5b1d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1922,6 +1922,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 					1);
 			res = do_pick_commit(item->command, item->commit,
 					opts, is_final_fixup(todo_list));
+			if (is_rebase_i(opts) && res < 0) {
+				/* Reschedule */
+				todo_list->current--;
+				if (save_todo(todo_list, opts))
+					return -1;
+			}
 			if (item->command == TODO_EDIT) {
 				struct commit *commit = item->commit;
 				if (!res)
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 24/34] sequencer (rebase -i): respect strategy/strategy_opts settings
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

The sequencer already has an idea about using different merge
strategies. We just piggy-back on top of that, using rebase -i's
own settings, when running the sequencer in interactive rebase mode.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index c11eceb70b..a67ecec961 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -112,6 +112,8 @@ static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
 static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
 static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
 static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash")
+static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
+static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
 
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
@@ -1408,6 +1410,26 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
 	return 0;
 }
 
+static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
+{
+	int i;
+
+	strbuf_reset(buf);
+	if (!read_oneliner(buf, rebase_path_strategy(), 0))
+		return;
+	opts->strategy = strbuf_detach(buf, NULL);
+	if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))
+		return;
+
+	opts->xopts_nr = split_cmdline(buf->buf, (const char ***)&opts->xopts);
+	for (i = 0; i < opts->xopts_nr; i++) {
+		const char *arg = opts->xopts[i];
+
+		skip_prefix(arg, "--", &arg);
+		opts->xopts[i] = xstrdup(arg);
+	}
+}
+
 static int read_populate_opts(struct replay_opts *opts)
 {
 	if (is_rebase_i(opts)) {
@@ -1421,11 +1443,13 @@ static int read_populate_opts(struct replay_opts *opts)
 				opts->gpg_sign = xstrdup(buf.buf + 2);
 			}
 		}
-		strbuf_release(&buf);
 
 		if (file_exists(rebase_path_verbose()))
 			opts->verbose = 1;
 
+		read_strategy_opts(opts, &buf);
+		strbuf_release(&buf);
+
 		return 0;
 	}
 
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 20/34] sequencer (rebase -i): copy commit notes at end
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When rebasing commits that have commit notes attached, the interactive
rebase rewrites those notes faithfully at the end. The sequencer must
do this, too, if it wishes to do interactive rebase's job.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 118696f6d3..7ab533abd9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -94,6 +94,15 @@ static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
  */
 static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
 /*
+ * For the post-rewrite hook, we make a list of rewritten commits and
+ * their new sha1s.  The rewritten-pending list keeps the sha1s of
+ * commits that have been processed, but not committed yet,
+ * e.g. because they are waiting for a 'squash' command.
+ */
+static GIT_PATH_FUNC(rebase_path_rewritten_list, "rebase-merge/rewritten-list")
+static GIT_PATH_FUNC(rebase_path_rewritten_pending,
+	"rebase-merge/rewritten-pending")
+/*
  * The following files are written by git-rebase just after parsing the
  * command-line (and are only consumed, not modified, by the sequencer).
  */
@@ -883,6 +892,44 @@ static int update_squash_messages(enum todo_command command,
 	return res;
 }
 
+static void flush_rewritten_pending(void) {
+	struct strbuf buf = STRBUF_INIT;
+	unsigned char newsha1[20];
+	FILE *out;
+
+	if (strbuf_read_file(&buf, rebase_path_rewritten_pending(), 82) > 0 &&
+			!get_sha1("HEAD", newsha1) &&
+			(out = fopen(rebase_path_rewritten_list(), "a"))) {
+		char *bol = buf.buf, *eol;
+
+		while (*bol) {
+			eol = strchrnul(bol, '\n');
+			fprintf(out, "%.*s %s\n", (int)(eol - bol),
+					bol, sha1_to_hex(newsha1));
+			if (!*eol)
+				break;
+			bol = eol + 1;
+		}
+		fclose(out);
+		unlink(rebase_path_rewritten_pending());
+	}
+	strbuf_release(&buf);
+}
+
+static void record_in_rewritten(struct object_id *oid,
+		enum todo_command next_command) {
+	FILE *out = fopen(rebase_path_rewritten_pending(), "a");
+
+	if (!out)
+		return;
+
+	fprintf(out, "%s\n", oid_to_hex(oid));
+	fclose(out);
+
+	if (!is_fixup(next_command))
+		flush_rewritten_pending();
+}
+
 static int do_pick_commit(enum todo_command command, struct commit *commit,
 		struct replay_opts *opts, int final_fixup)
 {
@@ -1750,6 +1797,17 @@ static int is_final_fixup(struct todo_list *todo_list)
 	return 1;
 }
 
+static enum todo_command peek_command(struct todo_list *todo_list, int offset)
+{
+	int i;
+
+	for (i = todo_list->current + offset; i < todo_list->nr; i++)
+		if (todo_list->items[i].command != TODO_NOOP)
+			return todo_list->items[i].command;
+
+	return -1;
+}
+
 static const char *reflog_message(struct replay_opts *opts,
 	const char *sub_action, const char *fmt, ...)
 {
@@ -1808,6 +1866,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 					item->arg, item->arg_len, opts, res,
 					!res);
 			}
+			if (is_rebase_i(opts) && !res)
+				record_in_rewritten(&item->commit->object.oid,
+					peek_command(todo_list, 1));
 			if (res && is_fixup(item->command)) {
 				if (res == 1)
 					intend_to_amend();
@@ -1837,6 +1898,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 
 	if (is_rebase_i(opts)) {
 		struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
+		struct stat st;
 
 		/* Stopped in the middle, as planned? */
 		if (todo_list->current < todo_list->nr)
@@ -1881,6 +1943,20 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			run_command_v_opt(argv, RUN_GIT_CMD);
 			strbuf_reset(&buf);
 		}
+		flush_rewritten_pending();
+		if (!stat(rebase_path_rewritten_list(), &st) &&
+				st.st_size > 0) {
+			struct child_process child = CHILD_PROCESS_INIT;
+
+			child.in = open(rebase_path_rewritten_list(), O_RDONLY);
+			child.git_cmd = 1;
+			argv_array_push(&child.args, "notes");
+			argv_array_push(&child.args, "copy");
+			argv_array_push(&child.args, "--for-rewrite=rebase");
+			/* we don't care if this copying failed */
+			run_command(&child);
+		}
+
 		strbuf_release(&buf);
 		strbuf_release(&head_ref);
 	}
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 23/34] sequencer (rebase -i): respect the rebase.autostash setting
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

Git's `rebase` command inspects the `rebase.autostash` config setting
to determine whether it should stash any uncommitted changes before
rebasing and re-apply them afterwards.

As we introduce more bits and pieces to let the sequencer act as
interactive rebase's backend, here is the part that adds support for
the autostash feature.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index cafd945e83..c11eceb70b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -111,6 +111,7 @@ static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
 static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
 static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
 static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
+static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash")
 
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
@@ -1808,6 +1809,47 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)
 	return -1;
 }
 
+static int apply_autostash(struct replay_opts *opts)
+{
+	struct strbuf stash_sha1 = STRBUF_INIT;
+	struct child_process child = CHILD_PROCESS_INIT;
+	int ret = 0;
+
+	if (!read_oneliner(&stash_sha1, rebase_path_autostash(), 1)) {
+		strbuf_release(&stash_sha1);
+		return 0;
+	}
+	strbuf_trim(&stash_sha1);
+
+	child.git_cmd = 1;
+	argv_array_push(&child.args, "stash");
+	argv_array_push(&child.args, "apply");
+	argv_array_push(&child.args, stash_sha1.buf);
+	if (!run_command(&child))
+		printf(_("Applied autostash."));
+	else {
+		struct child_process store = CHILD_PROCESS_INIT;
+
+		store.git_cmd = 1;
+		argv_array_push(&store.args, "stash");
+		argv_array_push(&store.args, "store");
+		argv_array_push(&store.args, "-m");
+		argv_array_push(&store.args, "autostash");
+		argv_array_push(&store.args, "-q");
+		argv_array_push(&store.args, stash_sha1.buf);
+		if (run_command(&store))
+			ret = error(_("cannot store %s"), stash_sha1.buf);
+		else
+			printf(_("Applying autostash resulted in conflicts.\n"
+				"Your changes are safe in the stash.\n"
+				"You can run \"git stash pop\" or"
+				" \"git stash drop\" at any time.\n"));
+	}
+
+	strbuf_release(&stash_sha1);
+	return ret;
+}
+
 static const char *reflog_message(struct replay_opts *opts,
 	const char *sub_action, const char *fmt, ...)
 {
@@ -1970,6 +2012,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 				run_command(&hook);
 			}
 		}
+		apply_autostash(opts);
 
 		strbuf_release(&buf);
 		strbuf_release(&head_ref);
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 21/34] sequencer (rebase -i): record interrupted commits in rewritten, too
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

When continuing after a `pick` command failed, we want that commit
to show up in the rewritten-list (and its notes to be rewritten), too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 7ab533abd9..0233999389 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2052,6 +2052,15 @@ int sequencer_continue(struct replay_opts *opts)
 		}
 		todo_list.current++;
 	}
+	else if (file_exists(rebase_path_stopped_sha())) {
+		struct strbuf buf = STRBUF_INIT;
+		struct object_id oid;
+
+		if (read_oneliner(&buf, rebase_path_stopped_sha(), 1) &&
+		    !get_sha1_committish(buf.buf, oid.hash))
+			record_in_rewritten(&oid, peek_command(&todo_list, 0));
+		strbuf_release(&buf);
+	}
 
 	res = pick_commits(&todo_list, opts);
 release_todo_list:
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 18/34] sequencer (rebase -i): refactor setting the reflog message
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

This makes the code DRYer, with the obvious benefit that we can enhance
the code further in a single place.

We can also reuse the functionality elsewhere by calling this new
function.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 33fb36dcbe..d20efad562 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1750,6 +1750,26 @@ static int is_final_fixup(struct todo_list *todo_list)
 	return 1;
 }
 
+static const char *reflog_message(struct replay_opts *opts,
+	const char *sub_action, const char *fmt, ...)
+{
+	va_list ap;
+	static struct strbuf buf = STRBUF_INIT;
+
+	va_start(ap, fmt);
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, action_name(opts));
+	if (sub_action)
+		strbuf_addf(&buf, " (%s)", sub_action);
+	if (fmt) {
+		strbuf_addstr(&buf, ": ");
+		strbuf_vaddf(&buf, fmt, ap);
+	}
+	va_end(ap);
+
+	return buf.buf;
+}
+
 static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 {
 	int res = 0;
@@ -1820,6 +1840,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 
 		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
 				starts_with(head_ref.buf, "refs/")) {
+			const char *msg;
 			unsigned char head[20], orig[20];
 
 			if (get_sha1("HEAD", head))
@@ -1827,19 +1848,17 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
 					get_sha1_hex(buf.buf, orig))
 				return error(_("could not read orig-head"));
-			strbuf_addf(&buf, "rebase -i (finish): %s onto ",
-				head_ref.buf);
 			if (!read_oneliner(&buf, rebase_path_onto(), 0))
 				return error(_("could not read 'onto'"));
-			if (update_ref(buf.buf, head_ref.buf, head, orig,
+			msg = reflog_message(opts, "finish", "%s onto %s",
+				head_ref.buf, buf.buf);
+			if (update_ref(msg, head_ref.buf, head, orig,
 					REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
 				return error(_("could not update %s"),
 					head_ref.buf);
-			strbuf_reset(&buf);
-			strbuf_addf(&buf,
-				"rebase -i (finish): returning to %s",
+			msg = reflog_message(opts, "finish", "returning to %s",
 				head_ref.buf);
-			if (create_symref("HEAD", head_ref.buf, buf.buf))
+			if (create_symref("HEAD", head_ref.buf, msg))
 				return error(_("could not update HEAD to %s"),
 					head_ref.buf);
 			strbuf_reset(&buf);
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 22/34] sequencer (rebase -i): run the post-rewrite hook, if needed
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 0233999389..cafd945e83 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1947,6 +1947,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 		if (!stat(rebase_path_rewritten_list(), &st) &&
 				st.st_size > 0) {
 			struct child_process child = CHILD_PROCESS_INIT;
+			const char *post_rewrite_hook =
+				find_hook("post-rewrite");
 
 			child.in = open(rebase_path_rewritten_list(), O_RDONLY);
 			child.git_cmd = 1;
@@ -1955,6 +1957,18 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			argv_array_push(&child.args, "--for-rewrite=rebase");
 			/* we don't care if this copying failed */
 			run_command(&child);
+
+			if (post_rewrite_hook) {
+				struct child_process hook = CHILD_PROCESS_INIT;
+
+				hook.in = open(rebase_path_rewritten_list(),
+					O_RDONLY);
+				hook.stdout_to_stderr = 1;
+				argv_array_push(&hook.args, post_rewrite_hook);
+				argv_array_push(&hook.args, "rebase");
+				/* we don't care if this hook failed */
+				run_command(&hook);
+			}
 		}
 
 		strbuf_release(&buf);
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v2 19/34] sequencer (rebase -i): set the reflog message consistently
From: Johannes Schindelin @ 2016-12-13 15:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

We already used the same reflog message as the scripted version of rebase
-i when finishing. With this commit, we do that also for all the commands
before that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index d20efad562..118696f6d3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,6 +1792,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_amend());
 		}
 		if (item->command <= TODO_SQUASH) {
+			if (is_rebase_i(opts))
+				setenv("GIT_REFLOG_ACTION", reflog_message(opts,
+					command_to_string(item->command), NULL),
+					1);
 			res = do_pick_commit(item->command, item->commit,
 					opts, is_final_fixup(todo_list));
 			if (item->command == TODO_EDIT) {
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox