Git development
 help / color / mirror / Atom feed
* [PATCH v3 08/38] sequencer (rebase -i): learn about the 'verbose' mode
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.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 | 28 ++++++++++++++++++++++++++++
 sequencer.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index e9c10d7fe5..ddc4d144d7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -65,6 +65,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)
 {
@@ -1088,6 +1090,9 @@ static int read_populate_opts(struct replay_opts *opts)
 		}
 		strbuf_release(&buf);
 
+		if (file_exists(rebase_path_verbose()))
+			opts->verbose = 1;
+
 		return 0;
 	}
 
@@ -1491,9 +1496,32 @@ 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) {
+			struct rev_info log_tree_opt;
+			struct object_id orig, head;
+
+			memset(&log_tree_opt, 0, sizeof(log_tree_opt));
+			init_revisions(&log_tree_opt, NULL);
+			log_tree_opt.diff = 1;
+			log_tree_opt.diffopt.output_format =
+				DIFF_FORMAT_DIFFSTAT;
+			log_tree_opt.disable_stdin = 1;
+
+			if (read_oneliner(&buf, rebase_path_orig_head(), 0) &&
+			    !get_sha1(buf.buf, orig.hash) &&
+			    !get_sha1("HEAD", head.hash)) {
+				diff_tree_sha1(orig.hash, head.hash,
+					       "", &log_tree_opt.diffopt);
+				log_tree_diff_flush(&log_tree_opt);
+			}
+		}
+		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 v3 07/38] sequencer (rebase -i): implement the 'exec' command
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.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 | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index b138a3906c..e9c10d7fe5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
 #include "quote.h"
 #include "trailer.h"
 #include "log-tree.h"
+#include "wt-status.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -632,6 +633,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
 };
@@ -640,6 +643,7 @@ static const char *todo_command_strings[] = {
 	"pick",
 	"revert",
 	"edit",
+	"exec",
 	"noop"
 };
 
@@ -938,6 +942,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';
@@ -1397,6 +1407,46 @@ 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 +1475,13 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 					item->arg, item->arg_len, opts, res,
 					!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 (!is_noop(item->command))
 			return error(_("unknown command %d"), item->command);
 
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 06/38] sequencer (rebase -i): implement the 'edit' command
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.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 | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 84f18e64e9..b138a3906c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -17,6 +17,7 @@
 #include "argv-array.h"
 #include "quote.h"
 #include "trailer.h"
+#include "log-tree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -45,6 +46,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 abbreviated 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).
  */
@@ -616,6 +631,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
 };
@@ -623,6 +639,7 @@ enum todo_command {
 static const char *todo_command_strings[] = {
 	"pick",
 	"revert",
+	"edit",
 	"noop"
 };
 
@@ -1302,9 +1319,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 *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)) {
+		const char *commit_buffer = get_commit_buffer(commit, NULL);
+		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)
@@ -1317,10 +1412,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);
-		else if (!is_noop(item->command))
+			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 (!is_noop(item->command))
 			return error(_("unknown command %d"), item->command);
 
 		todo_list->current++;
@@ -1328,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 v3 05/38] sequencer (rebase -i): implement the 'noop' command
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.git.johannes.schindelin@gmx.de>

The 'noop' command is probably the most boring of all rebase -i commands
to support in the sequencer.

Which makes it an excellent candidate for this first stab to add support
for rebase -i's commands to the sequencer.

For the moment, let's also treat empty lines and commented-out lines as
'noop'; We will refine that handling later in this patch series.

To make it easier to identify "classes" of todo_commands (such as:
determine whether a command is pick-like, i.e. handles a single commit),
let's enforce a certain order of said commands.

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

diff --git a/sequencer.c b/sequencer.c
index 690460bc67..84f18e64e9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -607,14 +607,23 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
 		return 1;
 }
 
+/*
+ * Note that ordering matters in this enum. Not only must it match the mapping
+ * below, it is also divided into several sections that matter.  When adding
+ * new commands, make sure you add it in the right section.
+ */
 enum todo_command {
+	/* commands that handle commits */
 	TODO_PICK = 0,
-	TODO_REVERT
+	TODO_REVERT,
+	/* commands that do nothing but are counted for reporting progress */
+	TODO_NOOP
 };
 
 static const char *todo_command_strings[] = {
 	"pick",
-	"revert"
+	"revert",
+	"noop"
 };
 
 static const char *command_to_string(const enum todo_command command)
@@ -624,6 +633,10 @@ static const char *command_to_string(const enum todo_command command)
 	die("Unknown command: %d", command);
 }
 
+static int is_noop(const enum todo_command command)
+{
+	return TODO_NOOP <= (size_t)command;
+}
 
 static int do_pick_commit(enum todo_command command, struct commit *commit,
 		struct replay_opts *opts)
@@ -879,6 +892,14 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 	/* left-trim */
 	bol += strspn(bol, " \t");
 
+	if (bol == eol || *bol == '\r' || *bol == comment_line_char) {
+		item->command = TODO_NOOP;
+		item->commit = NULL;
+		item->arg = bol;
+		item->arg_len = eol - bol;
+		return 0;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
 		if (skip_prefix(bol, todo_command_strings[i], &bol)) {
 			item->command = i;
@@ -887,6 +908,13 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
 	if (i >= ARRAY_SIZE(todo_command_strings))
 		return -1;
 
+	if (item->command == TODO_NOOP) {
+		item->commit = NULL;
+		item->arg = bol;
+		item->arg_len = eol - bol;
+		return 0;
+	}
+
 	/* Eat up extra spaces/ tabs before object name */
 	padding = strspn(bol, " \t");
 	if (!padding)
@@ -1289,7 +1317,12 @@ 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;
-		res = do_pick_commit(item->command, item->commit, opts);
+		if (item->command <= TODO_REVERT)
+			res = do_pick_commit(item->command, item->commit,
+					opts);
+		else if (!is_noop(item->command))
+			return error(_("unknown command %d"), item->command);
+
 		todo_list->current++;
 		if (res)
 			return res;
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 04/38] sequencer: support a new action: 'interactive rebase'
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.git.johannes.schindelin@gmx.de>

This patch introduces a new action for the sequencer. It really does not
do a whole lot of its own right now, but lays the ground work for
patches to come. The intention, of course, is to finally make the
sequencer the work horse of the interactive rebase (the original idea
behind the "sequencer" concept).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 36 ++++++++++++++++++++++++++++++++----
 sequencer.h |  3 ++-
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 720857beda..690460bc67 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -30,6 +30,14 @@ static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
 static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
 static GIT_PATH_FUNC(git_path_abort_safety_file, "sequencer/abort-safety")
 
+static GIT_PATH_FUNC(rebase_path, "rebase-merge")
+/*
+ * The file containing rebase commands, comments, and empty lines.
+ * This file is created by "git rebase -i" then edited by the user. As
+ * the lines are processed, they are removed from the front of this
+ * file and written to the tail of 'done'.
+ */
+static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
  * 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
@@ -42,19 +50,22 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
  */
 static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
 
-/* We will introduce the 'interactive rebase' mode later */
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
-	return 0;
+	return opts->action == REPLAY_INTERACTIVE_REBASE;
 }
 
 static const char *get_dir(const struct replay_opts *opts)
 {
+	if (is_rebase_i(opts))
+		return rebase_path();
 	return git_path_seq_dir();
 }
 
 static const char *get_todo_path(const struct replay_opts *opts)
 {
+	if (is_rebase_i(opts))
+		return rebase_path_todo();
 	return git_path_todo_file();
 }
 
