From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Matheus Tavares Bernardino <matheus.bernardino@usp.br>,
Derrick Stolee <stolee@gmail.com>,
Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH v2 0/7] Sparse index: integrate with commit and checkout
Date: Wed, 21 Jul 2021 21:22:37 -0700 [thread overview]
Message-ID: <CABPp-BGbM_+bznuCCLfctt-XqVdCuROa9+s_jpeQXmTHEyZmjg@mail.gmail.com> (raw)
In-Reply-To: <pull.973.v2.git.1626812081.gitgitgadget@gmail.com>
On Tue, Jul 20, 2021 at 1:14 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> This series extends our integration of sparse-index to 'git commit' and 'git
> checkout'.
>
> This is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work
> was already done in that topic, so these changes are simple.
>
> Recall that we have delayed our integration with 'git add' until we can work
> out the concerns about how to deal with pathspecs outside of the
> sparse-checkout definition. Those concerns might have some overlap with how
> 'git commit' takes a pathspec, but this seems like a rare enough case to
> handle here and we can be more careful with the behavior change in the next
> series which will integrate with git add.
>
> In addition to the tests that already exist in t1092, I have integrated
> these changes in microsoft/git and tested them against the Scalar functional
> tests, which go through quite a few complicated scenarios, verifying that
> things work the same across the full index and sparse-index cases.
>
>
> Update in V2
> ============
>
> * There is no change to the code, but it is presented in a slightly
> different order.
> * We've been discussing some complicated directory/file conflict cases, in
> particular with a staged change inside the directory. These tests are
> added and described as documenting incorrect behavior that should be
> changed.
> * After those tests are in place, we can motivate the change to
> twoway_merge() as necessary for a more-common situation (still rare) but
> still incorrect in an already-broken situation. Hopefully that balance is
> sufficient for now, until we can do the bigger work of fixing the bad
> behavior.
I read the first five patches previously. The tiny changes there in
the range-diff still look good to me.
I very much appreciate the new patch 6.
As noted in 7/7, I'm a little unhappy with the patch to
twoway_merge(), BUT you've clearly documented the shortcomings in very
good detail and pointed out how git has (likely for decades) messed up
in related ways for non-sparse checkouts with D/F conflicts. You've
documented it well enough and argued well enough about the relative
merits, that I have to agree with you that this is a good step
forward. I do hope we circle back and tie up the loose ends at some
point.
So, the whole series is:
Reviewed-by: Elijah Newren <newren@gmail.com>
>
> Thanks, -Stolee
>
> Derrick Stolee (7):
> p2000: add 'git checkout -' test and decrease depth
> p2000: compress repo names
> commit: integrate with sparse-index
> sparse-index: recompute cache-tree
> checkout: stop expanding sparse indexes
> t1092: document bad 'git checkout' behavior
> unpack-trees: resolve sparse-directory/file conflicts
>
> builtin/checkout.c | 8 +-
> builtin/commit.c | 3 +
> cache-tree.c | 2 -
> sparse-index.c | 2 +
> t/perf/p2000-sparse-operations.sh | 47 ++++--
> t/t1092-sparse-checkout-compatibility.sh | 197 ++++++++++++++++++++++-
> unpack-trees.c | 11 ++
> 7 files changed, 240 insertions(+), 30 deletions(-)
>
>
> base-commit: e5ca291076a8a936283bb2c57433c4393d3f80c2
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/973
>
> Range-diff vs v1:
>
> 1: bb3dd1fdd48 = 1: 6e74958f590 p2000: add 'git checkout -' test and decrease depth
> 2: eb15bf37685 = 2: 3e1d03c41be p2000: compress repo names
> 3: 413babe6e77 ! 3: cd94f820052 commit: integrate with sparse-index
> @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is e
> + ensure_not_expanded commit --include deep/deeper1/a -m deeper
> '
>
> - test_expect_success 'reset mixed and checkout orphan' '
> + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
> 4: ffe8473caab = 4: 65e79b8037c sparse-index: recompute cache-tree
> 5: 8710fee36b7 ! 5: e9a9981477e checkout: stop expanding sparse indexes
> @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
> + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1
> '
>
> - test_expect_success 'reset mixed and checkout orphan' '
> + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
> -: ----------- > 6: 4b801c854fb t1092: document bad 'git checkout' behavior
> -: ----------- > 7: 71e301501c8 unpack-trees: resolve sparse-directory/file conflicts
>
> --
> gitgitgadget
prev parent reply other threads:[~2021-07-22 4:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-29 2:13 [PATCH 0/5] Sparse index: integrate with commit and checkout Derrick Stolee via GitGitGadget
2021-06-29 2:13 ` [PATCH 1/5] p2000: add 'git checkout -' test and decrease depth Derrick Stolee via GitGitGadget
2021-06-29 2:13 ` [PATCH 2/5] p2000: compress repo names Derrick Stolee via GitGitGadget
2021-06-29 2:13 ` [PATCH 3/5] commit: integrate with sparse-index Derrick Stolee via GitGitGadget
2021-06-29 2:13 ` [PATCH 4/5] sparse-index: recompute cache-tree Derrick Stolee via GitGitGadget
2021-06-29 2:13 ` [PATCH 5/5] checkout: stop expanding sparse indexes Derrick Stolee via GitGitGadget
2021-07-09 21:26 ` [PATCH 0/5] Sparse index: integrate with commit and checkout Elijah Newren
2021-07-12 18:46 ` Derrick Stolee
2021-07-16 13:59 ` Derrick Stolee
2021-07-17 15:37 ` Elijah Newren
2021-07-19 14:05 ` Derrick Stolee
2021-07-20 20:14 ` [PATCH v2 0/7] " Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 1/7] p2000: add 'git checkout -' test and decrease depth Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 2/7] p2000: compress repo names Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 3/7] commit: integrate with sparse-index Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 4/7] sparse-index: recompute cache-tree Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 5/7] checkout: stop expanding sparse indexes Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 6/7] t1092: document bad 'git checkout' behavior Derrick Stolee via GitGitGadget
2021-07-20 20:14 ` [PATCH v2 7/7] unpack-trees: resolve sparse-directory/file conflicts Derrick Stolee via GitGitGadget
2021-07-22 4:19 ` Elijah Newren
2021-07-22 4:22 ` Elijah Newren [this message]
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-BGbM_+bznuCCLfctt-XqVdCuROa9+s_jpeQXmTHEyZmjg@mail.gmail.com \
--to=newren@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=matheus.bernardino@usp.br \
--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).