Re: [WIP v1 2/4] mv: add check_dir_in_index() and solve general dir check issue

From: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Date: Apr 4, 2022, 07:25 UTC
Message-ID: <CAJyCBOSQVA5oEW7_iKZW895ycQ4T=XAx422s4m+6VFDSDVeNMA@mail.gmail.com>
In-Reply-To: <22aadea2-9330-aa9e-7b6a-834585189144@github.com>

On Fri, Apr 1, 2022 at 10:49 PM Derrick Stolee <derrickstolee@github.com> wrote:

Show 44 quoted lines

>
> On 4/1/2022 8:49 AM, Shaoxuan Yuan wrote:> On Fri, Apr 1, 2022 at 5:28 AM Victoria Dye <vdye@github.com> wrote:
> >>
> >> Shaoxuan Yuan wrote:
> >>> Originally, moving a <source> directory which is not on-disk due
> >>> to its existence outside of sparse-checkout cone, "giv mv" command
> >>> errors out with "bad source".
> >>>
> >>> Add a helper check_dir_in_index() function to see if a directory
> >>> name exists in the index. Also add a SPARSE_DIRECTORY bit to mark
> >>> such directories.
> >>>
> >>
> >> Hmm, I think this patch would fit better in your eventual "sparse index
> >> integration" series than this "prerequisite fixes to sparse-checkout"
> >> series. Sparse directories *only* appear when you're using a sparse index
> >> so, theoretically, this shouldn't ever come up (and thus isn't testable)
> >> until you're using a sparse index.
> >
> > After reading your feedback, I realized that I totally misused
> > the phrase "sparse directory". Clearly, this patch series does not
> > deal with sparse-
> > index yet, as "sparse directory" is a cache entry that points to a
> > tree, if sparse-index
> > is enabled. Silly me ;)
> >
> > What I was *actually* trying to say is: I want to change the checking
> > logic of moving
> > a "directory that exists outside of sparse-checkout cone", and I
> > apparently misused
> > "sparse directory" to reference such a thing.
>
> In the case of a full index (or an expanded sparse index, which is
> currently always the case for `git mv`), you detect a sparse directory
> by looking for the directory in the index, _not_ finding it, and then
> seeing if the cache entry at the position where the directory _would_
> exist is marked with the SKIP_WORKTREE bit.
>
> This works in cone mode and the old mode because I assume you've already
> checked for the existence of the directory, so if there _was_ any
> non-SKIP_WORKTREE cache entry within the directory, then the directory
> would exist in the worktree.
>
> (These are good details to include in the commit message.)

I read and think about this part a few times, but I'm still confused.

As Victoria pointed out earlier, and I quote, "Sparse directories *only* appear when you're using a sparse index, so, theoretically, this shouldn't ever come up (and thus isn't testable) until you're using a sparse index." So I'm not so sure what do you mean by putting "full index" and "sparse directory" together.

Thus, I go ahead and try to detect a directory that is outside of sparse-checkout cone, without sparse-index enabled.

I found a problem that if you use cache_name_pos() to do this detection, I imagined the following example (I'm trying to imitate an output of "git ls-files -t"):

H a H b S d/file1 H e/file1

So in this index, I use cache_name_pos() to find a directory "c/". I imagine the the value returned would be -3, which indicates this directory would be inserted at index position 2. However, the cache entry at position 2 is "d/file1", which is marked with SKIP_WORKTREE, and this fact cannot guarantee that "c/" is a sparse directory, since ''c/" is not in the index per se.

Probably I'm missing something, or I'm just dumb.

> Thanks,
> -Stolee

-- 
Thanks & Regards,
Shaoxuan