@@ -122,7 +133,15 @@ int sequencer_remove_state(struct replay_opts *opts)
 
 static const char *action_name(const struct replay_opts *opts)
 {
-	return opts->action == REPLAY_REVERT ? N_("revert") : N_("cherry-pick");
+	switch (opts->action) {
+	case REPLAY_REVERT:
+		return N_("revert");
+	case REPLAY_PICK:
+		return N_("cherry-pick");
+	case REPLAY_INTERACTIVE_REBASE:
+		return N_("rebase -i");
+	}
+	die(_("Unknown action: %d"), opts->action);
 }
 
 struct commit_message {
@@ -364,7 +383,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
 
 	if (active_cache_changed &&
 	    write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
-		/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
+		/* TRANSLATORS: %s will be "revert", "cherry-pick" or
+		 * "rebase -i".
+		 */
 		return error(_("%s: Unable to write new index file"),
 			_(action_name(opts)));
 	rollback_lock_file(&index_lock);
@@ -1198,6 +1219,13 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
 	const char *todo_path = get_todo_path(opts);
 	int next = todo_list->current, offset, fd;
 
+	/*
+	 * rebase -i writes "git-rebase-todo" without the currently executing
+	 * command, appending it to "done" instead.
+	 */
+	if (is_rebase_i(opts))
+		next++;
+
 	fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
 	if (fd < 0)
 		return error_errno(_("could not lock '%s'"), todo_path);
diff --git a/sequencer.h b/sequencer.h
index 7a513c576b..cb21cfddee 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,7 +7,8 @@ const char *git_path_seq_dir(void);
 
 enum replay_action {
 	REPLAY_REVERT,
-	REPLAY_PICK
+	REPLAY_PICK,
+	REPLAY_INTERACTIVE_REBASE
 };
 
 struct replay_opts {
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 03/38] sequencer: use a helper to find the commit message
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.git.johannes.schindelin@gmx.de>

It is actually not safe to look for a commit message by looking for the
first empty line and skipping it.

The find_commit_subject() function looks more carefully, so let's use
it. Since we are interested in the entire commit message, we re-compute
the string length after verifying that the commit subject is not empty
(in which case the entire commit message would be empty, something that
should not happen but that we want to handle gracefully).

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

diff --git a/sequencer.c b/sequencer.c
index 3eededcb98..720857beda 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -703,14 +703,9 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 		next = commit;
 		next_label = msg.label;
 
-		/*
-		 * Append the commit log message to msgbuf; it starts
-		 * after the tree, parent, author, committer
-		 * information followed by "\n\n".
-		 */
-		p = strstr(msg.message, "\n\n");
-		if (p)
-			strbuf_addstr(&msgbuf, skip_blank_lines(p + 2));
+		/* Append the commit log message to msgbuf. */
+		if (find_commit_subject(msg.message, &p))
+			strbuf_addstr(&msgbuf, p);
 
 		if (opts->record_origin) {
 			if (!has_conforming_footer(&msgbuf, NULL, 0))
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 01/38] sequencer: avoid unnecessary curly braces
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.git.johannes.schindelin@gmx.de>

This was noticed while addressing Junio Hamano's concern that some
"else" operators were on separate lines than the preceding closing
brace.

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

diff --git a/sequencer.c b/sequencer.c
index 9adb7bbf1d..23793db08b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -632,9 +632,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 	}
 	discard_cache();
 
-	if (!commit->parents) {
+	if (!commit->parents)
 		parent = NULL;
-	}
 	else if (commit->parents->next) {
 		/* Reverting or cherry-picking a merge commit */
 		int cnt;
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 02/38] sequencer: move "else" keyword onto the same line as preceding brace
From: Johannes Schindelin @ 2017-01-02 15:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1483370556.git.johannes.schindelin@gmx.de>

It is the current coding style of the Git project to write

	if (...) {
		...
	} else {
		...
	}

instead of putting the closing brace and the "else" keyword on separate
lines.

Pointed out by Junio Hamano.

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

diff --git a/sequencer.c b/sequencer.c
index 23793db08b..3eededcb98 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1070,8 +1070,7 @@ static int create_seq_dir(void)
 		error(_("a cherry-pick or revert is already in progress"));
 		advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
 		return -1;
-	}
-	else if (mkdir(git_path_seq_dir(), 0777) < 0)
+	} else if (mkdir(git_path_seq_dir(), 0777) < 0)
 		return error_errno(_("could not create sequencer directory '%s'"),
 				   git_path_seq_dir());
 	return 0;
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related

* [PATCH v3 00/38] Teach the sequencer to act as rebase -i's backend
From: Johannes Schindelin @ 2017-01-02 15:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer,
	Jeff King
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

This marks the count down to '3': two more patch series after this
(really tiny ones) and we have a faster rebase -i.

The idea of this patch series is to teach the sequencer to understand
all of the commands in `git-rebase-todo` scripts, to execute them and to
behave pretty much very the same as `git rebase -i --continue` when
called with the newly-introduced REPLAY_INTERACTIVE_REBASE mode.

Most of these patches should be pretty much straight-forward. When not,
I tried to make a point of describing enough background in the commit
message. Please feel free to point out where my explanations fall short.

Note that even after this patch series is applied, rebase -i is still
unaffected. It will require the next patch series which introduces the
rebase--helper that essentially implements `git rebase -i --continue` by
calling the sequencer with the appropriate options.

The final patch series will move a couple of pre- and post-processing
steps into the rebase--helper/sequencer (such as expanding/shrinking the
SHA-1s, reordering the fixup!/squash! lines, etc). This might sound like
a mere add-on, but it is essential for the speed improvements: those
stupid little processing steps really dominated the execution time in my
tests.

Apart from mostly cosmetic patches (and the occasional odd bug that I
fixed promptly), I used these patches since mid May to perform all of my
interactive rebases. In mid June, I had the idea to teach rebase -i to
run *both* scripted rebase and rebase--helper and to cross-validate the
results. This slowed down all my interactive rebases since, but helped
me catch three rather obscure bugs (e.g. that git commit --fixup unfolds
long onelines and rebase -i still finds the correct original commit).

This is all only to say that I am rather confident that the current code
does the job.

Since sending out v1, I integrated all of these patch series
into Git for Windows v2.10.0, where they have been live ever since, and
used by myself (also in a Linux VM, as Git for Windows' master branch
always builds also on Linux and passes the test suite, too).

Just to reiterate why I do all this: it speeds up the interactive rebase
substantially. Even with a not yet fully builtin rebase -i, but just the
part after the user edited the `git-rebase-todo` script.

The performance test I introduced to demonstrate this (p3404) shows a
speed-up of +380% here (i.e. roughly 5x), from ~8.8 seconds to ~1.8
seconds. This is on Windows, where the performance impact of avoiding
shell scripting is most noticable.

On MacOSX and on Linux, the speed-up is less pronounced, but still
noticable, at least if you trust Travis CI, which I abused to perform
that test for me. Check for yourself (searching for "3404.2") here:
https://travis-ci.org/git/git/builds/156295227. According to those logs,
p3404 is speeded up from ~0.45 seconds to ~0.12 seconds on Linux (read:
about 3.5x) and from ~1.7 seconds to ~0.5 seconds on MacOSX (read:
almost 4x).

Changes since v2:

- fixed a TRANSLATORS: comment

- added a comment to clarify why save_todo() does not write the current
  command to git-rebase-todo

- fixed the comment for "stopped-sha" that said that a long commit name
  is stored in that file, when it is in fact an abbreviated one

- consistently put the "else" keyword on the same line as the preceding
  closing brace, if any (this adds two patches to the already large
  patch series, sorry)

- completely revamped the update_squash_messages() function to make it
  more obvious why it does not overflow its buffer, and to fix it when
  using it with localised messages

- now the sequencer code uses find_commit_subject() consistently to find
  the commit message (it does not use the subject's length returned by
  the find_commit_subject() function, of course); this adds yet another
  patch to this already large patch series, sorry.

- introduced an is_noop() function (as opposed to testing <= TODO_NOOP
  manually) to make the code less magic.

- fixed two potential leaks of get_commit_buffer()'s returned buffer

- fixed leaks in the error code paths when trying to update the ref at
  the end

- renamed read_author_script() into read_env_script() to reflect that
  it would accept any environment setting, and made it much more
  readable

- completely reworked the strategy how to suppress the output of
  successful cherry-picks: instead of introducing
  RUN_HIDE_STDERR_ON_SUCCESS, the code now uses pipe_command() to catch
  the output itself, so that it can be shown in case of error

- replaced a spawned `diff-tree` command by code using the diff functions
  directly


Johannes Schindelin (38):
  sequencer: avoid unnecessary curly braces
  sequencer: move "else" keyword onto the same line as preceding brace
  sequencer: use a helper to find the commit message
  sequencer: support a new action: 'interactive rebase'
  sequencer (rebase -i): implement the 'noop' command
  sequencer (rebase -i): implement the 'edit' command
  sequencer (rebase -i): implement the 'exec' command
  sequencer (rebase -i): learn about the 'verbose' mode
  sequencer (rebase -i): write the 'done' file
  sequencer (rebase -i): add support for the 'fixup' and 'squash'
    commands
  sequencer (rebase -i): implement the short commands
  sequencer (rebase -i): write an author-script file
  sequencer (rebase -i): allow continuing with staged changes
  sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
  sequencer (rebase -i): skip some revert/cherry-pick specific code path
  sequencer (rebase -i): the todo can be empty when continuing
  sequencer (rebase -i): update refs after a successful rebase
  sequencer (rebase -i): leave a patch upon error
  sequencer (rebase -i): implement the 'reword' command
  sequencer (rebase -i): allow fast-forwarding for edit/reword
  sequencer (rebase -i): refactor setting the reflog message
  sequencer (rebase -i): set the reflog message consistently
  sequencer (rebase -i): copy commit notes at end
  sequencer (rebase -i): record interrupted commits in rewritten, too
  sequencer (rebase -i): run the post-rewrite hook, if needed
  sequencer (rebase -i): respect the rebase.autostash setting
  sequencer (rebase -i): respect strategy/strategy_opts settings
  sequencer (rebase -i): allow rescheduling commands
  sequencer (rebase -i): implement the 'drop' command
  sequencer (rebase -i): differentiate between comments and 'noop'
  sequencer: make reading author-script more elegant
  sequencer: use run_command() directly
  sequencer (rebase -i): show only failed `git commit`'s output
  sequencer (rebase -i): show only failed cherry-picks' output
  sequencer (rebase -i): suggest --edit-todo upon unknown command
  sequencer (rebase -i): show the progress
  sequencer (rebase -i): write the progress into files
  sequencer (rebase -i): write out the final message

 sequencer.c | 1105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 sequencer.h |    4 +-
 2 files changed, 1013 insertions(+), 96 deletions(-)


base-commit: e05806da9ec4aff8adfed142ab2a2b3b02e33c8c
Published-As: https://github.com/dscho/git/releases/tag/sequencer-i-v3
Fetch-It-Via: git fetch https://github.com/dscho/git sequencer-i-v3

