From: Derrick Stolee <stolee@gmail.com>
To: Matheus Tavares Bernardino <matheus.bernardino@usp.br>,
Elijah Newren <newren@gmail.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Derrick Stolee <derrickstolee@github.com>,
Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 05/27] add: ensure full index
Date: Wed, 17 Mar 2021 16:55:33 -0400 [thread overview]
Message-ID: <862197d9-ca71-f682-f03f-62e45ea921ee@gmail.com> (raw)
In-Reply-To: <CAHd-oW4DMAjQ_Gjbzkx6csFRAK1iKHsTfuJLniOjWehwrnHdVQ@mail.gmail.com>
On 3/17/2021 4:35 PM, Matheus Tavares Bernardino wrote:
> On Wed, Mar 17, 2021 at 2:36 PM Elijah Newren <newren@gmail.com> wrote:
>>
>> On Tue, Mar 16, 2021 at 2:17 PM Derrick Stolee via GitGitGadget
>> <gitgitgadget@gmail.com> wrote:>> I'm not so sure about this one; from git-add(1):
>>
>> --renormalize
>> Apply the "clean" process freshly to all tracked files to forcibly
>> add them again to the index. This is useful after changing
>> core.autocrlf configuration or the text attribute in order to
>> correct files added with wrong CRLF/LF line endings. This option
>> implies -u.
>>
>> ... to "all tracked files". Should that be interpreted as all files
>> within the sparsity paths, especially considering that we're trying to
>> enable folks to work within the set of files of interest to them?
I wrote in reply to another similar comment that I'm being overly
cautious in creating these protections. When I can come back later
with careful tests, we can ensure that everything behaves properly.
> I had the same question when working on the add/rm sparse-checkout
> series. As seen at [1], --renormalize and --chmod are, currently, the
> only options in git-add that do not honor the sparsity patterns and do
> update SKIP_WORKTREE paths too.
>
> But is this by design or possibly just an oversight? In my series I
> ended up making both options honor the sparsity rules, with a warning
> when requested to update any sparse path. (If we are going with this
> approach, perhaps should we also amend the docs to remove any
> ambiguity as for what "all tracked files" means in this case?)
I'd be happy to see a resolution here, and it can happen in parallel
to what I'm doing here.
>> The code below already has the following check below:
>>
>> if (!S_ISREG(ce->ce_mode) && !S_ISLNK(ce->ce_mode))
>> continue; /* do not touch non blobs */
>>
>> suggesting that it'd correctly skip over the sparse directories, so I
>> think this one just isn't needed.
>
> But if sparse index is not enabled, it does not skip over the sparse
> files, right? So isn't the ensure_full_index() call here (and in
> chmod_pathspec()) important to be consistent with the case when sparse
> index is not in use?
The tests I am adding in t1092-sparse-checkout-compatibility.sh are
focused on ensuring the same behavior across sparse-checkouts with
and without the sparse-index, and also a full checkout (when possible).
Since the sparse-index requires cone-mode sparse-checkout (currently),
and the sparse files to be within a directory (or no index change
happens), the tests you created in t3705-add-sparse-checkout.sh don't
seem to apply. I'll need to find some important scenarios to duplicate
in t1092 without the full depth of t3705.
> Or maybe Stolee could rebase this on top of my
> series, where both these places already skip the sparse paths?
> (Assuming that's the behavior we are looking for. If not, I can also> fix my series.)
I think they can proceed in parallel and then continue more carefully
in the future. The series _after_ this one makes 'git status' and
'git add' work with the sparse-index, so at that point we will be
removing these protections at the right time, with tests. In addition,
those tests will ensure that we don't expand the sparse-index unless
absolutely necessary.
If that work collides with your approach, then I'll rebase onto a
merge of that topic and this one.
> [1]: https://lore.kernel.org/git/d93c3f51465a3e409819db63bd81802e7ef20ea9.1615588108.git.matheus.bernardino@usp.br/
(I will go take a look over here. I've been ignoring the thread for
too long.)
Thanks,
-Stolee
next prev parent reply other threads:[~2021-03-17 20:56 UTC|newest]
Thread overview: 111+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-16 21:16 [PATCH 00/27] Sparse Index: API protections Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 01/27] *: remove 'const' qualifier for struct index_state Derrick Stolee via GitGitGadget
2021-03-19 21:01 ` Junio C Hamano
2021-03-20 1:45 ` Derrick Stolee
2021-03-20 1:52 ` Junio C Hamano
2021-03-30 16:53 ` Derrick Stolee
2021-03-16 21:16 ` [PATCH 02/27] read-cache: expand on query into sparse-directory entry Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 03/27] sparse-index: API protection strategy Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 04/27] cache: move ensure_full_index() to cache.h Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 05/27] add: ensure full index Derrick Stolee via GitGitGadget
2021-03-17 17:35 ` Elijah Newren
2021-03-17 20:35 ` Matheus Tavares Bernardino
2021-03-17 20:55 ` Derrick Stolee [this message]
2021-03-16 21:16 ` [PATCH 06/27] checkout-index: " Derrick Stolee via GitGitGadget
2021-03-17 17:50 ` Elijah Newren
2021-03-17 20:05 ` Derrick Stolee
2021-03-17 21:10 ` Elijah Newren
2021-03-17 21:33 ` Derrick Stolee
2021-03-17 22:36 ` Elijah Newren
2021-03-18 1:17 ` Derrick Stolee
2021-03-16 21:16 ` [PATCH 07/27] checkout: " Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 08/27] commit: " Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 09/27] difftool: " Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 10/27] fsck: " Derrick Stolee via GitGitGadget
2021-03-16 21:16 ` [PATCH 11/27] grep: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 12/27] ls-files: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 13/27] merge-index: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 14/27] rm: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 15/27] sparse-checkout: " Derrick Stolee via GitGitGadget
2021-03-18 5:22 ` Elijah Newren
2021-03-23 13:13 ` Derrick Stolee
2021-03-16 21:17 ` [PATCH 16/27] update-index: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 17/27] diff-lib: " Derrick Stolee via GitGitGadget
2021-03-18 5:24 ` Elijah Newren
2021-03-23 13:15 ` Derrick Stolee
2021-03-16 21:17 ` [PATCH 18/27] dir: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 19/27] entry: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 20/27] merge-ort: " Derrick Stolee via GitGitGadget
2021-03-18 5:31 ` Elijah Newren
2021-03-23 13:26 ` Derrick Stolee
2021-03-16 21:17 ` [PATCH 21/27] merge-recursive: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 22/27] pathspec: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 23/27] read-cache: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 24/27] resolve-undo: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 25/27] revision: " Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 26/27] sparse-index: expand_to_path() Derrick Stolee via GitGitGadget
2021-03-16 21:17 ` [PATCH 27/27] name-hash: use expand_to_path() Derrick Stolee via GitGitGadget
2021-03-17 18:03 ` [PATCH 00/27] Sparse Index: API protections Elijah Newren
2021-03-18 6:32 ` Elijah Newren
2021-04-01 1:49 ` [PATCH v2 00/25] " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 01/25] sparse-index: API protection strategy Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 02/25] *: remove 'const' qualifier for struct index_state Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 03/25] read-cache: expand on query into sparse-directory entry Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 04/25] cache: move ensure_full_index() to cache.h Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 05/25] add: ensure full index Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 06/25] checkout-index: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 07/25] checkout: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 08/25] commit: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 09/25] difftool: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 10/25] fsck: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 11/25] grep: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 12/25] ls-files: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 13/25] merge-index: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 14/25] rm: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 15/25] stash: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 16/25] update-index: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 17/25] dir: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 18/25] entry: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 19/25] merge-recursive: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 20/25] pathspec: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 21/25] read-cache: " Derrick Stolee via GitGitGadget
2021-04-01 1:49 ` [PATCH v2 22/25] resolve-undo: " Derrick Stolee via GitGitGadget
2021-04-01 1:50 ` [PATCH v2 23/25] revision: " Derrick Stolee via GitGitGadget
2021-04-01 1:50 ` [PATCH v2 24/25] sparse-index: expand_to_path() Derrick Stolee via GitGitGadget
2021-04-05 19:32 ` Elijah Newren
2021-04-06 11:46 ` Derrick Stolee
2021-04-01 1:50 ` [PATCH v2 25/25] name-hash: use expand_to_path() Derrick Stolee via GitGitGadget
2021-04-05 19:53 ` Elijah Newren
2021-04-01 7:07 ` [PATCH v2 00/25] Sparse Index: API protections Junio C Hamano
2021-04-01 13:32 ` Derrick Stolee
2021-04-05 19:55 ` Elijah Newren
2021-04-12 21:07 ` [PATCH v3 00/26] " Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 01/26] sparse-index: API protection strategy Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 02/26] *: remove 'const' qualifier for struct index_state Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 03/26] read-cache: expand on query into sparse-directory entry Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 04/26] cache: move ensure_full_index() to cache.h Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 05/26] add: ensure full index Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 06/26] checkout-index: " Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 07/26] checkout: " Derrick Stolee via GitGitGadget
2021-04-12 21:07 ` [PATCH v3 08/26] commit: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 09/26] difftool: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 10/26] fsck: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 11/26] grep: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 12/26] ls-files: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 13/26] merge-index: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 14/26] rm: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 15/26] stash: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 16/26] update-index: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 17/26] dir: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 18/26] entry: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 19/26] merge-recursive: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 20/26] pathspec: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 21/26] read-cache: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 22/26] resolve-undo: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 23/26] revision: " Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 24/26] name-hash: don't add directories to name_hash Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 25/26] sparse-index: expand_to_path() Derrick Stolee via GitGitGadget
2021-04-12 21:08 ` [PATCH v3 26/26] name-hash: use expand_to_path() Derrick Stolee via GitGitGadget
2021-04-13 16:02 ` [PATCH v3 00/26] Sparse Index: API protections Elijah Newren
2021-04-14 20:44 ` Junio C Hamano
2021-04-15 2:42 ` Derrick Stolee
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=862197d9-ca71-f682-f03f-62e45ea921ee@gmail.com \
--to=stolee@gmail.com \
--cc=derrickstolee@github.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=matheus.bernardino@usp.br \
--cc=newren@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.
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 NNTP newsgroup(s).