mbox series

[v2,00/15] Switch directory rename detection default

Message ID 20190330003336.21940-1-newren@gmail.com (mailing list archive)
Headers show
Series Switch directory rename detection default | expand

Message

Elijah Newren March 30, 2019, 12:33 a.m. UTC
I know this is kinda big, but it's mostly simple mechanical cleanups.
Stuff I'd like reviewers to focus on:
  * Patch 15, particularly looking over the new testcases (13a-13d) in
    t6043 and the documentation.
  * Should I have switched 'unsigned short' to 'unsigned' instead of
    vice-versa in patch 1?
  * Similarly, does anyone have a reason to prefer oid,mode pair over
    using a diff_filespec as I did in patch 11?

The crux of this series is in patch 15, changing the default for
directory rename detection to treat it as a conflict.  It is similar
to the patch Junio previously reviewed; changes:

  * It now also handles files renamed into directories on one side
    where the other side renamed the directory away.  (Previously, it
    only handled new files added into such directories.)
    
  * As suggested by Junio, even if merge.directoryRenames=true is set
    in config (to accept directory rename heuristics as correct
    without reporting conflicts), messages will now be printed about
    the paths being changed by directory renames.
    
  * Error and info messages updated based in part on Junio's
    suggestions, a second check that the messages make sense would be
    useful.
  
  * There are now testcases testing the conflict and information
    message output (testcases 13[abcd] in t6043)


The reason this series exploded from one patch to fifteen was the need
to handle transitive renames (renames into directories on one side
while the other side renamed that directory away).  For that, I needed
to plumb more information through...but quickly got frustrated with
various ugly code bits and went down a rabbit hole.

The first 12 patches are solely cleanups

Patches 13 & 14 are simple changes to make additional information
available.

Patch 15 is primary purpose of the series, described above.


Elijah Newren (15):
  Use 'unsigned short' for mode, like diff_filespec does
  merge-recursive: rename merge_options argument from 'o' to 'opt'
  merge-recursive: rename diff_filespec 'one' to 'o'
  merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf'
  merge-recursive: use 'ci' for rename_conflict_info variable name
  merge-recursive: move some struct declarations together
  merge-recursive: shrink rename_conflict_info
  merge-recursive: remove ren[12]_other fields from rename_conflict_info
  merge-recursive: track branch where rename occurred in rename struct
  merge-recursive: cleanup handle_rename_* function signatures
  merge-recursive: switch from (oid,mode) pairs to a diff_filespec
  t6043: fix copied test description to match its purpose
  merge-recursive: track information associated with directory renames
  merge-recursive: give callers of handle_content_merge() access to
    contents
  merge-recursive: switch directory rename detection default

 Documentation/config/merge.txt         |   19 +-
 archive.c                              |    2 +-
 blame.c                                |    2 +-
 blame.h                                |    2 +-
 builtin/rm.c                           |    2 +-
 builtin/update-index.c                 |    2 +-
 cache.h                                |    2 +-
 fsck.c                                 |    2 +-
 line-log.c                             |    2 +-
 match-trees.c                          |    8 +-
 merge-recursive.c                      | 1853 ++++++++++++------------
 notes.c                                |    2 +-
 sha1-name.c                            |    2 +-
 t/t3401-rebase-and-am-rename.sh        |    8 +-
 t/t6043-merge-rename-directories.sh    |  462 +++++-
 t/t6046-merge-skip-unneeded-updates.sh |    8 +-
 tree-diff.c                            |    2 +-
 tree-walk.c                            |    6 +-
 tree-walk.h                            |    6 +-
 19 files changed, 1367 insertions(+), 1025 deletions(-)