Interdiff vs v2:

 diff --git a/run-command.c b/run-command.c
 index 5bb957afdd..ca905a9e80 100644
 --- a/run-command.c
 +++ b/run-command.c
 @@ -589,29 +589,6 @@ 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 65a21ddd4e..dd1c78c28d 100644
 --- a/run-command.h
 +++ b/run-command.h
 @@ -72,7 +72,6 @@ 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);
  
  /*
 diff --git a/sequencer.c b/sequencer.c
 index c0aeb4f5f7..0e7d2ca5c8 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -102,7 +102,7 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
  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.
 + * the abbreviated commit name of the corresponding patch.
   */
  static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
  /*
 @@ -464,8 +464,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
  
  	if (active_cache_changed &&
  	    write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
 -		/*
 -		 * TRANSLATORS: %s will be "revert", "cherry-pick" or
 +		/* TRANSLATORS: %s will be "revert", "cherry-pick" or
  		 * "rebase -i".
  		 */
  		return error(_("%s: Unable to write new index file"),
 @@ -524,8 +523,7 @@ static int write_author_script(const char *message)
  			/* Missing 'author' line? */
  			unlink(rebase_path_author_script());
  			return 0;
 -		}
 -		else if (skip_prefix(message, "author ", &message))
 +		} else if (skip_prefix(message, "author ", &message))
  			break;
  		else if ((eol = strchr(message, '\n')))
  			message = eol + 1;
 @@ -560,18 +558,17 @@ static int write_author_script(const char *message)
  }
  
  /*
 - * Read the author-script file into an environment block, ready for use in
 - * run_command(), that can be free()d afterwards.
 + * Read a list of environment variable assignments (such as the author-script
 + * file) into an environment block. Returns -1 on error, 0 otherwise.
   */
 -static char **read_author_script(void)
 +static int read_env_script(struct argv_array *env)
  {
  	struct strbuf script = STRBUF_INIT;
  	int i, count = 0;
 -	char *p, *p2, **env;
 -	size_t env_size;
 +	char *p, *p2;
  
  	if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
 -		return NULL;
 +		return -1;
  
  	for (p = script.buf; *p; p++)
  		if (skip_prefix(p, "'\\\\''", (const char **)&p2))
 @@ -583,19 +580,12 @@ static char **read_author_script(void)
  			count++;
  		}
  
 -	env_size = (count + 1) * sizeof(*env);
 -	strbuf_grow(&script, env_size);
 -	memmove(script.buf + env_size, script.buf, script.len);
 -	p = script.buf + env_size;
 -	env = (char **)strbuf_detach(&script, NULL);
 -
  	for (i = 0; i < count; i++) {
 -		env[i] = p;
 +		argv_array_push(env, p);
  		p += strlen(p) + 1;
  	}
 -	env[count] = NULL;
  
 -	return env;
 +	return 0;
  }
  
  static const char staged_changes_advice[] =
 @@ -628,19 +618,18 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
  			  int allow_empty, int edit, int amend,
  			  int cleanup_commit_message)
  {
 -	char **env = NULL;
 -	struct argv_array array;
 -	int opt = RUN_GIT_CMD, rc;
 +	struct child_process cmd = CHILD_PROCESS_INIT;
  	const char *value;
  
 +	cmd.git_cmd = 1;
 +
  	if (is_rebase_i(opts)) {
  		if (!edit) {
 -			opt |= RUN_COMMAND_STDOUT_TO_STDERR;
 -			opt |= RUN_HIDE_STDERR_ON_SUCCESS;
 +			cmd.stdout_to_stderr = 1;
 +			cmd.err = -1;
  		}
  
 -		env = read_author_script();
 -		if (!env) {
 +		if (read_env_script(&cmd.env_array)) {
  			const char *gpg_opt = gpg_sign_opt_quoted(opts);
  
  			return error(_(staged_changes_advice),
 @@ -648,39 +637,47 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
  		}
  	}
  
 -	argv_array_init(&array);
 -	argv_array_push(&array, "commit");
 -	argv_array_push(&array, "-n");
 +	argv_array_push(&cmd.args, "commit");
 +	argv_array_push(&cmd.args, "-n");
  
  	if (amend)
 -		argv_array_push(&array, "--amend");
 +		argv_array_push(&cmd.args, "--amend");
  	if (opts->gpg_sign)
 -		argv_array_pushf(&array, "-S%s", opts->gpg_sign);
 +		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
  	if (opts->signoff)
 -		argv_array_push(&array, "-s");
 +		argv_array_push(&cmd.args, "-s");
  	if (defmsg)
 -		argv_array_pushl(&array, "-F", defmsg, NULL);
 +		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
  	if (cleanup_commit_message)
 -		argv_array_push(&array, "--cleanup=strip");
 +		argv_array_push(&cmd.args, "--cleanup=strip");
  	if (edit)
 -		argv_array_push(&array, "-e");
 +		argv_array_push(&cmd.args, "-e");
  	else if (!cleanup_commit_message &&
  		 !opts->signoff && !opts->record_origin &&
  		 git_config_get_value("commit.cleanup", &value))
 -		argv_array_push(&array, "--cleanup=verbatim");
 +		argv_array_push(&cmd.args, "--cleanup=verbatim");
  
  	if (allow_empty)
 -		argv_array_push(&array, "--allow-empty");
 +		argv_array_push(&cmd.args, "--allow-empty");
  
  	if (opts->allow_empty_message)
 -		argv_array_push(&array, "--allow-empty-message");
 +		argv_array_push(&cmd.args, "--allow-empty-message");
  
 -	rc = run_command_v_opt_cd_env(array.argv, opt, NULL,
 -			(const char *const *)env);
 -	argv_array_clear(&array);
 -	free(env);
 +	if (cmd.err == -1) {
 +		/* hide stderr on success */
 +		struct strbuf buf = STRBUF_INIT;
 +		int rc = pipe_command(&cmd,
 +				      NULL, 0,
 +				      /* stdout is already redirected */
 +				      NULL, 0,
 +				      &buf, 0);
 +		if (rc)
 +			fputs(buf.buf, stderr);
 +		strbuf_release(&buf);
 +		return rc;
 +	}
  
 -	return rc;
 +	return run_command(&cmd);
  }
  
  static int is_original_commit_empty(struct commit *commit)
 @@ -786,6 +783,11 @@ static const char *command_to_string(const enum todo_command command)
  	die("Unknown command: %d", command);
  }
  
 +static int is_noop(const enum todo_command command)
 +{
 +	return TODO_NOOP <= command;
 +}
 +
  static int is_fixup(enum todo_command command)
  {
  	return command == TODO_FIXUP || command == TODO_SQUASH;
 @@ -799,36 +801,33 @@ static int update_squash_messages(enum todo_command command,
  	const char *message, *body;
  
  	if (file_exists(rebase_path_squash_msg())) {
 -		char *p, *p2;
 +		struct strbuf header = STRBUF_INIT;
 +		char *eol, *p;
  
  		if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
  			return error(_("could not read '%s'"),
  				rebase_path_squash_msg());
  
 +		p = buf.buf + 1;
 +		eol = strchrnul(buf.buf, '\n');
  		if (buf.buf[0] != comment_line_char ||
 -		    !skip_prefix(buf.buf + 1, " This is a combination of ",
 -				 (const char **)&p))
 +		    (p += strcspn(p, "0123456789\n")) == eol)
  			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);
 +				     (int)(eol - buf.buf), buf.buf);
 +		count = strtol(p, NULL, 10);
  
 -		if (count < 1 || *p2 != ' ')
 +		if (count < 1)
  			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 {
 +				     (int)(eol - buf.buf), buf.buf);
 +
 +		strbuf_addf(&header, "%c ", comment_line_char);
 +		strbuf_addf(&header,
 +			    _("This is a combination of %d commits."), ++count);
 +		strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len);
 +		strbuf_release(&header);
 +	} else {
  		unsigned char head[20];
  		struct commit *head_commit;
  		const char *head_message, *body;
 @@ -840,20 +839,22 @@ static int update_squash_messages(enum todo_command command,
  		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);
 +		find_commit_subject(head_message, &body);
  		if (write_message(body, strlen(body),
 -				  rebase_path_fixup_msg(), 0))
 +				  rebase_path_fixup_msg(), 0)) {
 +			unuse_commit_buffer(head_commit, head_message);
  			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);
 +		strbuf_addf(&buf, "%c ", comment_line_char);
 +		strbuf_addf(&buf, _("This is a combination of %d commits."),
 +			    count);
 +		strbuf_addf(&buf, "\n%c ", comment_line_char);
 +		strbuf_addstr(&buf, _("This is the 1st commit message:"));
 +		strbuf_addstr(&buf, "\n\n");
 +		strbuf_addstr(&buf, body);
  
  		unuse_commit_buffer(head_commit, head_message);
  	}
 @@ -861,25 +862,21 @@ static int update_squash_messages(enum todo_command command,
  	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);
 +	find_commit_subject(message, &body);
  
  	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_addf(&buf, "\n%c ", comment_line_char);
 +		strbuf_addf(&buf, _("This is the commit message #%d:"), count);
 +		strbuf_addstr(&buf, "\n\n");
 +		strbuf_addstr(&buf, body);
 +	} else if (command == TODO_FIXUP) {
 +		strbuf_addf(&buf, "\n%c ", comment_line_char);
 +		strbuf_addf(&buf, _("The commit message #%d will be skipped:"),
 +			    count);
 +		strbuf_addstr(&buf, "\n\n");
  		strbuf_add_commented_lines(&buf, body, strlen(body));
 -	}
 -	else
 +	} else
  		return error(_("unknown command: %d"), command);
  	unuse_commit_buffer(commit, message);
  
 @@ -956,9 +953,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
  	}
  	discard_cache();
  
 -	if (!commit->parents) {
 +	if (!commit->parents)
  		parent = NULL;
 -	}
  	else if (commit->parents->next) {
  		/* Reverting or cherry-picking a merge commit */
  		int cnt;
 @@ -1036,14 +1032,9 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
  		next = commit;
  		next_label = msg.label;
  
 -		/*
 -		 * Append the commit log message to msgbuf; it starts
 -		 * after the tree, parent, author, committer
 -		 * information followed by "\n\n".
 -		 */
 -		p = strstr(msg.message, "\n\n");
 -		if (p)
 -			strbuf_addstr(&msgbuf, skip_blank_lines(p + 2));
 +		/* Append the commit log message to msgbuf. */
 +		if (find_commit_subject(msg.message, &p))
 +			strbuf_addstr(&msgbuf, p);
  
  		if (opts->record_origin) {
  			if (!has_conforming_footer(&msgbuf, NULL, 0))
 @@ -1065,8 +1056,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
  		else if (file_exists(rebase_path_fixup_msg())) {
  			cleanup_commit_message = 1;
  			msg_file = rebase_path_fixup_msg();
 -		}
 -		else {
 +		} else {
  			const char *dest = git_path("SQUASH_MSG");
  			unlink(dest);
  			if (copy_file(dest, rebase_path_squash_msg(), 0666))
 @@ -1241,8 +1231,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
  		if (skip_prefix(bol, todo_command_info[i].str, &bol)) {
  			item->command = i;
  			break;
 -		}
 -		else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {
 +		} else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {
  			bol++;
  			item->command = i;
  			break;
 @@ -1312,7 +1301,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 (!is_noop(item->command))
  			fixup_okay = 1;
  	}
  
 @@ -1533,8 +1522,7 @@ static int create_seq_dir(void)
  		error(_("a cherry-pick or revert is already in progress"));
  		advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
  		return -1;
 -	}
 -	else if (mkdir(git_path_seq_dir(), 0777) < 0)
 +	} else if (mkdir(git_path_seq_dir(), 0777) < 0)
  		return error_errno(_("could not create sequencer directory '%s'"),
  				   git_path_seq_dir());
  	return 0;
 @@ -1667,6 +1655,10 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
  	const char *todo_path = get_todo_path(opts);
  	int next = todo_list->current, offset, fd;
  
 +	/*
 +	 * rebase -i writes "git-rebase-todo" without the currently executing
 +	 * command, appending it to "done" instead.
 +	 */
  	if (is_rebase_i(opts))
  		next++;
  
 @@ -1739,7 +1731,7 @@ 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;
 +	const char *subject, *p;
  	int res = 0;
  
  	p = short_commit_name(commit);
 @@ -1766,6 +1758,7 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
  
  	strbuf_addf(&buf, "%s/message", get_dir(opts));
  	if (!file_exists(buf.buf)) {
 +		const char *commit_buffer = get_commit_buffer(commit, NULL);
  		find_commit_subject(commit_buffer, &subject);
  		res |= write_message(subject, strlen(subject), buf.buf, 1);
  		unuse_commit_buffer(commit, commit_buffer);
 @@ -1805,8 +1798,7 @@ static int error_with_patch(struct commit *commit,
  			"Once you are satisfied with your changes, run\n"
  			"\n"
  			"  git rebase --continue\n", gpg_sign_opt_quoted(opts));
 -	}
 -	else if (exit_code)
 +	} else if (exit_code)
  		fprintf(stderr, "Could not apply %s... %.*s\n",
  			short_commit_name(commit), subject_len, subject);
  
 @@ -1854,8 +1846,7 @@ static int do_exec(const char *command_line)
  		if (status == 127)
  			/* command not found */
  			status = 1;
 -	}
 -	else if (dirty) {
 +	} 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"
 @@ -1878,7 +1869,7 @@ static int is_final_fixup(struct todo_list *todo_list)
  	while (++i < todo_list->nr)
  		if (is_fixup(todo_list->items[i].command))
  			return 0;
 -		else if (todo_list->items[i].command < TODO_NOOP)
 +		else if (!is_noop(todo_list->items[i].command))
  			break;
  	return 1;
  }
 @@ -1888,7 +1879,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 (!is_noop(todo_list->items[i].command))
  			return todo_list->items[i].command;
  
  	return -1;
 @@ -2021,21 +2012,18 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
  					intend_to_amend();
  				return error_failed_squash(item->commit, opts,
  					item->arg_len, item->arg);
 -			}
 -			else if (res && is_rebase_i(opts))
 +			} else if (res && is_rebase_i(opts))
  				return res | error_with_patch(item->commit,
  					item->arg, item->arg_len, opts, res,
  					item->command == TODO_REWORD);
 -		}
 -		else if (item->command == TODO_EXEC) {
 +		} 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)
 +		} else if (!is_noop(item->command))
  			return error(_("unknown command %d"), item->command);
  
  		todo_list->current++;
 @@ -2055,40 +2043,60 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
  				starts_with(head_ref.buf, "refs/")) {
  			const char *msg;
  			unsigned char head[20], orig[20];
 -
 -			if (get_sha1("HEAD", head))
 -				return error(_("cannot read HEAD"));
 +			int res;
 +
 +			if (get_sha1("HEAD", head)) {
 +				res = error(_("cannot read HEAD"));
 +cleanup_head_ref:
 +				strbuf_release(&head_ref);
 +				strbuf_release(&buf);
 +				return res;
 +			}
  			if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
 -					get_sha1_hex(buf.buf, orig))
 -				return error(_("could not read orig-head"));
 -			if (!read_oneliner(&buf, rebase_path_onto(), 0))
 -				return error(_("could not read 'onto'"));
 +					get_sha1_hex(buf.buf, orig)) {
 +				res = error(_("could not read orig-head"));
 +				goto cleanup_head_ref;
 +			}
 +			if (!read_oneliner(&buf, rebase_path_onto(), 0)) {
 +				res = error(_("could not read 'onto'"));
 +				goto cleanup_head_ref;
 +			}
  			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"),
 +					REF_NODEREF, UPDATE_REFS_MSG_ON_ERR)) {
 +				res = error(_("could not update %s"),
  					head_ref.buf);
 +				goto cleanup_head_ref;
 +			}
  			msg = reflog_message(opts, "finish", "returning to %s",
  				head_ref.buf);
 -			if (create_symref("HEAD", head_ref.buf, msg))
 -				return error(_("could not update HEAD to %s"),
 +			if (create_symref("HEAD", head_ref.buf, msg)) {
 +				res = error(_("could not update HEAD to %s"),
  					head_ref.buf);
 +				goto cleanup_head_ref;
 +			}
  			strbuf_reset(&buf);
  		}
  
  		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);
 +			struct rev_info log_tree_opt;
 +			struct object_id orig, head;
 +
 +			memset(&log_tree_opt, 0, sizeof(log_tree_opt));
 +			init_revisions(&log_tree_opt, NULL);
 +			log_tree_opt.diff = 1;
 +			log_tree_opt.diffopt.output_format =
 +				DIFF_FORMAT_DIFFSTAT;
 +			log_tree_opt.disable_stdin = 1;
 +
 +			if (read_oneliner(&buf, rebase_path_orig_head(), 0) &&
 +			    !get_sha1(buf.buf, orig.hash) &&
 +			    !get_sha1("HEAD", head.hash)) {
 +				diff_tree_sha1(orig.hash, head.hash,
 +					       "", &log_tree_opt.diffopt);
 +				log_tree_diff_flush(&log_tree_opt);
 +			}
  		}
  		flush_rewritten_pending();
  		if (!stat(rebase_path_rewritten_list(), &st) &&
 @@ -2195,8 +2203,7 @@ int sequencer_continue(struct replay_opts *opts)
  	if (is_rebase_i(opts)) {
  		if (commit_staged_changes(opts))
  			return -1;
 -	}
 -	else 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;
 @@ -2216,8 +2223,7 @@ int sequencer_continue(struct replay_opts *opts)
  			goto release_todo_list;
  		}
  		todo_list.current++;
 -	}
 -	else if (file_exists(rebase_path_stopped_sha())) {
 +	} else if (file_exists(rebase_path_stopped_sha())) {
  		struct strbuf buf = STRBUF_INIT;
  		struct object_id oid;
  

-- 
2.11.0.rc3.windows.1


^ permalink raw reply

* Re: [PATCH v2 05/34] sequencer (rebase -i): learn about the 'verbose' mode
From: Johannes Schindelin @ 2017-01-02 15:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqq37hr1orb.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 13 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > @@ -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);
> >  	}
> 
> It's a bit curious that the previous step avoided running a separate
> process and instead did "diff-tree -p" all in C, but this one does not.

I guess my only defence is that I tried to be a little lazy.

Fixed.
Dscho

^ permalink raw reply

* Re: [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id
From: Michael Haggerty @ 2017-01-02 15:07 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170101191847.564741-14-sandals@crustytoothpaste.net>

On 01/01/2017 08:18 PM, brian m. carlson wrote:
> Make each_reflog_ent_fn take two struct object_id pointers instead of
> two pointers to unsigned char.  Convert the various callbacks to use
> struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
> fsck_handle_reflog_oid.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/fsck.c       | 16 ++++++++--------
>  builtin/merge-base.c |  6 +++---
>  builtin/reflog.c     |  2 +-
>  reflog-walk.c        |  6 +++---
>  refs.c               | 24 ++++++++++++------------
>  refs.h               |  2 +-
>  refs/files-backend.c | 24 ++++++++++++------------
>  revision.c           | 12 ++++++------
>  sha1_name.c          |  2 +-
>  wt-status.c          |  6 +++---
>  10 files changed, 50 insertions(+), 50 deletions(-)
> 
> [...]
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index f9023939d..3da3141ee 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3113,15 +3113,15 @@ static int files_delete_reflog(struct ref_store *ref_store,
>  
>  static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
>  {
> -	unsigned char osha1[20], nsha1[20];
> +	struct object_id ooid, noid;
>  	char *email_end, *message;
>  	unsigned long timestamp;
>  	int tz;
>  
>  	/* old SP new SP name <email> SP time TAB msg LF */
>  	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
> -	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
> -	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
> +	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
> +	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||

Some magic numbers above could be converted to use constants.

>  	    !(email_end = strchr(sb->buf + 82, '>')) ||
>  	    email_end[1] != ' ' ||
>  	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
> @@ -3136,7 +3136,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
>  		message += 6;
>  	else
>  		message += 7;
> -	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
> +	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);

Here, too.

>  }
>  
>  static char *find_beginning_of_line(char *bob, char *scan)
> [...]
> @@ -4047,14 +4047,14 @@ static int files_reflog_expire(struct ref_store *ref_store,
>  		 */
>  		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
>  			!(type & REF_ISSYMREF) &&
> -			!is_null_sha1(cb.last_kept_sha1);
> +			!is_null_oid(&cb.last_kept_oid);
>  
>  		if (close_lock_file(&reflog_lock)) {
>  			status |= error("couldn't write %s: %s", log_file,
>  					strerror(errno));
>  		} else if (update &&
>  			   (write_in_full(get_lock_file_fd(lock->lk),
> -				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
> +				oid_to_hex(&cb.last_kept_oid), 40) != 40 ||

More magic numbers above.

>  			    write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
>  			    close_ref(lock) < 0)) {
>  			status |= error("couldn't write %s",
> [...]

I thought it would make sense to convert `struct read_ref_at_cb` in
`refs.c` to use `struct object_id` at the same time, but I see that
would require the interface to `read_ref_at()` to change. I guess it's
important to pick your battles in this campaign :-)

Michael


^ permalink raw reply

* Re: [PATCH v2 20/34] sequencer (rebase -i): copy commit notes at end
From: Johannes Schindelin @ 2017-01-02 14:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqvaujr7ed.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 16 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > @@ -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;
> 
> Makes me wonder, after having commented on 07/34 regarding the fact
> that in the end you would end up having three variants of no-op
> (i.e. NOOP, DROP and COMMENT), what definition of a "command" this
> function uses to return its result, when asked to "peek".

Well, it uses the todo_command idea of a "command"... ;-)

The only thing we do with this for now is to look whether the next command
is a fixup/squash (so that the user gets to edit the commit message just
once, for example, and also to record rewritten commits properly).

> I suspect that this will be updated in a later patch to do "< TODO_NOOP"
> instead?

Actually, no. I introduced a new function is_noop() and that is used now.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds
From: Johannes Schindelin @ 2017-01-02 14:38 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Jeff King, git, Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <f4a72743-3488-3466-5b9f-0dacec102a54@kdbg.org>

Hi Hannes,

On Wed, 14 Dec 2016, Johannes Sixt wrote:

> Am 14.12.2016 um 14:06 schrieb Jeff King:
> > On Wed, Dec 14, 2016 at 07:53:23AM -0500, Jeff King wrote:
> >
> > > I don't have a strong opinion on the patches under discussion, but
> > > here are a few pointers on the run-command interface:
> > > [...]
> >
> > And here is a patch representing my suggestions, on top of yours. Not
> > tested beyond "make test".
> 
> Thank you, that looks way better.
> 
> If there is agreement that this approach is preferable, I think we can
> have patches on top of the series; they would be orthogonal and do not
> have to take hostage of it. (And it looks like I won't be able to follow
> up until later this week[end].)

Seeing as the original intention was to do away with the
RUN_HIDE_STDERR_ON_SUCCESS flag, and that the sequencer-i branch *must*
include that functionality somehow, it is unfortunately not really
possible to do this on top of the patch series.

I say "unfortunately" because I feel pretty uncomfortable with replacing
something that has been tried and tested by something that still awaits
the test of time.

So the only possible course of action I see is to go the really long
route: incorporate the patches to use pipe_command() instead of
introducing a new RUN_* flag (which means basically munch up your patch
and Peff's and move it somewhere into the middle of the sequencer-i patch
series, which is exactly what I already did locally), cook the patches
beyond recognition in `next`, i.e. cook it really long to give it a really
good testing before moving the patches to `master`.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 09/17] builtin/merge: convert to struct object_id
From: Michael Haggerty @ 2017-01-02 14:34 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170101191847.564741-10-sandals@crustytoothpaste.net>

On 01/01/2017 08:18 PM, brian m. carlson wrote:
> Additionally convert several uses of the constant 40 into
> GIT_SHA1_HEXSZ.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/merge.c | 136 ++++++++++++++++++++++++++++----------------------------
>  1 file changed, 68 insertions(+), 68 deletions(-)
> 
> [...]
> @@ -437,25 +437,25 @@ static void merge_name(const char *remote, struct strbuf *msg)
>  	strbuf_branchname(&bname, remote);
>  	remote = bname.buf;
>  
> -	memset(branch_head, 0, sizeof(branch_head));
> +	memset(&branch_head, 0, sizeof(branch_head));

I think this could be

        oidclr(&branch_head);

>  	remote_head = get_merge_parent(remote);
>  	if (!remote_head)
>  		die(_("'%s' does not point to a commit"), remote);
> [...]
> @@ -1113,9 +1113,9 @@ static struct commit_list *collect_parents(struct commit *head_commit,
>  
>  int cmd_merge(int argc, const char **argv, const char *prefix)
>  {
> -	unsigned char result_tree[20];
> -	unsigned char stash[20];
> -	unsigned char head_sha1[20];
> +	struct object_id result_tree;
> +	struct object_id stash;
> +	struct object_id head_oid;

These could comfortably be declared on a single line now.

>  	struct commit *head_commit;
>  	struct strbuf buf = STRBUF_INIT;
>  	const char *head_arg;
> [...]

Michael


^ permalink raw reply

* Re: [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire
From: Christian Couder @ 2017-01-02 14:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqeg0t9oct.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  Documentation/config.txt | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index e0f5a77980..24e771d22e 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2786,6 +2786,17 @@ splitIndex.maxPercentChange::
>>       than 20 percent of the total number of entries.
>>       See linkgit:git-update-index[1].
>>
>> +splitIndex.sharedIndexExpire::
>> +     When the split index feature is used, shared index files with
>> +     a mtime older than this time will be removed when a new shared
>
> As end-user facing documentation, it would be much better if we can
> rephrase it for those who do not know what a 'mtime' is, and it
> would be even better if we can do so without losing precision.
>
> I think "shared index files that were not modified since the time
> this variable specifies will be removed" would be understandable and
> correct enough?

Yeah, I agree it is better for end users. I will use what you suggest.

>> +     index file is created. The value "now" expires all entries
>> +     immediately, and "never" suppresses expiration altogether.
>> +     The default value is "one.week.ago".
>> +     Note that each time a new split-index file is created, the
>> +     mtime of the related shared index file is updated to the
>> +     current time.
>
> To match the above suggestion, "Note that a shared index file is
> considered modified (for the purpose of expiration) each time a new
> split-index file is created based on it."?

Yeah, I also agree it is better and will use that.

^ permalink raw reply

* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Christian Couder @ 2017-01-02 14:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqa8bhb32x.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +/*
>> + * Signal that the shared index is used by updating its mtime.
>> + *
>> + * This way, shared index can be removed if they have not been used
>> + * for some time. It's ok to fail to update the mtime if we are on a
>> + * read only file system.
>> + */
>> +void freshen_shared_index(char *base_sha1_hex)
>> +{
>> +     const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
>> +     check_and_freshen_file(shared_index, 1);
>
> What happens when this call fails?  The function returns 0 if the
> file did not even exist.  It also returns 0 if you cannot update its
> timestamp.

Yeah and I don't think it's a problem in either case.
If we cannot update its timestamp, it's not a problem, as we could be
on a read-only file system, and you said in a previous iteration that
we should not even warn in this case.
If the file does not exist, it could be because it has just been
deleted for a good reason, and anyway, if it is a problem that the
shared index file has been deleted, it is better addressed when we
will actually need the shared index file to read something into from
it, rather than just to update its mtime.

> Shouldn't the series be exposing freshen_file() instead _and_ taking
> its error return value seriously?

So what should we do if freshen_file() returns 0 which means that the
freshening failed?

>> +}

^ permalink raw reply

* Re: [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2017-01-02 13:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqlgv1b34y.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  Documentation/config.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 221c5982c0..e0f5a77980 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2773,6 +2773,19 @@ showbranch.default::
>>       The default set of branches for linkgit:git-show-branch[1].
>>       See linkgit:git-show-branch[1].
>>
>> +splitIndex.maxPercentChange::
>> +     When the split index feature is used, this specifies the
>> +     percent of entries the split index can contain compared to the
>> +     whole number of entries in both the split index and the shared
>
> s/whole/total/ to match the last sentence of this section, perhaps?
> "The number of all entries" would also be OK, but "the whole number
> of entries" sounds a bit strange.

Yeah "total" is better than "whole" here. I will use "total".

^ permalink raw reply

* Re: [PATCH] don't use test_must_fail with grep
From: Pranit Bauva @ 2017-01-02 13:40 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Luke Diamand, Git Users, Stefan Beller
In-Reply-To: <285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org>

Hey Johannes,

On Sun, Jan 1, 2017 at 8:20 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> which makes me wonder: Is the message that we do expect not to occur
> actually printed on stdout? It sounds much more like an error message, i.e.,
> text that is printed on stderr. Wouldn't we need this?
>
>         git p4 commit >actual 2>&1 &&
>         ! grep "git author.*does not match" actual &&
>
> -- Hannes

This seems better! Since I am at it, I can remove the traces of pipes
in an another patch.

Regards,
Pranit Bauva

^ permalink raw reply

* [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Johannes Schindelin @ 2017-01-02 12:09 UTC (permalink / raw)
  To: git; +Cc: Segev Finer, Junio C Hamano

From: Segev Finer <segev208@gmail.com>

Git for Windows has special support for the popular SSH client PuTTY:
when using PuTTY's non-interactive version ("plink.exe"), we use the -P
option to specify the port rather than OpenSSH's -p option. TortoiseGit
ships with its own, forked version of plink.exe, that adds support for
the -batch option, and for good measure we special-case that, too.

However, this special-casing of PuTTY only covers the case where the
user overrides the SSH command via the environment variable GIT_SSH
(which allows specifying the name of the executable), not
GIT_SSH_COMMAND (which allows specifying a full command, including
additional command-line options).

When users want to pass any additional arguments to (Tortoise-)Plink,
such as setting a private key, they are required to either use a shell
script named plink or tortoiseplink or duplicate the logic that is
already in Git for passing the correct style of command line arguments,
which can be difficult, error prone and annoying to get right.

This patch simply reuses the existing logic and expands it to cover
GIT_SSH_COMMAND, too.

Note: it may look a little heavy-handed to duplicate the entire
command-line and then split it, only to extract the name of the
executable. However, this is not a performance-critical code path, and
the code is much more readable this way.

Signed-off-by: Segev Finer <segev208@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/putty-w-args-v1
Fetch-It-Via: git fetch https://github.com/dscho/git putty-w-args-v1


	Original Pull Request:
	https://github.com/git-for-windows/git/pull/1006

 connect.c        | 23 ++++++++++++++++-------
 t/t5601-clone.sh | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/connect.c b/connect.c
index 8cb93b0720..c81f77001b 100644
--- a/connect.c
+++ b/connect.c
@@ -772,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 			int putty = 0, tortoiseplink = 0;
 			char *ssh_host = hostandport;
 			const char *port = NULL;
+			char *ssh_argv0 = NULL;
 			transport_check_allowed("ssh");
 			get_host_and_port(&ssh_host, &port);
 
@@ -792,10 +793,15 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 
 			ssh = get_ssh_command();
-			if (!ssh) {
-				const char *base;
-				char *ssh_dup;
-
+			if (ssh) {
+				char *split_ssh = xstrdup(ssh);
+				const char **ssh_argv;
+
+				if (split_cmdline(split_ssh, &ssh_argv))
+					ssh_argv0 = xstrdup(ssh_argv[0]);
+				free(split_ssh);
+				free((void *)ssh_argv);
+			} else {
 				/*
 				 * GIT_SSH is the no-shell version of
 				 * GIT_SSH_COMMAND (and must remain so for
@@ -807,8 +813,11 @@ struct child_process *git_connect(int fd[2], const char *url,
 				if (!ssh)
 					ssh = "ssh";
 
-				ssh_dup = xstrdup(ssh);
-				base = basename(ssh_dup);
+				ssh_argv0 = xstrdup(ssh);
+			}
+
+			if (ssh_argv0) {
+				const char *base = basename(ssh_argv0);
 
 				tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
 					!strcasecmp(base, "tortoiseplink.exe");
@@ -816,7 +825,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 					!strcasecmp(base, "plink") ||
 					!strcasecmp(base, "plink.exe");
 
-				free(ssh_dup);
+				free(ssh_argv0);
 			}
 
 			argv_array_push(&conn->args, ssh);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index a433394200..5b228e2675 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -386,6 +386,21 @@ test_expect_success 'tortoiseplink is like putty, with extra arguments' '
 	expect_ssh "-batch -P 123" myhost src
 '
 
+test_expect_success 'double quoted plink.exe in GIT_SSH_COMMAND' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+	GIT_SSH_COMMAND="\"$TRASH_DIRECTORY/plink.exe\" -v" \
+		git clone "[myhost:123]:src" ssh-bracket-clone-plink-3 &&
+	expect_ssh "-v -P 123" myhost src
+'
+
+SQ="'"
+test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+	GIT_SSH_COMMAND="$SQ$TRASH_DIRECTORY/plink.exe$SQ -v" \
+		git clone "[myhost:123]:src" ssh-bracket-clone-plink-4 &&
+	expect_ssh "-v -P 123" myhost src
+'
+
 # Reset the GIT_SSH environment variable for clone tests.
 setup_ssh_wrapper
 

base-commit: e05806da9ec4aff8adfed142ab2a2b3b02e33c8c
-- 
2.11.0.rc3.windows.1

^ permalink raw reply related

* Re: [PATCH v3 10/21] read-cache: regenerate shared index if necessary
From: Christian Couder @ 2017-01-02 11:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqk2al9ocv.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +     case 0:
>> +             return 1; /* 0% means always write a new shared index */
>> +     case 100:
>> +             return 0; /* 100% means never write a new shared index */
>> +     default:
>> +             ; /* do nothing: just use the configured value */
>> +     }
>
> Just like you did in 04/21, write "break" to avoid mistakes made in
> the future, i.e.
>
>         default:
>                 break; /* just use the configured value */

Ok, I will do that.

>> +
>> +     /* Count not shared entries */
>> +     for (i = 0; i < istate->cache_nr; i++) {
>> +             struct cache_entry *ce = istate->cache[i];
>> +             if (!ce->index)
>> +                     not_shared++;
>> +     }
>> +
>> +     return istate->cache_nr * max_split < not_shared * 100;
>
> On a 32-bit arch with 2G int and more than 20 million paths in the
> index, multiplying by max_split that can come close to 100 can
> theoretically cause integer overflow, but in practice it probably
> does not matter.  Or does it?

From a cursory look a "struct cache_entry" takes at least 80 bytes
without counting the "char name[FLEX_ARRAY]" on a 32 bit machine, so I
don't think it would be a good idea to work on a repo with 20 million
paths on a 32 bit machine, but maybe theoretically it could be a
problem.

To be safe I think I will use:

return (int64_t)istate->cache_nr * max_split < (int64_t)not_shared * 100;

^ permalink raw reply

* [RFC PATCH 3/5] error/warning framework framework: coccinelli rules
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Provide coccinelli rules which check for error(), warning() etc. with
localised argument and create a patch to replace them with error_(),
warning_() etc. in order to fully localize them.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 contrib/coccinelle/errorl10n.cocci | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 contrib/coccinelle/errorl10n.cocci

diff --git a/contrib/coccinelle/errorl10n.cocci b/contrib/coccinelle/errorl10n.cocci
new file mode 100644
index 0000000000..d62a440644
--- /dev/null
+++ b/contrib/coccinelle/errorl10n.cocci
@@ -0,0 +1,47 @@
+@@
+expression E;
+@@
+- usage(_(E));
++ usage_(_(E));
+
+@@
+expression E;
+@@
+- usagef(_(E));
++ usagef_(_(E));
+
+@@
+expression E;
+@@
+- die(_(E));
++ die_(_(E));
+
+@@
+expression E;
+@@
+- error(_(E));
++ error_(_(E));
+
+@@
+expression E;
+@@
+- warning(_(E));
++ warning_(_(E));
+
+@@
+expression E;
+@@
+- die_errno(_(E));
++ die_errno_(_(E));
+
+@@
+expression E;
+@@
+- error_errno(_(E));
++ error_errno_(_(E));
+
+@@
+expression E;
+@@
+- warning_errno(_(E));
++ warning_errno_(_(E));
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 2/5] error/warn framework: provide localized variants
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Provide localized variants of error(), warning(), die() etc.
to go along with localized messages.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 git-compat-util.h |  8 ++++++
 usage.c           | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index f1bf0a6749..48209a6c67 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -405,13 +405,21 @@ struct strbuf;
 /* General helper functions */
 extern void vreportf(const char *prefix, const char *err, va_list params);
 extern NORETURN void usage(const char *err);
+extern NORETURN void usage_(const char *err);
 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void usagef_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void die_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void die_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 #ifndef NO_OPENSSL
 #ifdef APPLE_COMMON_CRYPTO
diff --git a/usage.c b/usage.c
index 4270b04bf9..d0cfb02a64 100644
--- a/usage.c
+++ b/usage.c
@@ -105,11 +105,25 @@ void NORETURN usagef(const char *err, ...)
 	va_end(params);
 }
 
+void NORETURN usagef_(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	usage_routine(_("usage: "), err, params);
+	va_end(params);
+}
+
 void NORETURN usage(const char *err)
 {
 	usagef("%s", err);
 }
 
+void NORETURN usage_(const char *err)
+{
+	usagef_("%s", err);
+}
+
 void NORETURN die(const char *err, ...)
 {
 	va_list params;
@@ -124,6 +138,20 @@ void NORETURN die(const char *err, ...)
 	va_end(params);
 }
 
+void NORETURN die_(const char *err, ...)
+{
+	va_list params;
+
+	if (die_is_recursing()) {
+		fputs(_("fatal: recursion detected in die handler\n"), stderr);
+		exit(128);
+	}
+
+	va_start(params, err);
+	die_routine(_("fatal: "), err, params);
+	va_end(params);
+}
+
 static const char *fmt_with_err(char *buf, int n, const char *fmt)
 {
 	char str_error[256], *err;
@@ -163,6 +191,22 @@ void NORETURN die_errno(const char *fmt, ...)
 	va_end(params);
 }
 
+void NORETURN die_errno_(const char *fmt, ...)
+{
+	char buf[1024];
+	va_list params;
+
+	if (die_is_recursing()) {
+		fputs(_("fatal: recursion detected in die_errno handler\n"),
+			stderr);
+		exit(128);
+	}
+
+	va_start(params, fmt);
+	die_routine(_("fatal: "), fmt_with_err(buf, sizeof(buf), fmt), params);
+	va_end(params);
+}
+
 #undef error_errno
 int error_errno(const char *fmt, ...)
 {
@@ -175,6 +219,17 @@ int error_errno(const char *fmt, ...)
 	return -1;
 }
 
+int error_errno_(const char *fmt, ...)
+{
+	char buf[1024];
+	va_list params;
+
+	va_start(params, fmt);
+	error_routine(_("error: "), fmt_with_err(buf, sizeof(buf), fmt), params);
+	va_end(params);
+	return -1;
+}
+
 #undef error
 int error(const char *err, ...)
 {
@@ -186,6 +241,16 @@ int error(const char *err, ...)
 	return -1;
 }
 
+int error_(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	error_routine(_("error: "), err, params);
+	va_end(params);
+	return -1;
+}
+
 void warning_errno(const char *warn, ...)
 {
 	char buf[1024];
@@ -204,3 +269,12 @@ void warning(const char *warn, ...)
 	warn_routine("warning: ", warn, params);
 	va_end(params);
 }
+
+void warning_(const char *warn, ...)
+{
+	va_list params;
+
+	va_start(params, warn);
+	warn_routine(_("warning: "), warn, params);
+	va_end(params);
+}
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 1/5] error/warning framework: prepare for l10n
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Currently, errors, warnings etc. are output with a fixed prefix "error: "
etc. that is not subject to l10n.

Change the call signatures of error_routine() etc. so that they receive
the prefix as first argument.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 apply.c           |  2 +-
 apply.h           |  4 ++--
 daemon.c          |  3 ++-
 fast-import.c     |  4 ++--
 git-compat-util.h | 10 +++++-----
 http-backend.c    |  4 ++--
 run-command.c     |  4 ++--
 usage.c           | 48 ++++++++++++++++++++++++------------------------
 8 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/apply.c b/apply.c
index 2ed808d429..0f93792e1c 100644
--- a/apply.c
+++ b/apply.c
@@ -112,7 +112,7 @@ void clear_apply_state(struct apply_state *state)
 	/* &state->fn_table is cleared at the end of apply_patch() */
 }
 
-static void mute_routine(const char *msg, va_list params)
+static void mute_routine(const char *prefix, const char *msg, va_list params)
 {
 	/* do nothing */
 }
diff --git a/apply.h b/apply.h
index b3d6783d55..56b5622868 100644
--- a/apply.h
+++ b/apply.h
@@ -100,8 +100,8 @@ struct apply_state {
 	 * set_error_routine() or set_warn_routine() to install muting
 	 * routines when in verbosity_silent mode.
 	 */
-	void (*saved_error_routine)(const char *err, va_list params);
-	void (*saved_warn_routine)(const char *warn, va_list params);
+	void (*saved_error_routine)(const char *prefix, const char *err, va_list params);
+	void (*saved_warn_routine)(const char *prefix, const char *warn, va_list params);
 
 	/* These control whitespace errors */
 	enum apply_ws_error_action ws_error_action;
diff --git a/daemon.c b/daemon.c
index 473e6b6b63..cd52a25001 100644
--- a/daemon.c
+++ b/daemon.c
@@ -114,8 +114,9 @@ static void loginfo(const char *err, ...)
 	va_end(params);
 }
 
-static void NORETURN daemon_die(const char *err, va_list params)
+static void NORETURN daemon_die(const char *prefix, const char *err, va_list params)
 {
+	/* no need to pass down prefix */
 	logreport(LOG_ERR, err, params);
 	exit(1);
 }
diff --git a/fast-import.c b/fast-import.c
index f561ba833b..4576f12163 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -496,13 +496,13 @@ static void end_packfile(void);
 static void unkeep_all_packs(void);
 static void dump_marks(void);
 
-static NORETURN void die_nicely(const char *err, va_list params)
+static NORETURN void die_nicely(const char *prefix, const char *err, va_list params)
 {
 	static int zombie;
 	char message[2 * PATH_MAX];
 
 	vsnprintf(message, sizeof(message), err, params);
-	fputs("fatal: ", stderr);
+	fputs(prefix, stderr);
 	fputs(message, stderr);
 	fputc('\n', stderr);
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092b..f1bf0a6749 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -439,11 +439,11 @@ static inline int const_error(void)
 #define error_errno(...) (error_errno(__VA_ARGS__), const_error())
 #endif
 
-extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
-extern void set_error_routine(void (*routine)(const char *err, va_list params));
-extern void (*get_error_routine(void))(const char *err, va_list params);
-extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
-extern void (*get_warn_routine(void))(const char *warn, va_list params);
+extern void set_die_routine(NORETURN_PTR void (*routine)(const char *prefix, const char *err, va_list params));
+extern void set_error_routine(void (*routine)(const char *prefix, const char *err, va_list params));
+extern void (*get_error_routine(void))(const char *prefix, const char *err, va_list params);
+extern void set_warn_routine(void (*routine)(const char *prefix, const char *warn, va_list params));
+extern void (*get_warn_routine(void))(const char *prefix, const char *warn, va_list params);
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/http-backend.c b/http-backend.c
index eef0a361f4..5c235e8b52 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -577,12 +577,12 @@ static void service_rpc(struct strbuf *hdr, char *service_name)
 }
 
 static int dead;
-static NORETURN void die_webcgi(const char *err, va_list params)
+static NORETURN void die_webcgi(const char *prefix, const char *err, va_list params)
 {
 	if (dead <= 1) {
 		struct strbuf hdr = STRBUF_INIT;
 
-		vreportf("fatal: ", err, params);
+		vreportf(prefix, err, params);
 
 		http_status(&hdr, 500, "Internal Server Error");
 		hdr_nocache(&hdr);
diff --git a/run-command.c b/run-command.c
index ca905a9e80..3133bf5320 100644
--- a/run-command.c
+++ b/run-command.c
@@ -618,9 +618,9 @@ static void *run_thread(void *data)
 	return (void *)ret;
 }
 
-static NORETURN void die_async(const char *err, va_list params)
+static NORETURN void die_async(const char *prefix, const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf(prefix, err, params);
 
 	if (in_async()) {
 		struct async *async = pthread_getspecific(async_key);
diff --git a/usage.c b/usage.c
index 17f52c1b5c..4270b04bf9 100644
--- a/usage.c
+++ b/usage.c
@@ -24,26 +24,26 @@ void vreportf(const char *prefix, const char *err, va_list params)
 	fputc('\n', fh);
 }
 
-static NORETURN void usage_builtin(const char *err, va_list params)
+static NORETURN void usage_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("usage: ", err, params);
+	vreportf(prefix, err, params);
 	exit(129);
 }
 
-static NORETURN void die_builtin(const char *err, va_list params)
+static NORETURN void die_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf(prefix, err, params);
 	exit(128);
 }
 
-static void error_builtin(const char *err, va_list params)
+static void error_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("error: ", err, params);
+	vreportf(prefix, err, params);
 }
 
-static void warn_builtin(const char *warn, va_list params)
+static void warn_builtin(const char *prefix, const char *warn, va_list params)
 {
-	vreportf("warning: ", warn, params);
+	vreportf(prefix, warn, params);
 }
 
 static int die_is_recursing_builtin(void)
@@ -54,33 +54,33 @@ static int die_is_recursing_builtin(void)
 
 /* If we are in a dlopen()ed .so write to a global variable would segfault
  * (ugh), so keep things static. */
-static NORETURN_PTR void (*usage_routine)(const char *err, va_list params) = usage_builtin;
-static NORETURN_PTR void (*die_routine)(const char *err, va_list params) = die_builtin;
-static void (*error_routine)(const char *err, va_list params) = error_builtin;
-static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
+static NORETURN_PTR void (*usage_routine)(const char *prefix, const char *err, va_list params) = usage_builtin;
+static NORETURN_PTR void (*die_routine)(const char *prefix, const char *err, va_list params) = die_builtin;
+static void (*error_routine)(const char *prefix, const char *err, va_list params) = error_builtin;
+static void (*warn_routine)(const char *prefix, const char *err, va_list params) = warn_builtin;
 static int (*die_is_recursing)(void) = die_is_recursing_builtin;
 
-void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params))
+void set_die_routine(NORETURN_PTR void (*routine)(const char *prefix, const char *err, va_list params))
 {
 	die_routine = routine;
 }
 
-void set_error_routine(void (*routine)(const char *err, va_list params))
+void set_error_routine(void (*routine)(const char *prefix, const char *err, va_list params))
 {
 	error_routine = routine;
 }
 
-void (*get_error_routine(void))(const char *err, va_list params)
+void (*get_error_routine(void))(const char *prefix, const char *err, va_list params)
 {
 	return error_routine;
 }
 
-void set_warn_routine(void (*routine)(const char *warn, va_list params))
+void set_warn_routine(void (*routine)(const char *prefix, const char *warn, va_list params))
 {
 	warn_routine = routine;
 }
 
-void (*get_warn_routine(void))(const char *warn, va_list params)
+void (*get_warn_routine(void))(const char *prefix, const char *warn, va_list params)
 {
 	return warn_routine;
 }
@@ -101,7 +101,7 @@ void NORETURN usagef(const char *err, ...)
 	va_list params;
 
 	va_start(params, err);
-	usage_routine(err, params);
+	usage_routine("usage: ", err, params);
 	va_end(params);
 }
 
@@ -120,7 +120,7 @@ void NORETURN die(const char *err, ...)
 	}
 
 	va_start(params, err);
-	die_routine(err, params);
+	die_routine("fatal: ", err, params);
 	va_end(params);
 }
 
@@ -159,7 +159,7 @@ void NORETURN die_errno(const char *fmt, ...)
 	}
 
 	va_start(params, fmt);
