Before writing a replacement merge strategy for recursive, I decided
to first cleanup the merge API -- streamlining merge-recursive.h and
making it more readable. It includes some fixes I noticed along the
way, and the last two patches were some forgotten changes of mine I
rediscovered that had minor textual conflicts before I rebased them on
this series.
While there are minor textual and semantic dependencies between
these patches (preventing me from splitting up this series), they
are logically separate and can be reviewed independently.
Stuff I'd most welcome review on:
* Is cache-tree.c the right place for write_tree_from_memory()?
[see patch 7] Should there be docs on how it differs from
write_index_as_tree(), already found in cache-tree? What does
the latter even do?
Some notes:
* Applies on master, merges cleanly to next & pu
* Only patches 3, 5-7 touch anything outside of merge-recursive
* Patches 1 & 3 are bugfixes, 2 & 4 arguably are too, but all are
quite minor and do not need to be part of the impending release
candidates or 2.23.0.
* I'm going to be out next week (July 29-Aug 3), so I can only
respond to feedback for the next couple days or it'll have to
wait until the 5th.
Stuff I didn't address:
* merge_recursive() empties the commit_list of merge_bases it is
passed, so the caller is expected to NOT use it afterwards. Seems
suboptimal to place such an expectation on the caller.
* All current callers (3 of them?) of merge_recursive() always pass
it a specially created reversed-list for the merge_bases. Some
history spelunking provides no details on any of these about why;
it appears that the 2nd and 3rd callers reversed the list because
the first did, and I'm guessing the first did in an attempt to
exactly match the git-merge-recursive.py scripts' behavior. But
if the API needs them in a reverse order from what people would
normally expect to pass them in, shouldn't it reverse them itself
instead of making all callers do it? Also, the order shouldn't
matter when there are no conflicts, and when there are conflicts
it'd only change which side of the conflict markers the made-up
virtual merge base would list things in. However, we do have
tests with recursive virtual merge bases and which test the
output, and I didn't want to try to clean those all up. Besides,
the current order shows nicely when commits are named things like
"L1", "L2", "R1", "R2" -- it's nice having a well defined left and
right side. Wasn't sure what to do yet, so I just punted for now;
this series is already long enough...
Elijah Newren (19):
merge-recursive: fix minor memory leak in error condition
merge-recursive: remove another implicit dependency on the_repository
Ensure index matches head before invoking merge machinery, round N
merge-recursive: exit early if index != head
merge-recursive: don't force external callers to do our logging
Change call signature of write_tree_from_memory()
Move write_tree_from_memory() from merge-recursive to cache-tree
merge-recursive: fix some overly long lines
merge-recursive: use common name for ancestors/common/base_list
merge-recursive: rename 'mrtree' to 'result_tree', for clarity
merge-recursive: rename merge_options argument to opt in header
merge-recursive: move some definitions around to clean up the header
merge-recursive: consolidate unnecessary fields in merge_options
merge-recursive: comment and reorder the merge_options fields
merge-recursive: split internal fields into a separate struct
merge-recursive.c: alphabetize include list
merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*
merge-recursive: be consistent with assert
merge-recursive: provide a better label for diff3 common ancestor
builtin/checkout.c | 2 +-
builtin/merge-recursive.c | 4 +
builtin/stash.c | 2 +
cache-tree.c | 30 ++
cache-tree.h | 2 +
merge-recursive.c | 517 +++++++++++++++++-------------
merge-recursive.h | 119 +++----
sequencer.c | 1 -
t/t3030-merge-recursive.sh | 9 +-
t/t6036-recursive-corner-cases.sh | 8 +-
t/t6047-diff3-conflict-markers.sh | 191 +++++++++++
11 files changed, 589 insertions(+), 296 deletions(-)
create mode 100755 t/t6047-diff3-conflict-markers.sh
--
2.22.0.564.gb52f8ae349
