git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/4] merge-ort: barebones API of new merge strategy with empty implementation

From
Taylor Blau <me@ttaylorr.com>
Date
Oct 23, 2020, 18:12 UTC
Message-ID
<X5MdD5Ft/DNbCfQv@nand.local>
In-Reply-To
<54ef18182cf3a4b5d02889c139e495fa68588652.1603286555.git.gitgitgadget@gmail.com>
On Wed, Oct 21, 2020 at 01:22:32PM +0000, Elijah Newren via GitGitGadget wrote:
Show 62 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> This is the beginning of a new merge strategy.  While there are some API
> differences, and the implementation has some differences in behavior, it
> is essentially meant as an eventual drop-in replacement for
> merge-recursive.c.  However, it is being built to exist side-by-side
> with merge-recursive so that we have plenty of time to find out how
> those differences pan out in the real world while people can still fall
> back to merge-recursive.  (Also, I intend to avoid modifying
> merge-recursive during this process, to keep it stable.)
>
> The primary difference noticable here is that the updating of the
> working tree and index is not done simultaneously with the merge
> algorithm, but is a separate post-processing step.  The new API is
> designed so that one can do repeated merges (e.g. during a rebase or
> cherry-pick) and only update the index and working tree one time at the
> end instead of updating it with every intermediate result.  Also, one
> can perform a merge between two branches, neither of which match the
> index or the working tree, without clobbering the index or working tree.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Makefile    |  1 +
>  merge-ort.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  merge-ort.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
>  create mode 100644 merge-ort.c
>  create mode 100644 merge-ort.h
>
> diff --git a/Makefile b/Makefile
> index 95571ee3fc..088770c2ae 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -921,6 +921,7 @@ LIB_OBJS += mailmap.o
>  LIB_OBJS += match-trees.o
>  LIB_OBJS += mem-pool.o
>  LIB_OBJS += merge-blobs.o
> +LIB_OBJS += merge-ort.o
>  LIB_OBJS += merge-recursive.o
>  LIB_OBJS += merge.o
>  LIB_OBJS += mergesort.o
> diff --git a/merge-ort.c b/merge-ort.c
> new file mode 100644
> index 0000000000..5230364a8d
> --- /dev/null
> +++ b/merge-ort.c
> @@ -0,0 +1,52 @@
> +/*
> + * "Ostensibly Recursive's Twin" merge strategy, or "ort" for short.  Meant
> + * as a drop-in replacement for the "recursive" merge strategy, allowing one
> + * to replace
> + *
> + *   git merge [-s recursive]
> + *
> + * with
> + *
> + *   git merge -s ort
> + *
> + * Note: git's parser allows the space between '-s' and its argument to be
> + * missing.  (Should I have backronymed "ham", "alsa", "kip", "nap, "alvo",
> + * "cale", "peedy", or "ins" instead of "ort"?)
> + */

One thing that might be good here (and I realize that you sort of get to it in your next patch) is some example usage. Maybe it's good enough to say "see the $FILE_MODIFIED_BY_NEXT_PATCH for example usage", but it may be good to be a little bit clearer here.

