mbox series

[v4,0/4] Beginning of new merge strategy: New API, empty implementation

Message ID pull.895.v4.git.git.1604003535.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Beginning of new merge strategy: New API, empty implementation | expand

Message

Philippe Blain via GitGitGadget Oct. 29, 2020, 8:32 p.m. UTC
In this series, I show the new merge API I have developed in merge-ort, and
show how it differs from that provided by merge-recursive. I do this in four
steps, each corresponding to a patch.

Changes since v3:

 * Make 'fast-rebase' be a test-tool subcommand instead of a special hidden
   builtin, as suggested by Jonathan Tan and SZEDER Gábor

Elijah Newren (4):
  merge-ort: barebones API of new merge strategy with empty
    implementation
  merge-ort-wrappers: new convience wrappers to mimic the old merge API
  fast-rebase: demonstrate merge-ort's API via new test-tool command
  merge,rebase,revert: select ort or recursive by config or environment

 Makefile                    |   3 +
 builtin/merge.c             |  26 ++++-
 builtin/rebase.c            |   9 +-
 builtin/revert.c            |   2 +
 merge-ort-wrappers.c        |  62 +++++++++++
 merge-ort-wrappers.h        |  25 +++++
 merge-ort.c                 |  52 +++++++++
 merge-ort.h                 |  58 ++++++++++
 sequencer.c                 |  71 ++++++++++--
 t/helper/test-fast-rebase.c | 211 ++++++++++++++++++++++++++++++++++++
 t/helper/test-tool.c        |   1 +
 t/helper/test-tool.h        |   1 +
 12 files changed, 506 insertions(+), 15 deletions(-)
 create mode 100644 merge-ort-wrappers.c
 create mode 100644 merge-ort-wrappers.h
 create mode 100644 merge-ort.c
 create mode 100644 merge-ort.h
 create mode 100644 t/helper/test-fast-rebase.c


base-commit: 69986e19ffcfb9af674ae5180689ab7bbf92ed28
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-895%2Fnewren%2Fort-api-with-empty-implementation-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-895/newren/ort-api-with-empty-implementation-v4
Pull-Request: https://github.com/git/git/pull/895

