Skip to content

am-3-merge-recursive-direct-v2

This is the second iteration of the long-awaited re-roll of the attempt to
avoid spawning merge-recursive from the builtin am and use merge_recursive()
directly instead.

The *real* reason for the reroll is that I need a libified recursive
merge to accelerate the interactive rebase by teaching the sequencer to
do rebase -i's grunt work.

In this endeavor, we need to be extra careful to retain backwards
compatibility. The test script t6022-merge-rename.sh, for example, verifies
that `git pull` exits with status 128 in case of a fatal error. To that end,
we need to make sure that fatal errors are handled by existing (builtin)
users via exit(128) (or die(), which calls exit(128) at the end).  New users
(such as a builtin helper doing rebase -i's grunt work) may want to print
some helpful advice what happened and how to get out of this mess before
erroring out.

The changes relative to the first iteration of this patch series:

- a variable that could be used uninitialized is now initialized (thanks,
  Travis & clang!)

- several commit messages were touched up (and I hope y'all agree, improved)

- an unnecessary hunk was reverted (this was a left-over from an
  unpublished iteration that needed to retain return values faithfully, i.e.
  it made a difference between -1 and -128 as error value)

- Junio's patch to use recursive_merge() directly in the builtin am was
  replaced by a different solution

- an error message was identified as, and converted into, a bug report
  instead

- the code in was_tracked() now avoids a loop when it is unnecessary,
  and further clarifies why we keep looking when cache_name_pos() did
  not find the entry we asked for

- die("BUG: ...") statements are no longer translated

- one die("BUG: ...") report that continued in upper-case after the "BUG:"
  prefix was fixed

- I addressed a gender bias that has been bugging me ever since I noticed it

- recursive merge's error messages are now printed after flushing the
  output buffer (instead of swallowing that output)

- callers of the recursive merge can now ask that the output buffer not be
  flushed, but retained without printing it instead. This gives the caller the
  control about handling errors which Junio asked for.

- some long-standing bugs have been recognized and addressed:

  - when the recursive merge failed, it lost the buffered output

  - the output buffer of the recursive merge was never released

  - some stdout/stderr interference that we tried to address in 2007 is
    now fully addressed (the progress output could be printed in the
    middle of the commit title because the latter was still directly printed
    to stdout, which is buffered, instead of being buffered and flushed)

- a lot of unnecessary 'ret' variables are gone now: originally, I wanted to
  retain the *exact* return value, but now errors are indicated by -1,
  always

- lastly, I remembered that my original attempt at fixing the pull --rebase
  issue contained a test case, and I forward-ported that, and augmented it

So while I addressed all comments, I also went through the patch series a
couple of times myself and whatever bugged me, I tried to resolve, too.

This patch series touches rather important code. Now that I addressed
concerns such as fixing translated bug reports, I would appreciate thorough
reviews with a focus on the critical parts of the code, those that could
result in regressions.

Johannes Schindelin (17):
  Verify that `git pull --rebase` shows the helpful advice when failing
  Report bugs consistently
  Avoid translating bug messages
  merge-recursive: clarify code in was_tracked()
  Prepare the builtins for a libified merge_recursive()
  merge_recursive: abort properly upon errors
  merge-recursive: avoid returning a wholesale struct
  merge-recursive: allow write_tree_from_memory() to error out
  merge-recursive: handle return values indicating errors
  merge-recursive: switch to returning errors instead of dying
  am: counteract gender bias
  am -3: use merge_recursive() directly again
  merge-recursive: flush output buffer before printing error messages
  merge-recursive: write the commit title in one go
  merge-recursive: offer an option to retain the output in 'obuf'
  Ensure that the output buffer is released after calling merge_trees()
  merge-recursive: flush output buffer even when erroring out

 builtin/am.c           |  55 ++----
 builtin/checkout.c     |   5 +-
 builtin/ls-files.c     |   3 +-
 builtin/merge.c        |   2 +
 builtin/update-index.c |   2 +-
 grep.c                 |   8 +-
 imap-send.c            |   4 +-
 merge-recursive.c      | 490 +++++++++++++++++++++++++++++--------------------
 merge-recursive.h      |   2 +-
 sequencer.c            |   5 +
 sha1_file.c            |   4 +-
 t/t5520-pull.sh        |  30 +++
 trailer.c              |   2 +-
 transport.c            |   2 +-
 wt-status.c            |   4 +-
 15 files changed, 369 insertions(+), 249 deletions(-)

Submitted-As: http://mid.gmane.org/cover.1467717729.git.johannes.schindelin@gmx.de
In-Reply-To: http://mid.gmane.org/cover.1467199553.git.johannes.schindelin@gmx.de
Assets 2