Re: [PATCH 05/27] add: ensure full index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 16, 2021 at 2:17 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> Before iterating over all cache entries, ensure that a sparse index is
> expanded to a full index to avoid unexpected behavior.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
>  builtin/add.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index ea762a41e3a2..ab2ef4a63530 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -141,6 +141,7 @@ static int renormalize_tracked_files(const struct pathspec *pathspec, int flags)
>  {
>         int i, retval = 0;
>
> +       ensure_full_index(&the_index);
>         for (i = 0; i < active_nr; i++) {
>                 struct cache_entry *ce = active_cache[i];

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?  If
it doesn't imply that, wouldn't users want an extra option to be able
to behave that way?  And if we add an option, why are we adding a
special option for people wanting to behave sparsely in a
sparse-index-cone-mode-sparse-checkout rather than for the special
case of folks wanting to behave densely in such a setup?

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.

However, on a tangent, that S_ISLNK looks rather suspicious to me.
Why would we renormalize symlinks?  I mean, sure, symlinks aren't
likely to have CRLF/LF characters in them, but if they did, wouldn't
it be wrong to renormalize them?



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux