[PATCH v4 00/25] Prepare the sequencer for the upcoming rebase -i patches

[Johannes Schindelin <johannes.schindelin@gmx.de>]

This patch series marks the '4' in the countdown to speed up rebase -i
by implementing large parts in C (read: there will be three more patch
series after that before the full benefit hits git.git: sequencer-i,
rebase--helper and rebase-i-extra). It is based on the `libify-sequencer`
patch series I submitted earlier.

The patches in this series merely prepare the sequencer code for the
next patch series that actually teaches the sequencer to run rebase -i's
commands.

The reason to split these two patch series is simple: to keep them at a
sensible size.

The two patch series after that are much smaller: a two-patch "series"
that switches rebase -i to use the sequencer (except with --root or
--preserve-merges), and a couple of patches to move several pretty
expensive script processing steps to C (think: autosquash).

The end game of this patch series is a git-rebase--helper that makes
rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says
that even MacOSX and Linux benefit (4x and 3x, respectively).

I have been working on this since early February, whenever time allowed,
and it is time to put it into the users' hands. To that end, I already
integrated the whole shebang into Git for Windows 2.10.0 and 2.10.1
where it has been running without complaints (and some quite positive
feedback).

Changes vs v3:

- fixed TRANSLATORS: comment to help the tool extracting those comments.

- reordered the patch introducing the short_commit_name() function so it
  can be used in the patch revamping the todo parsing right away, as
  opposed to fixing up the find_unique_abbrev() ugliness in a later
  patch.

- backed out the write_message_gently() function of this patch series:
  it is not used by the end of this patch series and would therefore let
  the build fail with DEVELOPER=1. That function is now introduced as
  part of the patch in the sequencer-i series that adds support for the
  'edit' command.

- edited "skip CR/skip LF" to say "strip" instead.

- used xstrdup_or_null() where appropriate.

- abstracted out git_config_string_dup() instead of adding
  near-duplicate code.

- marked the append_new_todo() function as file-local, pointed out by
  Ramsay.

- made the "you have staged changes" advice prettier by moving it out of
  the run_git_commit() function, based on a suggestion by Hannes Sixt.


Johannes Schindelin (25):
  sequencer: use static initializers for replay_opts
  sequencer: use memoized sequencer directory path
  sequencer: avoid unnecessary indirection
  sequencer: future-proof remove_sequencer_state()
  sequencer: eventually release memory allocated for the option values
  sequencer: future-proof read_populate_todo()
  sequencer: refactor the code to obtain a short commit name
  sequencer: completely revamp the "todo" script parsing
  sequencer: strip CR from the todo script
  sequencer: avoid completely different messages for different actions
  sequencer: get rid of the subcommand field
  sequencer: remember the onelines when parsing the todo file
  sequencer: prepare for rebase -i's commit functionality
  sequencer: introduce a helper to read files written by scripts
  sequencer: allow editing the commit message on a case-by-case basis
  sequencer: support amending commits
  sequencer: support cleaning up commit messages
  sequencer: do not try to commit when there were merge conflicts
  sequencer: left-trim lines read from the script
  sequencer: refactor write_message()
  sequencer: remove overzealous assumption in rebase -i mode
  sequencer: mark action_name() for translation
  sequencer: quote filenames in error messages
  sequencer: start error messages consistently with lower case
  sequencer: mark all error messages for translation

 builtin/commit.c              |   2 +-
 builtin/revert.c              |  46 ++-
 sequencer.c                   | 679 ++++++++++++++++++++++++++++--------------
 sequencer.h                   |  23 +-
 t/t3501-revert-cherry-pick.sh |   2 +-
 5 files changed, 492 insertions(+), 260 deletions(-)


base-commit: 3cdd5d19178a54d2e51b5098d43b57571241d0ab
Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v4
Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v4

