git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Derrick Stolee <stolee@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 6/6] sparse-index: integrate with cherry-pick and rebase
Date: Fri, 20 Aug 2021 17:12:41 -0700	[thread overview]
Message-ID: <CABPp-BF_djmCi6XdsMGx6F8fOqzJLog24rW=WsFo2VJgeyu99A@mail.gmail.com> (raw)
In-Reply-To: <350ed86a4533d7fe332aebdf88cc474ceaf30ffe.1629220124.git.gitgitgadget@gmail.com>

On Tue, Aug 17, 2021 at 10:08 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The hard work was already done with 'git merge' and the ORT strategy.
> Just add extra tests to see that we get the expected results in the
> non-conflict cases.

:-)

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/rebase.c                         |  6 ++++
>  builtin/revert.c                         |  3 ++
>  t/t1092-sparse-checkout-compatibility.sh | 41 ++++++++++++++++++++++--
>  3 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 33e09619005..27433d7c5a2 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -559,6 +559,9 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
>         argc = parse_options(argc, argv, prefix, options,
>                         builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0);
>
> +       prepare_repo_settings(the_repository);
> +       the_repository->settings.command_requires_full_index = 0;
> +
>         if (!is_null_oid(&squash_onto))
>                 opts.squash_onto = &squash_onto;
>
> @@ -1430,6 +1433,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>                 usage_with_options(builtin_rebase_usage,
>                                    builtin_rebase_options);
>
> +       prepare_repo_settings(the_repository);
> +       the_repository->settings.command_requires_full_index = 0;
> +

While rebase can use either merge-recursive or merge-ort under the
covers, this change is okay because merge-recursive.c now has code to
expand the index unconditionally, and merge-ort does so only
conditionally in the cases needed.  Right?  (Same for change below to
revert.c)

>         options.allow_empty_message = 1;
>         git_config(rebase_config, &options);
>         /* options.gpg_sign_opt will be either "-S" or NULL */
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 237f2f18d4c..6c4c22691bd 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -136,6 +136,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
>                         PARSE_OPT_KEEP_ARGV0 |
>                         PARSE_OPT_KEEP_UNKNOWN);
>
> +       prepare_repo_settings(the_repository);
> +       the_repository->settings.command_requires_full_index = 0;
> +
>         /* implies allow_empty */
>         if (opts->keep_redundant_commits)
>                 opts->allow_empty = 1;
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index a52d2edda54..c047b95b121 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -532,6 +532,38 @@ test_expect_success 'merge with conflict outside cone' '
>         test_all_match git rev-parse HEAD^{tree}
>  '
>
> +test_expect_success 'cherry-pick/rebase with conflict outside cone' '
> +       init_repos &&
> +
> +       for OPERATION in cherry-pick rebase
> +       do
> +               test_all_match git checkout -B tip &&
> +               test_all_match git reset --hard merge-left &&
> +               test_all_match git status --porcelain=v2 &&
> +               test_all_match test_must_fail git $OPERATION merge-right &&
> +               test_all_match git status --porcelain=v2 &&
> +
> +               # Resolve the conflict in different ways:
> +               # 1. Revert to the base
> +               test_all_match git checkout base -- deep/deeper2/a &&
> +               test_all_match git status --porcelain=v2 &&
> +
> +               # 2. Add the file with conflict markers
> +               test_all_match git add folder1/a &&
> +               test_all_match git status --porcelain=v2 &&
> +
> +               # 3. Rename the file to another sparse filename and
> +               #    accept conflict markers as resolved content.
> +               run_on_all mv folder2/a folder2/z &&
> +               test_all_match git add folder2 &&
> +               test_all_match git status --porcelain=v2 &&
> +
> +               test_all_match git $OPERATION --continue &&
> +               test_all_match git status --porcelain=v2 &&
> +               test_all_match git rev-parse HEAD^{tree} || return 1
> +       done
> +'
> +
>  test_expect_success 'merge with outside renames' '
>         init_repos &&
>
> @@ -670,9 +702,12 @@ test_expect_success 'sparse-index is not expanded' '
>         echo >>sparse-index/untracked.txt &&
>         ensure_not_expanded add . &&
>
> -       ensure_not_expanded checkout -f update-deep &&
> -       ensure_not_expanded merge -s ort -m merge update-folder1 &&
> -       ensure_not_expanded merge -s ort -m merge update-folder2
> +       for OPERATION in "merge -s ort -m merge" cherry-pick rebase
> +       do
> +               ensure_not_expanded checkout -f -B temp update-deep &&
> +               ensure_not_expanded $OPERATION update-folder1 &&
> +               ensure_not_expanded $OPERATION update-folder2 || return 1
> +       done

Won't this fail on linux-gcc in the automated testing, since it uses
GIT_TEST_MERGE_ALGORITHM=recursive and you didn't override the merge
strategy for cherry-pick and rebase?