-	die_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+	die_routine("fatal: ", fmt_with_err(buf, sizeof(buf), fmt), params);
 	va_end(params);
 }
 
@@ -170,7 +170,7 @@ int error_errno(const char *fmt, ...)
 	va_list params;
 
 	va_start(params, fmt);
-	error_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+	error_routine("error: ", fmt_with_err(buf, sizeof(buf), fmt), params);
 	va_end(params);
 	return -1;
 }
@@ -181,7 +181,7 @@ int error(const char *err, ...)
 	va_list params;
 
 	va_start(params, err);
-	error_routine(err, params);
+	error_routine("error: ", err, params);
 	va_end(params);
 	return -1;
 }
@@ -192,7 +192,7 @@ void warning_errno(const char *warn, ...)
 	va_list params;
 
 	va_start(params, warn);
-	warn_routine(fmt_with_err(buf, sizeof(buf), warn), params);
+	warn_routine("warning: ", fmt_with_err(buf, sizeof(buf), warn), params);
 	va_end(params);
 }
 
@@ -201,6 +201,6 @@ void warning(const char *warn, ...)
 	va_list params;
 
 	va_start(params, warn);
-	warn_routine(warn, params);
+	warn_routine("warning: ", warn, params);
 	va_end(params);
 }
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 0/5] Localise error headers
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git

