Re: [PATCH v3 03/15] merge-tree: add option parsing and initial shell for real merge function
From: Emily Shaffer <hidden>
Date: 2022-02-07 22:41:25
On Wed, Feb 02, 2022 at 07:34:29AM +0000, Elijah Newren via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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