Interdiff vs v3:

 diff --git a/builtin/revert.c b/builtin/revert.c
 index 0a7b5f4..4ca5b51 100644
 --- a/builtin/revert.c
 +++ b/builtin/revert.c
 @@ -166,10 +166,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
  		usage_with_options(usage_str, options);
  
  	/* These option values will be free()d */
 -	if (opts->gpg_sign)
 -		opts->gpg_sign = xstrdup(opts->gpg_sign);
 -	if (opts->strategy)
 -		opts->strategy = xstrdup(opts->strategy);
 +	opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
 +	opts->strategy = xstrdup_or_null(opts->strategy);
  
  	if (cmd == 'q')
  		return sequencer_remove_state(opts);
 diff --git a/sequencer.c b/sequencer.c
 index 86d86ce..1cf70f7 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -265,12 +265,6 @@ static int write_message(struct strbuf *msgbuf, const char *filename)
  	return res;
  }
  
 -static int write_file_gently(const char *filename,
 -			     const char *text, int append_eol)
 -{
 -	return write_with_lock_file(filename, text, strlen(text), append_eol);
 -}
 -
  /*
   * Reads a file that was presumably written by a shell script, i.e.
   * with an end-of-line marker that needs to be stripped.
 @@ -489,6 +483,20 @@ static char **read_author_script(void)
  	return env;
  }
  
 +static const char staged_changes_advice[] =
 +N_("you have staged changes in your working tree\n"
 +"If these changes are meant to be squashed into the previous commit, run:\n"
 +"\n"
 +"  git commit --amend %s\n"
 +"\n"
 +"If they are meant to go into a new commit, run:\n"
 +"\n"
 +"  git commit %s\n"
 +"\n"
 +"In both cases, once you're done, continue with:\n"
 +"\n"
 +"  git rebase --continue\n");
 +
  /*
   * If we are cherry-pick, and if the merge did not result in
   * hand-editing, we will hit this commit and inherit the original
 @@ -515,18 +523,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
  		if (!env) {
  			const char *gpg_opt = gpg_sign_opt_quoted(opts);
  
 -			return error(_("you have staged changes in your "
 -				       "working tree. If these changes are "
 -				       "meant to be\n"
 -				       "squashed into the previous commit, "
 -				       "run:\n\n"
 -				       "  git commit --amend %s\n\n"
 -				       "If they are meant to go into a new "
 -				       "commit, run:\n\n"
 -				       "  git commit %s\n\n"
 -				       "In both cases, once you're done, "
 -				       "continue with:\n\n"
 -				       "  git rebase --continue\n"),
 +			return error(_(staged_changes_advice),
  				     gpg_opt, gpg_opt);
  		}
  	}
 @@ -702,10 +699,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
  		return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
  
  	if (parent && parse_commit(parent) < 0)
 -		/*
 -		 * TRANSLATORS: The first %s will be a "todo" command like
 -		 * "revert" or "pick", the second %s a SHA1.
 -		 */
 +		/* TRANSLATORS: The first %s will be a "todo" command like
 +		   "revert" or "pick", the second %s a SHA1. */
  		return error(_("%s: cannot parse parent commit %s"),
  			command_to_string(command),
  			oid_to_hex(&parent->object.oid));
 @@ -885,7 +880,7 @@ static void todo_list_release(struct todo_list *todo_list)
  	todo_list->nr = todo_list->alloc = 0;
  }
  
 -struct todo_item *append_new_todo(struct todo_list *todo_list)
 +static struct todo_item *append_new_todo(struct todo_list *todo_list)
  {
  	ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc);
  	return todo_list->items + todo_list->nr++;
 @@ -939,10 +934,10 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
  	for (i = 1; *p; i++, p = next_p) {
  		char *eol = strchrnul(p, '\n');
  
 -		next_p = *eol ? eol + 1 /* skip LF */ : eol;
 +		next_p = *eol ? eol + 1 /* strip LF */ : eol;
  
  		if (p != eol && eol[-1] == '\r')
 -			eol--; /* skip Carriage Return */
 +			eol--; /* strip Carriage Return */
  
  		item = append_new_todo(todo_list);
  		item->offset_in_buf = p - todo_list->buf.buf;
 @@ -994,6 +989,16 @@ static int read_populate_todo(struct todo_list *todo_list,
  	return 0;
  }
  
 +static int git_config_string_dup(char **dest,
 +				 const char *var, const char *value)
 +{
 +	if (!value)
 +		return config_error_nonbool(var);
 +	free(*dest);
 +	*dest = xstrdup(value);
 +	return 0;
 +}
 +
  static int populate_opts_cb(const char *key, const char *value, void *data)
  {
  	struct replay_opts *opts = data;
 @@ -1013,22 +1018,10 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
  		opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
  	else if (!strcmp(key, "options.mainline"))
  		opts->mainline = git_config_int(key, value);
 -	else if (!strcmp(key, "options.strategy")) {
 -		if (!value)
 -			config_error_nonbool(key);
 -		else {
 -			free(opts->strategy);
 -			opts->strategy = xstrdup(value);
 -		}
 -	}
 -	else if (!strcmp(key, "options.gpg-sign")) {
 -		if (!value)
 -			config_error_nonbool(key);
 -		else {
 -			free(opts->gpg_sign);
 -			opts->gpg_sign = xstrdup(value);
 -		}
 -	}
 +	else if (!strcmp(key, "options.strategy"))
 +		git_config_string_dup(&opts->strategy, key, value);
 +	else if (!strcmp(key, "options.gpg-sign"))
 +		git_config_string_dup(&opts->gpg_sign, key, value);
  	else if (!strcmp(key, "options.strategy-option")) {
  		ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
  		opts->xopts[opts->xopts_nr++] = xstrdup(value);

-- 
2.10.1.513.g00ef6dd