Currently, the headers "error: ", "warning: " etc. - generated by die(),
warning() etc. - are not localized, but we feed many localized error messages
into these functions so that we produce error messages with mixed localisation.

This series introduces variants of die() etc. that use localised variants of
the headers, i.e. _("error: ") etc., and are to be fed localized messages. So,
instead of die(_("not workee")), which would produce a mixed localisation (such
as "error: geht ned"), one should use die_(_("not workee")) (resulting in
"Fehler: geht ned").

In this implementation, the gettext call for the header and the body are done
in different places (error function vs. caller) but this call pattern seems to
be the easiest variant for the caller, because the message body has to be marked
for localisation in any case, and N_() requires more letters than _(), an extra
argument to die() etc. even more than the extra "_" in the function name.

1/5 prepares the error machinery
2/5 provides new variants error_() etc.
3/5 has coccinelli rules error(_(E)) -> error_(_(E)) etc.
4/5 applies the coccinelli patches

5/5 is not to be applied to the main tree, but helps you try out the feature:
it has changes to de.po and git.pot so that e.g. "git branch" has fully localised
error messages (see the recipe in the commit message).

Michael J Gruber (5):
  error/warn framework: prepare for l10n
  error/warn framework: provide localized variants
  error/warn framework framework: coccinelli rules
  error/warn framework: localize
  WIP: Feature demonstration

 advice.c                           |   16 +-
 apply.c                            |   10 +-
 apply.h                            |    4 +-
 archive.c                          |    6 +-
 attr.c                             |    3 +-
 bisect.c                           |    2 +-
 branch.c                           |    4 +-
 builtin/add.c                      |   20 +-
 builtin/am.c                       |   27 +-
 builtin/archive.c                  |   10 +-
 builtin/blame.c                    |   12 +-
 builtin/branch.c                   |   45 +-
 builtin/bundle.c                   |    4 +-
 builtin/check-ignore.c             |   14 +-
 builtin/check-mailmap.c            |    2 +-
 builtin/checkout-index.c           |    2 +-
 builtin/checkout.c                 |   27 +-
 builtin/clean.c                    |   10 +-
 builtin/clone.c                    |   39 +-
 builtin/column.c                   |    2 +-
 builtin/commit.c                   |   87 +-
 builtin/config.c                   |    2 +-
 builtin/describe.c                 |    6 +-
 builtin/diff.c                     |    2 +-
 builtin/fetch.c                    |   21 +-
 builtin/gc.c                       |    3 +-
 builtin/grep.c                     |   14 +-
 builtin/help.c                     |    4 +-
 builtin/index-pack.c               |   42 +-
 builtin/interpret-trailers.c       |    2 +-
 builtin/log.c                      |   48 +-
 builtin/merge-recursive.c          |    2 +-
 builtin/merge.c                    |   53 +-
 builtin/mv.c                       |    6 +-
 builtin/notes.c                    |   40 +-
 builtin/pack-objects.c             |    4 +-
 builtin/prune.c                    |    2 +-
 builtin/pull.c                     |   10 +-
 builtin/push.c                     |   22 +-
 builtin/remote.c                   |   10 +-
 builtin/repack.c                   |    4 +-
 builtin/replace.c                  |    2 +-
 builtin/reset.c                    |   10 +-
 builtin/rev-list.c                 |    2 +-
 builtin/rev-parse.c                |    2 +-
 builtin/revert.c                   |    4 +-
 builtin/rm.c                       |    6 +-
 builtin/shortlog.c                 |    2 +-
 builtin/show-branch.c              |    7 +-
 builtin/submodule--helper.c        |    9 +-
 builtin/tag.c                      |   20 +-
 builtin/unpack-objects.c           |    2 +-
 builtin/update-index.c             |    8 +-
 builtin/worktree.c                 |    6 +-
 bundle.c                           |    4 +-
 config.c                           |    4 +-
 connect.c                          |    6 +-
 connected.c                        |    2 +-
 contrib/coccinelle/errorl10n.cocci |   47 +
 daemon.c                           |    3 +-
 diff.c                             |    8 +-
 dir.c                              |    4 +-
 fast-import.c                      |    4 +-
 fetch-pack.c                       |   26 +-
 git-compat-util.h                  |   18 +-
 help.c                             |    2 +-
 http-backend.c                     |    4 +-
 http.c                             |    4 +-
 merge.c                            |    2 +-
 notes-utils.c                      |    2 +-
 pathspec.c                         |   13 +-
 po/de.po                           | 2922 ++++++++++++++++++++++++------------
 po/git.pot                         | 2781 ++++++++++++++++++++++------------
 pretty.c                           |    2 +-
 ref-filter.c                       |   20 +-
 remote.c                           |    2 +-
 revision.c                         |    4 +-
 run-command.c                      |    6 +-
 send-pack.c                        |   12 +-
 sequencer.c                        |    7 +-
 setup.c                            |    6 +-
 sha1_file.c                        |    2 +-
 submodule.c                        |    8 +-
 trailer.c                          |    4 +-
 transport.c                        |    2 +-
 tree-walk.c                        |    2 +-
 usage.c                            |  122 +-
 wrapper.c                          |    2 +-
 wt-status.c                        |    2 +-
 89 files changed, 4408 insertions(+), 2374 deletions(-)
 create mode 100644 contrib/coccinelle/errorl10n.cocci

-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply

* Re: Error: could not fork child process: Resource temporarily unavailable (-1)
From: Johannes Schindelin @ 2017-01-02 10:58 UTC (permalink / raw)
  To: M. Abuhena Sobuj; +Cc: git
In-Reply-To: <CACkJq_gQQURdyUf5HvSqvw0Ee_yjCnFZ0DXJxZt42JOwEOd2tg@mail.gmail.com>

Hi,

On Mon, 2 Jan 2017, M. Abuhena Sobuj wrote:

> My Git Bash was perfect but suddenly It stooped work for me.
> 
> I just got message
> "Error: could not fork child process: Resource temporarily unavailable (-1).
> DLL rebasing may be required. See 'rebaseall / rebase --help'."

Is this a 32-bit system? If so, simply try to reinstall Git for Windows,
that should make it work again [*1*].

If that does not solve the problem, you will want to open a ticket at
https://github.com/git-for-windows/git/issues/new

Ciao,
Johannes

Footnote *1*: https://github.com/git-for-windows/git/wiki/32-bit-issues

^ permalink raw reply


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