To fix, you'd either need to put a GIT_TEST_MERGE_ALGORITHM=ort at the
beginning of the file, or add `--strategy ort` options to the
cherry-pick and rebase commands.


>  '
>
>  test_expect_success 'sparse-index is not expanded: merge conflict in cone' '
> --
> gitgitgadget

  reply	other threads:[~2021-08-21  0:12 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17 17:08 [PATCH 0/6] Sparse Index: Integrate with merge, cherry-pick, rebase, and revert Derrick Stolee via GitGitGadget
2021-08-17 17:08 ` [PATCH 1/6] t1092: use ORT merge strategy Derrick Stolee via GitGitGadget
2021-08-18 17:16   ` Taylor Blau
2021-08-18 18:10   ` Junio C Hamano
2021-08-18 18:42     ` Derrick Stolee
2021-08-18 22:22       ` Junio C Hamano
2021-08-20 21:23       ` Elijah Newren
2021-08-20 23:32         ` Junio C Hamano
2021-08-21  0:20           ` Elijah Newren
2021-08-21  4:20             ` Junio C Hamano
2021-08-21 23:48               ` Elijah Newren
2021-08-17 17:08 ` [PATCH 2/6] diff: ignore sparse paths in diffstat Derrick Stolee via GitGitGadget
2021-08-20 21:32   ` Elijah Newren
2021-08-24 18:30     ` Derrick Stolee
2021-08-27 22:27       ` Elijah Newren
2021-08-17 17:08 ` [PATCH 3/6] merge: make sparse-aware with ORT Derrick Stolee via GitGitGadget
2021-08-20 21:40   ` Elijah Newren
2021-08-17 17:08 ` [PATCH 4/6] merge-ort: expand only for out-of-cone conflicts Derrick Stolee via GitGitGadget
2021-08-20 21:53   ` Elijah Newren
2021-08-17 17:08 ` [PATCH 5/6] t1092: add cherry-pick, rebase tests Derrick Stolee via GitGitGadget
2021-08-20 21:58   ` Elijah Newren
2021-08-17 17:08 ` [PATCH 6/6] sparse-index: integrate with cherry-pick and rebase Derrick Stolee via GitGitGadget
2021-08-21  0:12   ` Elijah Newren [this message]
2021-08-24 21:52 ` [PATCH v2 0/6] Sparse Index: Integrate with merge, cherry-pick, rebase, and revert Derrick Stolee via GitGitGadget
2021-08-24 21:52   ` [PATCH v2 1/6] diff: ignore sparse paths in diffstat Derrick Stolee via GitGitGadget
2021-08-24 21:52   ` [PATCH v2 2/6] merge: make sparse-aware with ORT Derrick Stolee via GitGitGadget
2021-08-27 22:43     ` Elijah Newren
2021-08-30 17:18       ` Derrick Stolee
2021-09-08  1:49         ` Derrick Stolee
2021-08-24 21:52   ` [PATCH v2 3/6] merge-ort: expand only for out-of-cone conflicts Derrick Stolee via GitGitGadget
2021-08-27 22:47     ` Elijah Newren
2021-08-30 17:21       ` Derrick Stolee
2021-08-24 21:52   ` [PATCH v2 4/6] t1092: add cherry-pick, rebase tests Derrick Stolee via GitGitGadget
2021-08-24 21:52   ` [PATCH v2 5/6] sequencer: ensure full index if not ORT strategy Derrick Stolee via GitGitGadget
2021-08-24 21:52   ` [PATCH v2 6/6] sparse-index: integrate with cherry-pick and rebase Derrick Stolee via GitGitGadget
2021-08-27 22:56   ` [PATCH v2 0/6] Sparse Index: Integrate with merge, cherry-pick, rebase, and revert Elijah Newren
2021-08-30 17:26     ` Derrick Stolee
2021-09-08 11:23   ` [PATCH v3 " Derrick Stolee via GitGitGadget
2021-09-08 11:23     ` [PATCH v3 1/6] diff: ignore sparse paths in diffstat Derrick Stolee via GitGitGadget
2021-09-08 11:23     ` [PATCH v3 2/6] merge: make sparse-aware with ORT Derrick Stolee via GitGitGadget
2021-09-08 11:23     ` [PATCH v3 3/6] merge-ort: expand only for out-of-cone conflicts Derrick Stolee via GitGitGadget
2021-09-08 11:23     ` [PATCH v3 4/6] t1092: add cherry-pick, rebase tests Derrick Stolee via GitGitGadget
2021-09-08 11:24     ` [PATCH v3 5/6] sequencer: ensure full index if not ORT strategy Derrick Stolee via GitGitGadget
2021-09-08 11:24     ` [PATCH v3 6/6] sparse-index: integrate with cherry-pick and rebase Derrick Stolee via GitGitGadget
2021-09-09 14:16     ` [PATCH v3 0/6] Sparse Index: Integrate with merge, cherry-pick, rebase, and revert Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BF_djmCi6XdsMGx6F8fOqzJLog24rW=WsFo2VJgeyu99A@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).