On Wed, Feb 02, 2022 at 07:34:29AM +0000, Elijah Newren via GitGitGadget wrote: > > > Let merge-tree accept a `--write-tree` parameter for choosing real > merges instead of trivial merges, and accept an optional > `--trivial-merge` option to get the traditional behavior. Note that > these accept different numbers of arguments, though, so these names > need not actually be used. > > Note that real merges differ from trivial merges in that they handle: > - three way content merges > - recursive ancestor consolidation > - renames > - proper directory/file conflict handling > - etc. > Basically all the stuff you'd expect from `git merge`, just without > updating the index and working tree. The initial shell added here does > nothing more than die with "real merges are not yet implemented", but > that will be fixed in subsequent commits. > > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > --- > builtin/merge-tree.c | 61 +++++++++++++++++++++++++++++++++++++------- > git.c | 2 +- > 2 files changed, 53 insertions(+), 10 deletions(-) > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > index 914ec960b7e..e98ec8a9f1d 100644 > --- a/builtin/merge-tree.c > +++ b/builtin/merge-tree.c > @@ -3,13 +3,12 @@ > #include "tree-walk.h" > #include "xdiff-interface.h" > #include "object-store.h" > +#include "parse-options.h" > #include "repository.h" > #include "blob.h" > #include "exec-cmd.h" > #include "merge-blobs.h" > > -static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>"; > - > struct merge_list { > struct merge_list *next; > struct merge_list *link; /* other stages for this object */ > @@ -366,15 +365,17 @@ static void *get_tree_descriptor(struct repository *r, > return buf; > } > > -static int trivial_merge(int argc, const char **argv) > +static int trivial_merge(const char *base, > + const char *branch1, > + const char *branch2) > { > struct repository *r = the_repository; > struct tree_desc t[3]; > void *buf1, *buf2, *buf3; > > - buf1 = get_tree_descriptor(r, t+0, argv[1]); > - buf2 = get_tree_descriptor(r, t+1, argv[2]); > - buf3 = get_tree_descriptor(r, t+2, argv[3]); > + buf1 = get_tree_descriptor(r, t+0, base); > + buf2 = get_tree_descriptor(r, t+1, branch1); > + buf3 = get_tree_descriptor(r, t+2, branch2); > trivial_merge_trees(t, ""); > free(buf1); > free(buf2); > @@ -384,9 +385,51 @@ static int trivial_merge(int argc, const char **argv) > return 0; > } > > +struct merge_tree_options { > + int mode; > +}; > + > +static int real_merge(struct merge_tree_options *o, > + const char *branch1, const char *branch2) > +{ > + die(_("real merges are not yet implemented")); > +} > + > int cmd_merge_tree(int argc, const char **argv, const char *prefix) > { > - if (argc != 4) > - usage(merge_tree_usage); > - return trivial_merge(argc, argv); > + struct merge_tree_options o = { 0 }; > + int expected_remaining_argc; > + > + const char * const merge_tree_usage[] = { > + N_("git merge-tree [--write-tree] <branch1> <branch2>"), > + N_("git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2>"), > + NULL > + }; > + struct option mt_options[] = { > + OPT_CMDMODE(0, "write-tree", &o.mode, > + N_("do a real merge instead of a trivial merge"), > + 'w'), > + OPT_CMDMODE(0, "trivial-merge", &o.mode, > + N_("do a trivial merge only"), 't'), > + OPT_END() > + }; In the review club last week I had mentioned I thought OPT_CMDMODE worked well with enums. I found some a reasonably nice example in builtin/replace.c:cmd_replace(), although I have some Opinions about the enum declaration placement there. Regardless, I think using an enum instead of a single character would make this more readable - otherwise I need to remember what 'w' means when I'm reasoning about how many args to expect below. > + > + /* Parse arguments */ > + argc = parse_options(argc, argv, prefix, mt_options, > + merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION); > + if (o.mode) { > + expected_remaining_argc = (o.mode == 'w' ? 2 : 3); > + if (argc != expected_remaining_argc) > + usage_with_options(merge_tree_usage, mt_options); > + } else { > + if (argc < 2 || argc > 3) > + usage_with_options(merge_tree_usage, mt_options); > + o.mode = (argc == 2 ? 'w' : 't'); > + } > + > + /* Do the relevant type of merge */ > + if (o.mode == 'w') > + return real_merge(&o, argv[0], argv[1]); > + else > + return trivial_merge(argv[0], argv[1], argv[2]); > } Sorry for the slow reply. - Emily