| Message ID | 8aa41e749471df3bd9d593b8f55db6506eafea12.1623069252.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Sparse-index: integrate with status | expand |
On Mon, Jun 7, 2021 at 5:34 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > When creating a full index from a sparse one, we create cache entries > for every blob within a given sparse directory entry. These are > correctly marked with the CE_SKIP_WORKTREE flag, but they must also be > marked with the CE_EXTENDED flag to ensure that the skip-worktree bit is > correctly written to disk in the case that the index is not converted > back down to a sparse-index. In our previous discussion on this patch from v3 (https://lore.kernel.org/git/cb9161ca-dc6e-b77b-1a41-385ed8920bb2@gmail.com/), you said you'd explain the reason for this change in a bit more detail, but the commit message has not changed. Could this be corrected? > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > sparse-index.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sparse-index.c b/sparse-index.c > index 1b49898d0cb7..b2b3fbd75050 100644 > --- a/sparse-index.c > +++ b/sparse-index.c > @@ -222,7 +222,7 @@ static int add_path_to_index(const struct object_id *oid, > strbuf_addstr(base, path); > > ce = make_cache_entry(istate, mode, oid, base->buf, 0, 0); > - ce->ce_flags |= CE_SKIP_WORKTREE; > + ce->ce_flags |= CE_SKIP_WORKTREE | CE_EXTENDED; > set_index_entry(istate, istate->cache_nr++, ce); > > strbuf_setlen(base, len); > -- > gitgitgadget
On 6/8/2021 2:56 PM, Elijah Newren wrote: > On Mon, Jun 7, 2021 at 5:34 AM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> From: Derrick Stolee <dstolee@microsoft.com> >> >> When creating a full index from a sparse one, we create cache entries >> for every blob within a given sparse directory entry. These are >> correctly marked with the CE_SKIP_WORKTREE flag, but they must also be >> marked with the CE_EXTENDED flag to ensure that the skip-worktree bit is >> correctly written to disk in the case that the index is not converted >> back down to a sparse-index. > > In our previous discussion on this patch from v3 > (https://lore.kernel.org/git/cb9161ca-dc6e-b77b-1a41-385ed8920bb2@gmail.com/), > you said you'd explain the reason for this change in a bit more > detail, but the commit message has not changed. Thank you for the reminder. > Could this be corrected? How does this sound? When creating a full index from a sparse one, we create cache entries for every blob within a given sparse directory entry. These are correctly marked with the CE_SKIP_WORKTREE flag, but the CE_EXTENDED flag is not included. The CE_EXTENDED flag would exist if we loaded a full index from disk with these entries marked with CE_SKIP_WORKTREE, so we can add the flag here to be consistent. This allows us to directly compare the flags present in cache entries when testing the sparse-index feature, but has no significance to its correctness in the user-facing functionality. I have this in my local branch for now, but can update it before the next version. Thanks, -Stolee
On Wed, Jun 9, 2021 at 10:39 AM Derrick Stolee <stolee@gmail.com> wrote: > > On 6/8/2021 2:56 PM, Elijah Newren wrote: > > On Mon, Jun 7, 2021 at 5:34 AM Derrick Stolee via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > >> > >> From: Derrick Stolee <dstolee@microsoft.com> > >> > >> When creating a full index from a sparse one, we create cache entries > >> for every blob within a given sparse directory entry. These are > >> correctly marked with the CE_SKIP_WORKTREE flag, but they must also be > >> marked with the CE_EXTENDED flag to ensure that the skip-worktree bit is > >> correctly written to disk in the case that the index is not converted > >> back down to a sparse-index. > > > > In our previous discussion on this patch from v3 > > (https://lore.kernel.org/git/cb9161ca-dc6e-b77b-1a41-385ed8920bb2@gmail.com/), > > you said you'd explain the reason for this change in a bit more > > detail, but the commit message has not changed. > > Thank you for the reminder. > > > Could this be corrected? > > How does this sound? > > When creating a full index from a sparse one, we create cache entries > for every blob within a given sparse directory entry. These are > correctly marked with the CE_SKIP_WORKTREE flag, but the CE_EXTENDED > flag is not included. The CE_EXTENDED flag would exist if we loaded a > full index from disk with these entries marked with CE_SKIP_WORKTREE, so > we can add the flag here to be consistent. This allows us to directly > compare the flags present in cache entries when testing the sparse-index > feature, but has no significance to its correctness in the user-facing > functionality. > > I have this in my local branch for now, but can update it before the next > version. Thanks; this looks good to me.
diff --git a/sparse-index.c b/sparse-index.c index 1b49898d0cb7..b2b3fbd75050 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -222,7 +222,7 @@ static int add_path_to_index(const struct object_id *oid, strbuf_addstr(base, path); ce = make_cache_entry(istate, mode, oid, base->buf, 0, 0); - ce->ce_flags |= CE_SKIP_WORKTREE; + ce->ce_flags |= CE_SKIP_WORKTREE | CE_EXTENDED; set_index_entry(istate, istate->cache_nr++, ce); strbuf_setlen(base, len);