From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Elijah Newren <newren@gmail.com>
Subject: [PATCH 00/19] Cleanup merge API
Date: Thu, 25 Jul 2019 10:45:52 -0700 [thread overview]
Message-ID: <20190725174611.14802-1-newren@gmail.com> (raw)
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
next reply other threads:[~2019-07-25 17:46 UTC|newest]
Thread overview: 157+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 17:45 Elijah Newren [this message]
2019-07-25 17:45 ` [PATCH 01/19] merge-recursive: fix minor memory leak in error condition Elijah Newren
2019-07-25 17:45 ` [PATCH 02/19] merge-recursive: remove another implicit dependency on the_repository Elijah Newren
2019-07-25 17:45 ` [PATCH 03/19] Ensure index matches head before invoking merge machinery, round N Elijah Newren
2019-07-25 19:41 ` Johannes Schindelin
2019-07-25 19:58 ` Elijah Newren
2019-07-26 11:23 ` Johannes Schindelin
2019-07-25 17:45 ` [PATCH 04/19] merge-recursive: exit early if index != head Elijah Newren
2019-07-25 19:51 ` Johannes Schindelin
2019-07-25 20:12 ` Elijah Newren
2019-07-25 17:45 ` [PATCH 05/19] merge-recursive: don't force external callers to do our logging Elijah Newren
2019-07-25 17:45 ` [PATCH 06/19] Change call signature of write_tree_from_memory() Elijah Newren
2019-07-25 19:55 ` Johannes Schindelin
2019-07-26 4:58 ` Elijah Newren
2019-07-25 17:45 ` [PATCH 07/19] Move write_tree_from_memory() from merge-recursive to cache-tree Elijah Newren
2019-07-25 17:46 ` [PATCH 08/19] merge-recursive: fix some overly long lines Elijah Newren
2019-07-25 17:46 ` [PATCH 09/19] merge-recursive: use common name for ancestors/common/base_list Elijah Newren
2019-07-25 17:46 ` [PATCH 10/19] merge-recursive: rename 'mrtree' to 'result_tree', for clarity Elijah Newren
2019-07-25 20:02 ` Johannes Schindelin
2019-07-25 17:46 ` [PATCH 11/19] merge-recursive: rename merge_options argument to opt in header Elijah Newren
2019-07-25 17:46 ` [PATCH 12/19] merge-recursive: move some definitions around to clean up the header Elijah Newren
2019-07-25 17:46 ` [PATCH 13/19] merge-recursive: consolidate unnecessary fields in merge_options Elijah Newren
2019-07-25 17:46 ` [PATCH 14/19] merge-recursive: comment and reorder the merge_options fields Elijah Newren
2019-07-25 17:46 ` [PATCH 15/19] merge-recursive: split internal fields into a separate struct Elijah Newren
2019-07-25 20:12 ` Johannes Schindelin
2019-07-25 20:27 ` Elijah Newren
2019-07-26 11:25 ` Johannes Schindelin
2019-07-26 15:30 ` Elijah Newren
2019-07-25 17:46 ` [PATCH 16/19] merge-recursive: alphabetize include list Elijah Newren
2019-07-25 17:46 ` [PATCH 17/19] merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* Elijah Newren
2019-07-25 17:46 ` [PATCH 18/19] merge-recursive: be consistent with assert Elijah Newren
2019-07-25 17:46 ` [PATCH 19/19] merge-recursive: provide a better label for diff3 common ancestor Elijah Newren
2019-07-25 20:28 ` Johannes Schindelin
2019-07-25 18:12 ` [PATCH 00/19] Cleanup merge API Junio C Hamano
2019-07-25 19:06 ` Elijah Newren
2019-07-25 22:50 ` Junio C Hamano
2019-07-26 14:07 ` Johannes Schindelin
2019-07-25 19:15 ` Johannes Schindelin
2019-07-26 15:15 ` Elijah Newren
2019-07-26 15:52 ` [PATCH v2 00/20] " Elijah Newren
2019-07-26 15:52 ` [PATCH v2 01/20] merge-recursive: fix minor memory leak in error condition Elijah Newren
2019-07-26 18:31 ` Junio C Hamano
2019-07-26 23:19 ` Elijah Newren
2019-07-29 14:13 ` Derrick Stolee
2019-07-26 15:52 ` [PATCH v2 02/20] merge-recursive: remove another implicit dependency on the_repository Elijah Newren
2019-07-26 18:42 ` Junio C Hamano
2019-07-26 15:52 ` [PATCH v2 03/20] Ensure index matches head before invoking merge machinery, round N Elijah Newren
2019-07-26 19:14 ` Junio C Hamano
2019-07-26 23:22 ` Elijah Newren
2019-07-26 15:52 ` [PATCH v2 04/20] merge-recursive: exit early if index != head Elijah Newren
2019-07-26 19:32 ` Junio C Hamano
2019-07-26 23:26 ` Elijah Newren
2019-07-29 15:56 ` Junio C Hamano
2019-07-26 15:52 ` [PATCH v2 05/20] merge-recursive: remove useless parameter in merge_trees() Elijah Newren
2019-07-26 19:37 ` Junio C Hamano
2019-07-26 15:52 ` [PATCH v2 06/20] merge-recursive: don't force external callers to do our logging Elijah Newren
2019-07-26 19:57 ` Junio C Hamano
2019-07-26 23:28 ` Elijah Newren
2019-07-26 15:52 ` [PATCH v2 07/20] Use write_index_as_tree() in lieu of write_tree_from_memory() Elijah Newren
2019-07-26 20:11 ` Junio C Hamano
2019-07-26 23:39 ` Elijah Newren
2019-07-29 4:46 ` Junio C Hamano
2019-07-26 15:52 ` [PATCH v2 08/20] merge-recursive: fix some overly long lines Elijah Newren
2019-07-26 15:52 ` [PATCH v2 09/20] merge-recursive: use common name for ancestors/common/base_list Elijah Newren
2019-07-26 15:52 ` [PATCH v2 10/20] merge-recursive: rename 'mrtree' to 'result_tree', for clarity Elijah Newren
2019-07-26 15:52 ` [PATCH v2 11/20] merge-recursive: rename merge_options argument to opt in header Elijah Newren
2019-07-26 15:52 ` [PATCH v2 12/20] merge-recursive: move some definitions around to clean up the header Elijah Newren
2019-07-26 15:52 ` [PATCH v2 13/20] merge-recursive: consolidate unnecessary fields in merge_options Elijah Newren
2019-07-26 15:52 ` [PATCH v2 14/20] merge-recursive: comment and reorder the merge_options fields Elijah Newren
2019-07-26 15:52 ` [PATCH v2 15/20] merge-recursive: avoid losing output and leaking memory holding that output Elijah Newren
2019-07-26 15:52 ` [PATCH v2 16/20] merge-recursive: split internal fields into a separate struct Elijah Newren
2019-07-26 15:52 ` [PATCH v2 17/20] merge-recursive: alphabetize include list Elijah Newren
2019-07-26 15:52 ` [PATCH v2 18/20] merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* Elijah Newren
2019-07-26 15:52 ` [PATCH v2 19/20] merge-recursive: be consistent with assert Elijah Newren
2019-07-26 15:52 ` [PATCH v2 20/20] merge-recursive: provide a better label for diff3 common ancestor Elijah Newren
2019-08-15 21:40 ` [PATCH v3 00/24] Clean up merge API Elijah Newren
2019-08-15 21:40 ` [PATCH v3 01/24] merge-recursive: be consistent with assert Elijah Newren
2019-08-15 21:40 ` [PATCH v3 02/24] checkout: provide better conflict hunk description with detached HEAD Elijah Newren
2019-08-15 21:40 ` [PATCH v3 03/24] merge-recursive: enforce opt->ancestor != NULL when calling merge_trees() Elijah Newren
2019-08-16 21:05 ` Junio C Hamano
2019-08-15 21:40 ` [PATCH v3 04/24] merge-recursive: provide a better label for diff3 common ancestor Elijah Newren
2019-08-16 21:33 ` Junio C Hamano
2019-08-16 22:39 ` Elijah Newren
2019-08-15 21:40 ` [PATCH v3 05/24] merge-recursive: introduce an enum for detect_directory_renames values Elijah Newren
2019-08-15 21:40 ` [PATCH v3 06/24] merge-recursive: future-proof update_file_flags() against memory leaks Elijah Newren
2019-08-15 21:40 ` [PATCH v3 07/24] merge-recursive: remove another implicit dependency on the_repository Elijah Newren
2019-08-15 21:40 ` [PATCH v3 08/24] Ensure index matches head before invoking merge machinery, round N Elijah Newren
2019-08-15 21:40 ` [PATCH v3 09/24] merge-recursive: exit early if index != head Elijah Newren
2019-08-15 21:40 ` [PATCH v3 10/24] merge-recursive: remove useless parameter in merge_trees() Elijah Newren
2019-08-15 21:40 ` [PATCH v3 11/24] merge-recursive: don't force external callers to do our logging Elijah Newren
2019-08-15 21:40 ` [PATCH v3 12/24] cache-tree: share code between functions writing an index as a tree Elijah Newren
2019-08-16 22:01 ` Junio C Hamano
2019-08-16 22:39 ` Elijah Newren
2019-08-15 21:40 ` [PATCH v3 13/24] merge-recursive: fix some overly long lines Elijah Newren
2019-08-15 21:40 ` [PATCH v3 14/24] merge-recursive: use common name for ancestors/common/base_list Elijah Newren
2019-08-15 21:40 ` [PATCH v3 15/24] merge-recursive: rename 'mrtree' to 'result_tree', for clarity Elijah Newren
2019-08-15 21:40 ` [PATCH v3 16/24] merge-recursive: rename merge_options argument to opt in header Elijah Newren
2019-08-15 21:40 ` [PATCH v3 17/24] merge-recursive: move some definitions around to clean up the header Elijah Newren
2019-08-15 21:40 ` [PATCH v3 18/24] merge-recursive: consolidate unnecessary fields in merge_options Elijah Newren
2019-08-16 22:14 ` Junio C Hamano
2019-08-16 22:59 ` Elijah Newren
2019-08-16 23:24 ` Junio C Hamano
2019-08-15 21:40 ` [PATCH v3 19/24] merge-recursive: comment and reorder the merge_options fields Elijah Newren
2019-08-15 21:40 ` [PATCH v3 20/24] merge-recursive: avoid losing output and leaking memory holding that output Elijah Newren
2019-08-15 21:40 ` [PATCH v3 21/24] merge-recursive: split internal fields into a separate struct Elijah Newren
2019-08-16 21:19 ` SZEDER Gábor
2019-08-16 23:00 ` Elijah Newren
2019-08-16 22:17 ` Junio C Hamano
2019-08-15 21:40 ` [PATCH v3 22/24] merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* Elijah Newren
2019-08-15 21:40 ` [PATCH v3 23/24] merge-recursive: add sanity checks for relevant merge_options Elijah Newren
2019-08-16 19:52 ` Junio C Hamano
2019-08-16 22:08 ` Elijah Newren
2019-08-16 23:15 ` Junio C Hamano
2019-08-16 19:59 ` Junio C Hamano
2019-08-16 22:09 ` Elijah Newren
2019-08-15 21:40 ` [PATCH v3 24/24] merge-recursive: alphabetize include list Elijah Newren
2019-08-17 18:41 ` [PATCH v4 00/24] Clean up merge API Elijah Newren
2019-08-17 18:41 ` [PATCH v4 01/24] merge-recursive: be consistent with assert Elijah Newren
2019-08-17 18:41 ` [PATCH v4 02/24] checkout: provide better conflict hunk description with detached HEAD Elijah Newren
2019-08-17 18:41 ` [PATCH v4 03/24] merge-recursive: enforce opt->ancestor != NULL when calling merge_trees() Elijah Newren
2019-08-17 18:41 ` [PATCH v4 04/24] merge-recursive: provide a better label for diff3 common ancestor Elijah Newren
2019-09-30 21:14 ` Jeff King
2019-09-30 21:19 ` Jeff King
2019-09-30 22:54 ` [PATCH] merge-recursive: fix the diff3 common ancestor label for virtual commits Elijah Newren
2019-10-01 6:56 ` Elijah Newren
2019-10-01 6:58 ` [PATCH v2] " Elijah Newren
2019-10-01 14:49 ` Jeff King
2019-10-01 18:17 ` [PATCH v3] " Elijah Newren
2019-10-07 2:51 ` Junio C Hamano
2019-10-07 15:52 ` [PATCH] merge-recursive: fix the fix to the diff3 common ancestor label Elijah Newren
2019-10-08 2:32 ` Junio C Hamano
2019-10-08 2:36 ` Junio C Hamano
2019-10-08 16:16 ` Elijah Newren
2019-08-17 18:41 ` [PATCH v4 05/24] merge-recursive: introduce an enum for detect_directory_renames values Elijah Newren
2019-08-17 18:41 ` [PATCH v4 06/24] merge-recursive: future-proof update_file_flags() against memory leaks Elijah Newren
2019-08-17 18:41 ` [PATCH v4 07/24] merge-recursive: remove another implicit dependency on the_repository Elijah Newren
2019-08-17 18:41 ` [PATCH v4 08/24] Ensure index matches head before invoking merge machinery, round N Elijah Newren
2019-09-02 23:01 ` Johannes Schindelin
2019-09-03 13:34 ` Johannes Schindelin
2019-09-03 18:17 ` Elijah Newren
2019-08-17 18:41 ` [PATCH v4 09/24] merge-recursive: exit early if index != head Elijah Newren
2019-08-17 18:41 ` [PATCH v4 10/24] merge-recursive: remove useless parameter in merge_trees() Elijah Newren
2019-08-17 18:41 ` [PATCH v4 11/24] merge-recursive: don't force external callers to do our logging Elijah Newren
2019-08-17 18:41 ` [PATCH v4 12/24] cache-tree: share code between functions writing an index as a tree Elijah Newren
2019-08-17 18:41 ` [PATCH v4 13/24] merge-recursive: fix some overly long lines Elijah Newren
2019-08-17 18:41 ` [PATCH v4 14/24] merge-recursive: use common name for ancestors/common/base_list Elijah Newren
2019-08-17 18:41 ` [PATCH v4 15/24] merge-recursive: rename 'mrtree' to 'result_tree', for clarity Elijah Newren
2019-08-17 18:41 ` [PATCH v4 16/24] merge-recursive: rename merge_options argument to opt in header Elijah Newren
2019-08-17 18:41 ` [PATCH v4 17/24] merge-recursive: move some definitions around to clean up the header Elijah Newren
2019-08-17 18:41 ` [PATCH v4 18/24] merge-recursive: consolidate unnecessary fields in merge_options Elijah Newren
2019-08-17 18:41 ` [PATCH v4 19/24] merge-recursive: comment and reorder the merge_options fields Elijah Newren
2019-08-17 18:41 ` [PATCH v4 20/24] merge-recursive: avoid losing output and leaking memory holding that output Elijah Newren
2019-08-17 18:41 ` [PATCH v4 21/24] merge-recursive: split internal fields into a separate struct Elijah Newren
2019-08-19 17:17 ` Junio C Hamano
2019-08-17 18:41 ` [PATCH v4 22/24] merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* Elijah Newren
2019-08-17 18:41 ` [PATCH v4 23/24] merge-recursive: add sanity checks for relevant merge_options Elijah Newren
2019-08-17 18:41 ` [PATCH v4 24/24] merge-recursive: alphabetize include list Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190725174611.14802-1-newren@gmail.com \
--to=newren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).