mbox series

[v7,0/2] merge-tree: allow specifying a base commit when --write-tree is passed

Message ID pull.1397.v7.git.1668210314.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series merge-tree: allow specifying a base commit when --write-tree is passed | expand

Message

Philippe Blain via GitGitGadget Nov. 11, 2022, 11:45 p.m. UTC
Thanks for everyone's careful reviews.

In this patch, I introduce a new --merge-base=<commit> option to allow
callers to specify a merge-base for the merge and extend the input accepted
by --stdin to also allow a specified merge-base with each merge requested.

Regards, Kyle

Changes since v1:

 * Changed merge_incore_recursive() to merge_incore_nonrecursive() when
   merge-base is specified.
 * Fixed c style problem.
 * Moved commit lookup/die logic out to the parsing logic in
   cmd_merge_tree().
 * use test_commit for test

Changes since v2:

 * commit message
 * Rebased on top of en/merge-tree-sequence.
 * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To
   make it easier to pass parameters, I moved
   lookup_commit_reference_by_name() to real_ merge() again.
 * Added test comment.

Changes since v3:

 * support --merge-base in conjunction with --stdin

Changes since v4:

 * commit message
 * added input format document
 * changed the input format for specifying the merge-base when --stdin is
   passed
 * changed the output when --stdin and --merge-base are used at the same
   time
 * add comment for test

Changes since v5:

 * improved test: remove the test repo after the test; avoid sub-shell.

Changes since v6:

 * fixed comment of test

Kyle Zhao (2):
  merge-tree.c: add --merge-base=<commit> option
  merge-tree.c: allow specifying the merge-base when --stdin is passed

 Documentation/git-merge-tree.txt | 16 ++++++++
 builtin/merge-tree.c             | 65 ++++++++++++++++++++++++++------
 t/t4301-merge-tree-write-tree.sh | 62 ++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 12 deletions(-)


base-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/1397

Range-diff vs v6:

 1:  1cf1c69b8e8 = 1:  1cf1c69b8e8 merge-tree.c: add --merge-base=<commit> option
 2:  40d56544e6e ! 2:  48e55d4e97c merge-tree.c: allow specifying the merge-base when --stdin is passed
     @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as par
       '
       
      +# Since the earlier tests have verified that individual merge-tree calls
     -+# are doing the right thing, this test case is only used to test whether
     -+# the input format is available.
     ++# are doing the right thing, this test case is only used to verify that
     ++# we can also trigger merges via --stdin, and that when we do we get
     ++# the same answer as running a bunch of separate merges.
      +
      +test_expect_success 'check the input format when --stdin is passed' '
      +	test_when_finished "rm -rf repo" &&

Comments

Elijah Newren Nov. 12, 2022, 12:32 a.m. UTC | #1
On Fri, Nov 11, 2022 at 3:45 PM Kyle Zhao via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> Thanks for everyone's careful reviews.
>
> In this patch, I introduce a new --merge-base=<commit> option to allow
> callers to specify a merge-base for the merge and extend the input accepted
> by --stdin to also allow a specified merge-base with each merge requested.
>
> Regards, Kyle
>
> Changes since v1:
>
>  * Changed merge_incore_recursive() to merge_incore_nonrecursive() when
>    merge-base is specified.
>  * Fixed c style problem.
>  * Moved commit lookup/die logic out to the parsing logic in
>    cmd_merge_tree().
>  * use test_commit for test
>
> Changes since v2:
>
>  * commit message
>  * Rebased on top of en/merge-tree-sequence.
>  * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To
>    make it easier to pass parameters, I moved
>    lookup_commit_reference_by_name() to real_ merge() again.
>  * Added test comment.
>
> Changes since v3:
>
>  * support --merge-base in conjunction with --stdin
>
> Changes since v4:
>
>  * commit message
>  * added input format document
>  * changed the input format for specifying the merge-base when --stdin is
>    passed
>  * changed the output when --stdin and --merge-base are used at the same
>    time
>  * add comment for test
>
> Changes since v5:
>
>  * improved test: remove the test repo after the test; avoid sub-shell.
>
> Changes since v6:
>
>  * fixed comment of test

Thanks; this version looks good to me and is ready to merge down.

> Kyle Zhao (2):
>   merge-tree.c: add --merge-base=<commit> option
>   merge-tree.c: allow specifying the merge-base when --stdin is passed
>
>  Documentation/git-merge-tree.txt | 16 ++++++++
>  builtin/merge-tree.c             | 65 ++++++++++++++++++++++++++------
>  t/t4301-merge-tree-write-tree.sh | 62 ++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+), 12 deletions(-)
>
>
> base-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v7
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v7
> Pull-Request: https://github.com/gitgitgadget/git/pull/1397
>
> Range-diff vs v6:
>
>  1:  1cf1c69b8e8 = 1:  1cf1c69b8e8 merge-tree.c: add --merge-base=<commit> option
>  2:  40d56544e6e ! 2:  48e55d4e97c merge-tree.c: allow specifying the merge-base when --stdin is passed
>      @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as par
>        '
>
>       +# Since the earlier tests have verified that individual merge-tree calls
>      -+# are doing the right thing, this test case is only used to test whether
>      -+# the input format is available.
>      ++# are doing the right thing, this test case is only used to verify that
>      ++# we can also trigger merges via --stdin, and that when we do we get
>      ++# the same answer as running a bunch of separate merges.
>       +
>       +test_expect_success 'check the input format when --stdin is passed' '
>       + test_when_finished "rm -rf repo" &&
>
> --
> gitgitgadget
Taylor Blau Nov. 13, 2022, 4:53 a.m. UTC | #2
On Fri, Nov 11, 2022 at 11:45:12PM +0000, Kyle Zhao via GitGitGadget wrote:
> Thanks for everyone's careful reviews.
>
> In this patch, I introduce a new --merge-base=<commit> option to allow
> callers to specify a merge-base for the merge and extend the input accepted
> by --stdin to also allow a specified merge-base with each merge requested.

Thanks for the updated round. I applied it locally, now let's see what
reviewers think of it...

Thanks,
Taylor
Taylor Blau Nov. 13, 2022, 4:54 a.m. UTC | #3
On Sat, Nov 12, 2022 at 11:53:34PM -0500, Taylor Blau wrote:
> On Fri, Nov 11, 2022 at 11:45:12PM +0000, Kyle Zhao via GitGitGadget wrote:
> > Thanks for everyone's careful reviews.
> >
> > In this patch, I introduce a new --merge-base=<commit> option to allow
> > callers to specify a merge-base for the merge and extend the input accepted
> > by --stdin to also allow a specified merge-base with each merge requested.
>
> Thanks for the updated round. I applied it locally, now let's see what
> reviewers think of it...

Oops ;-). I should have looked further down in my inbox before replying.
Looks like this is ready to merge down according to Elijah, so let's
start cooking this in 'next'.

Thanks again.

Thanks,
Taylor