Re: [PATCH v4] clone: report duplicate entries on case-insensitive filesystems

From: Duy Nguyen <pclouds@gmail.com>
Date: Aug 15, 2018, 19:35 UTC
Message-ID: <CACsJy8AYQL3oDLyt14eJ1emynngqKQv9GXju56gU9u4mHrFHOg@mail.gmail.com>
In-Reply-To: <20180815190816.GA26521@tor.lan>

On Wed, Aug 15, 2018 at 9:08 PM Torsten Bögershausen <tboegi@web.de> wrote:

Show 13 quoted lines

> > +#if !defined(GIT_WINDOWS_NATIVE) /* inode is always zero on Windows */
> > +     for (i = 0; i < state->istate->cache_nr; i++) {
> > +             struct cache_entry *dup = state->istate->cache[i];
> > +
> > +             if (dup == ce)
> > +                     break;
> > +
> > +             if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
> > +                     continue;
> > +
>
> Should the following be protected by core.checkstat ?
>         if (check_stat) {

Good catch! st_ino is ignored if core.checkStat is false. I will probably send a separate patch to add more details to config.txt about this key.

Show 11 quoted lines

> > +             if (dup->ce_stat_data.sd_ino == st->st_ino) {
> > +                     dup->ce_flags |= CE_MATCHED;
> > +                     break;
> > +             }
> > +     }
> > +#endif
>
> Another thing is that we switch of the ASCII case-folding-detection-logic
> off for Windows users, even if we otherwise rely on icase.
> I think we can use fspathcmp() as a fallback. when inodes fail,
> because we may be on a network file system.

I admit I did not think about network file system. Will spend some time (and hopefully not on nfs kernel code) on it.

For falling back on fspathcmp even on Windows, is it really safe? I'm on Linux and never have to deal with this issue to have any experience. It does sound good though because it should be a subset for any "weird" filesystems out there.

Show 35 quoted lines

> (I don't have a test setup at the moment, but what happens with inodes
> when a Windows machine exports a share to Linux or Mac ?)
>
> Is there a chance to get the fspathcmp() back, like this ?
>
> static void mark_colliding_entries(const struct checkout *state,
>                                    struct cache_entry *ce, struct stat *st)
> {
>         int i;
>         ce->ce_flags |= CE_MATCHED;
>
>         for (i = 0; i < state->istate->cache_nr; i++) {
>                 struct cache_entry *dup = state->istate->cache[i];
>                 int folded = 0;
>
>                 if (dup == ce)
>                         break;
>
>                 if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
>                         continue;
>
>                 if (!fspathcmp(dup->name, ce->name))
>                         folded = 1;
>
> #if !defined(GIT_WINDOWS_NATIVE) /* inode is always zero on Windows */
>                 if (check_stat && (dup->ce_stat_data.sd_ino == st->st_ino))
>                         folded = 1;
> #endif
>                 if (folded) {
>                         dup->ce_flags |= CE_MATCHED;
>                         break;
>                 }
>         }
> }
>

-- 
Duy