Perhaps it may even be appropriate to add a Documentation/technical/api-merge-ort.txt or something (if you haven't already in one of your other branches...).

Show 63 quoted lines
> +
> +#include "cache.h"
> +#include "merge-ort.h"
> +
> +void merge_switch_to_result(struct merge_options *opt,
> +			    struct tree *head,
> +			    struct merge_result *result,
> +			    int update_worktree_and_index,
> +			    int display_update_msgs)
> +{
> +	die("Not yet implemented");
> +	merge_finalize(opt, result);
> +}
> +
> +void merge_finalize(struct merge_options *opt,
> +		    struct merge_result *result)
> +{
> +	die("Not yet implemented");
> +}
> +
> +void merge_inmemory_nonrecursive(struct merge_options *opt,
> +				 struct tree *merge_base,
> +				 struct tree *side1,
> +				 struct tree *side2,
> +				 struct merge_result *result)
> +{
> +	die("Not yet implemented");
> +}
> +
> +void merge_inmemory_recursive(struct merge_options *opt,
> +			      struct commit_list *merge_bases,
> +			      struct commit *side1,
> +			      struct commit *side2,
> +			      struct merge_result *result)
> +{
> +	die("Not yet implemented");
> +}
> diff --git a/merge-ort.h b/merge-ort.h
> new file mode 100644
> index 0000000000..9c655cd3ad
> --- /dev/null
> +++ b/merge-ort.h
> @@ -0,0 +1,49 @@
> +#ifndef MERGE_ORT_H
> +#define MERGE_ORT_H
> +
> +#include "merge-recursive.h"
> +
> +struct commit;
> +struct tree;
> +
> +struct merge_result {
> +	/* whether the merge is clean */
> +	int clean;
> +
> +	/* Result of merge.  If !clean, represents what would go in worktree */
> +	struct tree *tree;
> +
> +	/*
> +	 * Additional metadata used by merge_switch_to_result() or future calls
> +	 * to merge_inmemory_*().
> +	 */
> +	unsigned _;

I was a little surprised that '_' is allowed, since I can't think of any other variable that is called that (and a search with "git grep ' _;' -- **/*.h" confirms that this is the only one. It does compile with DEVELOPER=1, so...

Is this meant to be a flags-like? I believe you that it's necessary, but I wonder if it could be named more clearly, even though it's private.

Show 16 quoted lines
> +	void *priv;
> +};
> +
> +/* rename-detecting three-way merge, no recursion. */
> +void merge_inmemory_recursive(struct merge_options *opt,
> +			      struct commit_list *merge_bases,
> +			      struct commit *side1,
> +			      struct commit *side2,
> +			      struct merge_result *result);
> +
> +/* rename-detecting three-way merge with recursive ancestor consolidation. */
> +void merge_inmemory_nonrecursive(struct merge_options *opt,
> +				 struct tree *merge_base,
> +				 struct tree *side1,
> +				 struct tree *side2,
> +				 struct merge_result *result);

Good. This API looks sane to me, and contains nothing more nor less than I'd expect. I appreciate that the _recursive declaration is separate from the _nonrecursive one (and not hidden behind a flag that switches between the two).

Show 6 quoted lines
> +/* Update the working tree and index from head to result after inmemory merge */
> +void merge_switch_to_result(struct merge_options *opt,
> +			    struct tree *head,
> +			    struct merge_result *result,
> +			    int update_worktree_and_index,
> +			    int display_update_msgs);

Seems like these last two could probably be made into an enum and passed as flags instead of adding more parameters on the end.

Show 8 quoted lines
> +/* Do needed cleanup when not calling merge_switch_to_result() */
> +void merge_finalize(struct merge_options *opt,
> +		    struct merge_result *result);
> +
> +#endif
> --
> gitgitgadget
>

Thanks, Taylor

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 4 of 44 in “Beginning of new merge strategy: New API, empty implementation”
  1. 0/4 Beginning of new merge strategy: New API, empty implementationElijah Newren via GitGitGadget, Oct 21, 2020
  2. 2/4 merge-ort-wrappers: new convience wrappers to mimic the old merge APIElijah Newren via GitGitGadget, Oct 21, 2020
  3. 1/4 merge-ort: barebones API of new merge strategy with empty implementationElijah Newren via GitGitGadget, Oct 21, 2020
  4. Taylor BlauOct 23, 2020
  5. Elijah NewrenOct 23, 2020
  6. Peter BaumannOct 24, 2020
  7. 4/4 merge,rebase,revert: select ort or recursive by config or environmentElijah Newren via GitGitGadget, Oct 21, 2020
  8. 3/4 fast-rebase: demonstrate merge-ort's API via temporary/hidden commandElijah Newren via GitGitGadget, Oct 21, 2020
  9. Elijah NewrenOct 22, 2020
  10. Jonathan TanOct 26, 2020
  11. Elijah NewrenOct 27, 2020
  12. 0/4 Beginning of new merge strategy: New API, empty implementationElijah Newren via GitGitGadget, Oct 26, 2020
  13. 2/4 merge-ort-wrappers: new convience wrappers to mimic the old merge APIElijah Newren via GitGitGadget, Oct 26, 2020
  14. 4/4 merge,rebase,revert: select ort or recursive by config or environmentElijah Newren via GitGitGadget, Oct 26, 2020
  15. 3/4 fast-rebase: demonstrate merge-ort's API via temporary/hidden commandElijah Newren via GitGitGadget, Oct 26, 2020
  16. 1/4 merge-ort: barebones API of new merge strategy with empty implementationElijah Newren via GitGitGadget, Oct 26, 2020
  17. Junio C HamanoOct 26, 2020
  18. Elijah NewrenOct 26, 2020
  19. Junio C HamanoOct 26, 2020
  20. Elijah NewrenOct 26, 2020
  21. 0/4 Beginning of new merge strategy: New API, empty implementationElijah Newren via GitGitGadget, Oct 27, 2020
  22. 2/4 merge-ort-wrappers: new convience wrappers to mimic the old merge APIElijah Newren via GitGitGadget, Oct 27, 2020
  23. 1/4 merge-ort: barebones API of new merge strategy with empty implementationElijah Newren via GitGitGadget, Oct 27, 2020
  24. Eric SunshineOct 27, 2020
  25. Elijah NewrenOct 27, 2020
  26. Eric SunshineOct 27, 2020
  27. 3/4 fast-rebase: demonstrate merge-ort's API via temporary/hidden commandElijah Newren via GitGitGadget, Oct 27, 2020
  28. SZEDER GáborOct 27, 2020
  29. 4/4 merge,rebase,revert: select ort or recursive by config or environmentElijah Newren via GitGitGadget, Oct 27, 2020
  30. 0/4 Beginning of new merge strategy: New API, empty implementationElijah Newren via GitGitGadget, Oct 29, 2020
  31. 1/4 merge-ort: barebones API of new merge strategy with empty implementationElijah Newren via GitGitGadget, Oct 29, 2020
  32. 3/4 fast-rebase: demonstrate merge-ort's API via new test-tool commandElijah Newren via GitGitGadget, Oct 29, 2020
  33. 2/4 merge-ort-wrappers: new convience wrappers to mimic the old merge APIElijah Newren via GitGitGadget, Oct 29, 2020
  34. 4/4 merge,rebase,revert: select ort or recursive by config or environmentElijah Newren via GitGitGadget, Oct 29, 2020
  35. Jacob KellerNov 2, 2020
  36. Elijah NewrenNov 2, 2020
  37. Elijah NewrenNov 7, 2020
  38. 0/4 Beginning of new merge strategy: New API, empty implementationElijah Newren via GitGitGadget, Nov 2, 2020
  39. 3/4 fast-rebase: demonstrate merge-ort's API via new test-tool commandElijah Newren via GitGitGadget, Nov 2, 2020
  40. 2/4 merge-ort-wrappers: new convience wrappers to mimic the old merge APIElijah Newren via GitGitGadget, Nov 2, 2020
  41. 4/4 merge,rebase,revert: select ort or recursive by config or environmentElijah Newren via GitGitGadget, Nov 2, 2020
  42. 1/4 merge-ort: barebones API of new merge strategy with empty implementationElijah Newren via GitGitGadget, Nov 2, 2020
  43. Junio C HamanoNov 3, 2020
  44. Elijah NewrenOct 24, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.