From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>
Subject: [RFC/PATCH 00/18] Add --index-only option to git merge
Date: Thu, 7 Apr 2016 23:58:28 -0700 [thread overview]
Message-ID: <1460098726-5958-1-git-send-email-newren@gmail.com> (raw)
This patch series adds an --index-only flag to git merge, the idea
being to allow a merge to be performed entirely in the index without
touching (or even needing) a working tree.
The core fix, to merge-recursive, was actually quite easy. The
recursive merge logic already had the ability to ignore the working
directory and operate entirely on the index -- it needed to do this
when creating a virtual merge base, i.e. when o->call_depth > 0. It's
just that o->call_depth was also used for other purposes, so I just
needed to introduce a new flag to disambiguate and switch all the
necessary index-only-related call sites to use it. It actually seems
to make the code slightly easier to read too, which is a nice bonus.
That was all done in patch 12 of this series.
Adding all the necessary testcases and switching over the other merge
strategies turned out to be the harder part...and still has a problem,
as I'll mention below.
A brief-ish summary of the series:
* Patches 1 and 2 are unrelated cleanups, which could be submitted
independently. However, submitting them as separate patches would
result in a minor conflict, so I'm just including them together.
* Patches 3 and 4 add some testcases and a fix for a separate issue
found while testing this series which could be split off and
submitted separately, but fixing this problem and enforcing the
starting state I expected permitted me to reduce the range of
testcases I needed to consider for the --index-only merges. So I
thought it made sense to include in this series.
In particular, I was worried about how git merge behaved when the
index differed from HEAD prior to the merge starting. I discovered
that it wasn't just allowed for fast-forward merges (where behavior
is well-defined) but that it was also (accidentally I'm assuming)
allowed for octopus merges with weird/surprising behavior.
* Patches 5-10 add testcases for the --index-only option for each of
the merge strategies/cases: recursive, fast-forward update, resolve,
octopus, ours, subtree.
* Patch 11 adds the --index-only option to the code and adds the
documentation, while patches 12-18 implement the --index-only option
for each of the strategies/cases.
Some things I am concerned about:
* The option name (--index-only) may be slightly misleading since the
index isn't the only thing modified within the git directory, other
normal things under there are still modified (e.g. writing MERGE_*
files, sticking blobs/trees/commits in objects/*, and updating refs
and reflogs). I personally prefer this name and think the confusion
would be minor, but I'm a bit unsure and wanted some other opinions
on this.
* I didn't add this option to the separate git-merge-recursive
executable and make the worktree-optional modification to the git
subcommands merge-recursive, merge-recursive-ours,
merge-recursive-theirs, and merge-subtree in git.c. Should I, or
are these separate binaries and subcommands just present for
historical backward compatibility reasons with people expected to
call e.g. "git merge -s recursive" these days?
* The addition of --index-only to the various git-merge*.sh scripts is
lacking in flexibility (e.g. has to be passed as the first
argument). These scripts seemed to have fairly rigid calling
conventions already, suggesting there's not much value in making
them flexible, but perhaps that was the wrong conclusion to draw.
* Expanding on the last item, git-merge-one-file.sh is of particular
concern; it seemed to strongly assume exactly seven arguments and
that the position of each mattered. I didn't want to break that, so
I added --index-only as an optional 8th argument, even though it
seems slightly odd force an option argument to always come after
positional ones (and it made the changes to merge_entry in
merge-index.c slightly easier to implement). Does that seem okay?
* For a long time I had two remaining bugs, one of which was in
checkout_fast_forward. I was feeling kind of sheepish about that,
because how much simpler could it be than handling a fast-forward
merge (with possible uncommited index entries to carry forward)?
Getting stuck on a simple case like that would be embarrassing.
Luckily, I figured out that bug. So, that leaves just one case left
that I can't seem to figure out: read_tree_trivial. So much better,
right? Even it's name is sitting there, mocking me. "Ha ha, I'm
read_tree_*trivial* and you can't figure me out." read_tree_trivial
is a jerk.
Elijah Newren (18):
Remove duplicate code
Avoid checking working copy when creating a virtual merge base
Document weird bug in octopus merges via testcases
merge-octopus: Abort if index not clean
Add testcase for --index-only merges needing the recursive strategy
Add testcase for --index-only merges needing an ff update
Add testcase for --index-only merges with the resolve strategy
Add testcase for --index-only merges with the octopus strategy
Add testcase for --index-only merges with the ours strategy
Add testcase for --index-only merges with the subtree strategy
merge: Add a new --index-only option, not yet implemented
Add --index-only support for recursive merges
Add --index-only support with read_tree_trivial merges, kind of
Add --index-only support for ff_only merges
merge: Pass --index-only along to external merge strategy programs
git-merge-one-file.sh: support --index-only option
git-merge-resolve.sh: support --index-only option
git-merge-octopus.sh: support --index-only option
Documentation/git-merge.txt | 14 +
builtin/merge-index.c | 9 +-
builtin/merge.c | 13 +-
builtin/pull.c | 4 +-
cache.h | 3 +-
git-merge-octopus.sh | 19 +-
git-merge-one-file.sh | 44 ++-
git-merge-resolve.sh | 12 +-
git.c | 2 +-
merge-recursive.c | 40 +--
merge-recursive.h | 1 +
merge.c | 8 +-
sequencer.c | 4 +-
t/t6043-merge-index-only.sh | 443 +++++++++++++++++++++++++++++++
t/t6044-merge-unrelated-index-changes.sh | 156 +++++++++++
15 files changed, 730 insertions(+), 42 deletions(-)
create mode 100755 t/t6043-merge-index-only.sh
create mode 100755 t/t6044-merge-unrelated-index-changes.sh
--
2.8.0.18.gc685494
next reply other threads:[~2016-04-08 6:59 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-08 6:58 Elijah Newren [this message]
2016-04-08 6:58 ` [RFC/PATCH 01/18] Remove duplicate code Elijah Newren
2016-04-08 23:34 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 02/18] Avoid checking working copy when creating a virtual merge base Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 03/18] Document weird bug in octopus merges via testcases Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 04/18] merge-octopus: Abort if index not clean Elijah Newren
2016-04-08 19:31 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy Elijah Newren
2016-04-08 19:37 ` Junio C Hamano
2016-04-08 20:14 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 06/18] Add testcase for --index-only merges needing an ff update Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 07/18] Add testcase for --index-only merges with the resolve strategy Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 08/18] Add testcase for --index-only merges with the octopus strategy Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 09/18] Add testcase for --index-only merges with the ours strategy Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 10/18] Add testcase for --index-only merges with the subtree strategy Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented Elijah Newren
2016-04-08 22:33 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 12/18] Add --index-only support for recursive merges Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 13/18] Add --index-only support with read_tree_trivial merges, kind of Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 14/18] Add --index-only support for ff_only merges Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 15/18] merge: Pass --index-only along to external merge strategy programs Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 16/18] git-merge-one-file.sh: support --index-only option Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 17/18] git-merge-resolve.sh: " Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 18/18] git-merge-octopus.sh: " Elijah Newren
2016-04-08 13:01 ` [RFC/PATCH 00/18] Add --index-only option to git merge Michael J Gruber
2016-04-09 3:09 ` Elijah Newren
2016-04-08 18:08 ` Junio C Hamano
2016-04-09 2:35 ` Elijah Newren
2016-04-09 4:44 ` Junio C Hamano
2016-04-10 5:33 ` 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=1460098726-5958-1-git-send-email-newren@gmail.com \
--to=newren@gmail.com \
--cc=git@vger.kernel.org \
/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).