Range-diff vs v3:

 1:  3357ea415e = 1:  3357ea415e merge-ort: barebones API of new merge strategy with empty implementation
 2:  d7f6a834ab = 2:  d7f6a834ab merge-ort-wrappers: new convience wrappers to mimic the old merge API
 3:  27ad756600 ! 3:  fce0db8778 fast-rebase: demonstrate merge-ort's API via temporary/hidden command
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    fast-rebase: demonstrate merge-ort's API via temporary/hidden command
     +    fast-rebase: demonstrate merge-ort's API via new test-tool command
      
     -    Add a special built-in that is only of use to git-developers and only
     -    during the development of merge-ort, and which is designed to
     -    immediately fail and print:
     -       git: 'fast-rebase' is not a git command
     -    unless a special GIT_TEST_MERGE_ALGORITHM environment variable is set.
     -
     -    This special builtin serves two purposes:
     +    Add a new test-tool command named 'fast-rebase', which is a
     +    super-slimmed down and nowhere near as capable version of 'git rebase'.
     +    'test-tool fast-rebase' is not currently planned for usage in the
     +    testsuite, but is here for two purposes:
      
            1) Demonstrate the desired API of merge-ort.  In particular,
               fast-rebase takes advantage of the separation of the merging
               operation from the updating of the index and working tree, to
               allow it to pick N commits, but only update the index and working
               tree once at the end.  Look for the calls to
     -         merge_inmemory_nonrecursive() and merge_switch_to_result().
     +         merge_incore_nonrecursive() and merge_switch_to_result().
      
            2) Provide a convenient benchmark that isn't polluted by the heavy
               disk writing and forking of unnecessary processes that comes from
     @@ Commit message
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Makefile ##
     -@@ Makefile: BUILTIN_OBJS += builtin/difftool.o
     - BUILTIN_OBJS += builtin/env--helper.o
     - BUILTIN_OBJS += builtin/fast-export.o
     - BUILTIN_OBJS += builtin/fast-import.o
     -+BUILTIN_OBJS += builtin/fast-rebase.o
     - BUILTIN_OBJS += builtin/fetch-pack.o
     - BUILTIN_OBJS += builtin/fetch.o
     - BUILTIN_OBJS += builtin/fmt-merge-msg.o
     -
     - ## builtin.h ##
     -@@ builtin.h: int cmd_difftool(int argc, const char **argv, const char *prefix);
     - int cmd_env__helper(int argc, const char **argv, const char *prefix);
     - int cmd_fast_export(int argc, const char **argv, const char *prefix);
     - int cmd_fast_import(int argc, const char **argv, const char *prefix);
     -+int cmd_fast_rebase(int argc, const char **argv, const char *prefix);
     - int cmd_fetch(int argc, const char **argv, const char *prefix);
     - int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
     - int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
     +@@ Makefile: TEST_BUILTINS_OBJS += test-dump-fsmonitor.o
     + TEST_BUILTINS_OBJS += test-dump-split-index.o
     + TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
     + TEST_BUILTINS_OBJS += test-example-decorate.o
     ++TEST_BUILTINS_OBJS += test-fast-rebase.o
     + TEST_BUILTINS_OBJS += test-genrandom.o
     + TEST_BUILTINS_OBJS += test-genzeros.o
     + TEST_BUILTINS_OBJS += test-hash-speed.o
      
     - ## builtin/fast-rebase.c (new) ##
     + ## t/helper/test-fast-rebase.c (new) ##
      @@
      +/*
      + * "git fast-rebase" builtin command
     @@ builtin/fast-rebase.c (new)
      + */
      +
      +#define USE_THE_INDEX_COMPATIBILITY_MACROS
     -+#include "builtin.h"
     ++#include "test-tool.h"
      +
      +#include "cache-tree.h"
      +#include "commit.h"
     @@ builtin/fast-rebase.c (new)
      +	return (struct commit *)obj;
      +}
      +
     -+int cmd_fast_rebase(int argc, const char **argv, const char *prefix)
     ++int cmd__fast_rebase(int argc, const char **argv)
      +{
      +	struct commit *onto;
      +	struct commit *last_commit = NULL, *last_picked_commit = NULL;
     @@ builtin/fast-rebase.c (new)
      +	struct strbuf reflog_msg = STRBUF_INIT;
      +	struct strbuf branch_name = STRBUF_INIT;
      +
     ++	/*
     ++	 * test-tool stuff doesn't set up the git directory by default; need to
     ++	 * do that manually.
     ++	 */
     ++	setup_git_directory();
     ++
      +	if (argc == 2 && !strcmp(argv[1], "-h")) {
      +		printf("Sorry, I am not a psychiatrist; I can not give you the help you need.  Oh, you meant usage...\n");
      +		exit(129);
      +	}
      +
     -+	if (!getenv("GIT_TEST_MERGE_ALGORITHM")) {
     -+		fprintf_ln(stderr, _("git: 'fast-rebase' is not a git command. See 'git --help'."));
     -+		exit(1);
     -+	}
     -+
      +	if (argc != 5 || strcmp(argv[1], "--onto"))
      +		die("usage: read the code, figure out how to use it, then do so");
      +
     @@ builtin/fast-rebase.c (new)
      +	return (clean == 0);
      +}
      
     - ## git.c ##
     -@@ git.c: static struct cmd_struct commands[] = {
     - 	{ "env--helper", cmd_env__helper },
     - 	{ "fast-export", cmd_fast_export, RUN_SETUP },
     - 	{ "fast-import", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },
     -+	{ "fast-rebase", cmd_fast_rebase, RUN_SETUP /* | NEED_WORK_TREE */ },
     - 	{ "fetch", cmd_fetch, RUN_SETUP },
     - 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
     - 	{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
     + ## t/helper/test-tool.c ##
     +@@ t/helper/test-tool.c: static struct test_cmd cmds[] = {
     + 	{ "dump-split-index", cmd__dump_split_index },
     + 	{ "dump-untracked-cache", cmd__dump_untracked_cache },
     + 	{ "example-decorate", cmd__example_decorate },
     ++	{ "fast-rebase", cmd__fast_rebase },
     + 	{ "genrandom", cmd__genrandom },
     + 	{ "genzeros", cmd__genzeros },
     + 	{ "hashmap", cmd__hashmap },
     +
     + ## t/helper/test-tool.h ##
     +@@ t/helper/test-tool.h: int cmd__dump_fsmonitor(int argc, const char **argv);
     + int cmd__dump_split_index(int argc, const char **argv);
     + int cmd__dump_untracked_cache(int argc, const char **argv);
     + int cmd__example_decorate(int argc, const char **argv);
     ++int cmd__fast_rebase(int argc, const char **argv);
     + int cmd__genrandom(int argc, const char **argv);
     + int cmd__genzeros(int argc, const char **argv);
     + int cmd__hashmap(int argc, const char **argv);
 4:  0479d59c33 = 4:  75d19804bd merge,rebase,revert: select ort or recursive by config or environment

Comments

Jacob Keller Nov. 2, 2020, 9:27 a.m. UTC | #1
On Thu, Oct 29, 2020 at 1:34 PM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> In this series, I show the new merge API I have developed in merge-ort, and
> show how it differs from that provided by merge-recursive. I do this in four
> steps, each corresponding to a patch.
>

I'm definitely excited by this project. I'm curious if you have any
further implementation as a WIP that could be played with to see the
end result of the new merging?

I definitely like this approach where you work in smaller increments
to make the implementation easier to review!

Thanks,
Jake

> Changes since v3:
>
>  * Make 'fast-rebase' be a test-tool subcommand instead of a special hidden
>    builtin, as suggested by Jonathan Tan and SZEDER Gábor
>
> Elijah Newren (4):
>   merge-ort: barebones API of new merge strategy with empty
>     implementation
>   merge-ort-wrappers: new convience wrappers to mimic the old merge API
>   fast-rebase: demonstrate merge-ort's API via new test-tool command
>   merge,rebase,revert: select ort or recursive by config or environment
>
>  Makefile                    |   3 +
>  builtin/merge.c             |  26 ++++-
>  builtin/rebase.c            |   9 +-
>  builtin/revert.c            |   2 +
>  merge-ort-wrappers.c        |  62 +++++++++++
>  merge-ort-wrappers.h        |  25 +++++
>  merge-ort.c                 |  52 +++++++++
>  merge-ort.h                 |  58 ++++++++++
>  sequencer.c                 |  71 ++++++++++--
>  t/helper/test-fast-rebase.c | 211 ++++++++++++++++++++++++++++++++++++
>  t/helper/test-tool.c        |   1 +
>  t/helper/test-tool.h        |   1 +
>  12 files changed, 506 insertions(+), 15 deletions(-)
>  create mode 100644 merge-ort-wrappers.c
>  create mode 100644 merge-ort-wrappers.h
>  create mode 100644 merge-ort.c
>  create mode 100644 merge-ort.h
>  create mode 100644 t/helper/test-fast-rebase.c
>
>
> base-commit: 69986e19ffcfb9af674ae5180689ab7bbf92ed28
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-895%2Fnewren%2Fort-api-with-empty-implementation-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-895/newren/ort-api-with-empty-implementation-v4
> Pull-Request: https://github.com/git/git/pull/895
>
> Range-diff vs v3:
>
>  1:  3357ea415e = 1:  3357ea415e merge-ort: barebones API of new merge strategy with empty implementation
>  2:  d7f6a834ab = 2:  d7f6a834ab merge-ort-wrappers: new convience wrappers to mimic the old merge API
>  3:  27ad756600 ! 3:  fce0db8778 fast-rebase: demonstrate merge-ort's API via temporary/hidden command
>      @@ Metadata
>       Author: Elijah Newren <newren@gmail.com>
>
>        ## Commit message ##
>      -    fast-rebase: demonstrate merge-ort's API via temporary/hidden command
>      +    fast-rebase: demonstrate merge-ort's API via new test-tool command
>
>      -    Add a special built-in that is only of use to git-developers and only
>      -    during the development of merge-ort, and which is designed to
>      -    immediately fail and print:
>      -       git: 'fast-rebase' is not a git command
>      -    unless a special GIT_TEST_MERGE_ALGORITHM environment variable is set.
>      -
>      -    This special builtin serves two purposes:
>      +    Add a new test-tool command named 'fast-rebase', which is a
>      +    super-slimmed down and nowhere near as capable version of 'git rebase'.
>      +    'test-tool fast-rebase' is not currently planned for usage in the
>      +    testsuite, but is here for two purposes:
>
>             1) Demonstrate the desired API of merge-ort.  In particular,
>                fast-rebase takes advantage of the separation of the merging
>                operation from the updating of the index and working tree, to
>                allow it to pick N commits, but only update the index and working
>                tree once at the end.  Look for the calls to
>      -         merge_inmemory_nonrecursive() and merge_switch_to_result().
>      +         merge_incore_nonrecursive() and merge_switch_to_result().
>
>             2) Provide a convenient benchmark that isn't polluted by the heavy
>                disk writing and forking of unnecessary processes that comes from
>      @@ Commit message
>           Signed-off-by: Elijah Newren <newren@gmail.com>
>
>        ## Makefile ##
>      -@@ Makefile: BUILTIN_OBJS += builtin/difftool.o
>      - BUILTIN_OBJS += builtin/env--helper.o
>      - BUILTIN_OBJS += builtin/fast-export.o
>      - BUILTIN_OBJS += builtin/fast-import.o
>      -+BUILTIN_OBJS += builtin/fast-rebase.o
>      - BUILTIN_OBJS += builtin/fetch-pack.o
>      - BUILTIN_OBJS += builtin/fetch.o
>      - BUILTIN_OBJS += builtin/fmt-merge-msg.o
>      -
>      - ## builtin.h ##
>      -@@ builtin.h: int cmd_difftool(int argc, const char **argv, const char *prefix);
>      - int cmd_env__helper(int argc, const char **argv, const char *prefix);
>      - int cmd_fast_export(int argc, const char **argv, const char *prefix);
>      - int cmd_fast_import(int argc, const char **argv, const char *prefix);
>      -+int cmd_fast_rebase(int argc, const char **argv, const char *prefix);
>      - int cmd_fetch(int argc, const char **argv, const char *prefix);
>      - int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
>      - int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
>      +@@ Makefile: TEST_BUILTINS_OBJS += test-dump-fsmonitor.o
>      + TEST_BUILTINS_OBJS += test-dump-split-index.o
>      + TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
>      + TEST_BUILTINS_OBJS += test-example-decorate.o
>      ++TEST_BUILTINS_OBJS += test-fast-rebase.o
>      + TEST_BUILTINS_OBJS += test-genrandom.o
>      + TEST_BUILTINS_OBJS += test-genzeros.o
>      + TEST_BUILTINS_OBJS += test-hash-speed.o
>
>      - ## builtin/fast-rebase.c (new) ##
>      + ## t/helper/test-fast-rebase.c (new) ##
>       @@
>       +/*
>       + * "git fast-rebase" builtin command
>      @@ builtin/fast-rebase.c (new)
>       + */
>       +
>       +#define USE_THE_INDEX_COMPATIBILITY_MACROS
>      -+#include "builtin.h"
>      ++#include "test-tool.h"
>       +
>       +#include "cache-tree.h"
>       +#include "commit.h"
>      @@ builtin/fast-rebase.c (new)
>       + return (struct commit *)obj;
>       +}
>       +
>      -+int cmd_fast_rebase(int argc, const char **argv, const char *prefix)
>      ++int cmd__fast_rebase(int argc, const char **argv)
>       +{
>       + struct commit *onto;
>       + struct commit *last_commit = NULL, *last_picked_commit = NULL;
>      @@ builtin/fast-rebase.c (new)
>       + struct strbuf reflog_msg = STRBUF_INIT;
>       + struct strbuf branch_name = STRBUF_INIT;
>       +
>      ++ /*
>      ++  * test-tool stuff doesn't set up the git directory by default; need to
>      ++  * do that manually.
>      ++  */
>      ++ setup_git_directory();
>      ++
>       + if (argc == 2 && !strcmp(argv[1], "-h")) {
>       +         printf("Sorry, I am not a psychiatrist; I can not give you the help you need.  Oh, you meant usage...\n");
>       +         exit(129);
>       + }
>       +
>      -+ if (!getenv("GIT_TEST_MERGE_ALGORITHM")) {
>      -+         fprintf_ln(stderr, _("git: 'fast-rebase' is not a git command. See 'git --help'."));
>      -+         exit(1);
>      -+ }
>      -+
>       + if (argc != 5 || strcmp(argv[1], "--onto"))
>       +         die("usage: read the code, figure out how to use it, then do so");
>       +
>      @@ builtin/fast-rebase.c (new)
>       + return (clean == 0);
>       +}
>
>      - ## git.c ##
>      -@@ git.c: static struct cmd_struct commands[] = {
>      -  { "env--helper", cmd_env__helper },
>      -  { "fast-export", cmd_fast_export, RUN_SETUP },
>      -  { "fast-import", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },
>      -+ { "fast-rebase", cmd_fast_rebase, RUN_SETUP /* | NEED_WORK_TREE */ },
>      -  { "fetch", cmd_fetch, RUN_SETUP },
>      -  { "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
>      -  { "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
>      + ## t/helper/test-tool.c ##
>      +@@ t/helper/test-tool.c: static struct test_cmd cmds[] = {
>      +  { "dump-split-index", cmd__dump_split_index },
>      +  { "dump-untracked-cache", cmd__dump_untracked_cache },
>      +  { "example-decorate", cmd__example_decorate },
>      ++ { "fast-rebase", cmd__fast_rebase },
>      +  { "genrandom", cmd__genrandom },
>      +  { "genzeros", cmd__genzeros },
>      +  { "hashmap", cmd__hashmap },
>      +
>      + ## t/helper/test-tool.h ##
>      +@@ t/helper/test-tool.h: int cmd__dump_fsmonitor(int argc, const char **argv);
>      + int cmd__dump_split_index(int argc, const char **argv);
>      + int cmd__dump_untracked_cache(int argc, const char **argv);
>      + int cmd__example_decorate(int argc, const char **argv);
>      ++int cmd__fast_rebase(int argc, const char **argv);
>      + int cmd__genrandom(int argc, const char **argv);
>      + int cmd__genzeros(int argc, const char **argv);
>      + int cmd__hashmap(int argc, const char **argv);
>  4:  0479d59c33 = 4:  75d19804bd merge,rebase,revert: select ort or recursive by config or environment
>
> --
> gitgitgadget
Elijah Newren Nov. 2, 2020, 6:52 p.m. UTC | #2
On Mon, Nov 2, 2020 at 1:28 AM Jacob Keller <jacob.keller@gmail.com> wrote:
>
> On Thu, Oct 29, 2020 at 1:34 PM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > In this series, I show the new merge API I have developed in merge-ort, and
> > show how it differs from that provided by merge-recursive. I do this in four
> > steps, each corresponding to a patch.
> >
>
> I'm definitely excited by this project. I'm curious if you have any
> further implementation as a WIP that could be played with to see the
> end result of the new merging?
>
> I definitely like this approach where you work in smaller increments
> to make the implementation easier to review!

I usually keep the 'ort' branch of https://github.com/newren/git
functional (no promises, though).  It has lots of ifdefs, super ugly
commits, todos & fixmes, and random additional (non-code) files where
I was tracking various things I was working on, so the code and the
tree may not be super readable, but it should be usable (and passes
all the tests) -- just set pull.twohead=ort in your git config, or set
the environment variable GIT_TEST_MERGE_ALGORITHM=ort.

One warning: git cherry-pick --continue fails with "Cannot specify
both --continue and --strategy"; my handling to set a --strategy
option when pull.twohead was set apparently needs some tweaks.  If you
spot any bugs or other issues, let me know.
Elijah Newren Nov. 7, 2020, 6:09 a.m. UTC | #3
Hi Jake,

On Mon, Nov 2, 2020 at 10:52 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Nov 2, 2020 at 1:28 AM Jacob Keller <jacob.keller@gmail.com> wrote:
> >
> > On Thu, Oct 29, 2020 at 1:34 PM Elijah Newren via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> > >
> > > In this series, I show the new merge API I have developed in merge-ort, and
> > > show how it differs from that provided by merge-recursive. I do this in four
> > > steps, each corresponding to a patch.
> > >
> >
> > I'm definitely excited by this project. I'm curious if you have any
> > further implementation as a WIP that could be played with to see the
> > end result of the new merging?
> >
> > I definitely like this approach where you work in smaller increments
> > to make the implementation easier to review!
>
> I usually keep the 'ort' branch of https://github.com/newren/git
> functional (no promises, though).  It has lots of ifdefs, super ugly
> commits, todos & fixmes, and random additional (non-code) files where
> I was tracking various things I was working on, so the code and the
> tree may not be super readable, but it should be usable (and passes
> all the tests) -- just set pull.twohead=ort in your git config, or set
> the environment variable GIT_TEST_MERGE_ALGORITHM=ort.
>
> One warning: git cherry-pick --continue fails with "Cannot specify
> both --continue and --strategy"; my handling to set a --strategy
> option when pull.twohead was set apparently needs some tweaks.  If you
> spot any bugs or other issues, let me know.

In addition to fixing this "cherry-pick --continue" bug last Monday, I
discovered a bug yesterday while re-merging all the merge commits in
the linux kernel causing it to fail an assertion.  I'm surprised I
hadn't hit that sooner, but if you're testing it out you may want to
update your copy to the latest version of the 'ort' branch (make sure
it has commit 067e5c1a38, "merge-ort: fix bug with cached_target_names
not being initialized in redos", 2020-11-06).