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

2016-10-17 Thread Junio C Hamano
Johannes Schindelin  writes:

> 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 difference between the end results of v3 and v4 looked OK
(except the 08/25 "strip LF" change that is unneeded) to me, so I'll
skip the early part of the series from my review that correspond to
the ones in v3 that I've already reviewed and found good, and
continue from the later ones.

Thanks.


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

2016-10-14 Thread Johannes Schindelin
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/seque