On 3/17/2021 3:55 PM, Derrick Stolee wrote:
> On 3/17/2021 9:43 AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, Mar 16 2021, Derrick Stolee via GitGitGadget wrote:
>>> @@ -251,6 +251,8 @@ static inline unsigned int create_ce_mode(unsigned int mode)
>>> {
>>> if (S_ISLNK(mode))
>>> return S_IFLNK;
>>> + if (mode == S_IFDIR)
>>> + return S_IFDIR;
>>
>> Does this actually need to be mode == S_IFDIR v.s. S_ISDIR(mode)? Those
>> aren't the same thing...
>>
>>> if (S_ISDIR(mode) || S_ISGITLINK(mode))
>>> return S_IFGITLINK;
>>
>> ...and if it can be S_ISDIR(mode) then this becomes just
>> S_ISGITLINK(mode), but losing the "if" there makes me suspect that some
>> dir == submodule heuristic is being broken somewhere..
>
> I have a vague recollection that I did that at one point, and
> it didn't work. However, using the simpler
>
> if (S_ISDIR(mode))
> return S_IFDIR;
> if (S_ISGITLINK(mode))
> return S_IFGITLINK;
>
> passes all of my tests.
I'm not sure why it was passing yesterday (maybe I was in the
wrong worktree) but I _do_ get failures, such as this one in t2105:
expecting success of 2105.4 'add gitlink to relative .git file':
git update-index --add -- sub2
+ git update-index --add -- sub2
warning: index entry is a directory, but not sparse (00000000)
error: Could not read 50e526bb426771f6036ad3a8b0c81d511d91fc2a
BUG: read-cache.c:324: unsupported ce_mode: 40000
Aborted (core dumped)
error: last command exited with $?=134
not ok 4 - add gitlink to relative .git file
#
# git update-index --add -- sub2
#
In this case, the mode that is specified is equal to 040775,
so we need to use the permission bits outside of __S_IFMT
(0170000) to determine if this is a sparse directory or a
submodule entry. Submodules will never be sparse, so
permissions matter. Sparse directories never actually exist,
so permissions don't matter.
Playing around with it, I still only see the exact equality
as working for me.
I can, however, use this format for these if statements:
if (S_ISSPARSEDIR(mode))
return S_IFDIR;
if (S_ISDIR(mode) || S_ISGITLINK(mode))
return S_IFGITLINK;
The S_ISSPARSEDIR macro expands to the exact equality.
Now, if we intended to make this work differently, then a
change would be required to construct_sparse_dir_entry()
in sparse-index.c:
static struct cache_entry *construct_sparse_dir_entry(
struct index_state *istate,
const char *sparse_dir,
struct cache_tree *tree)
{
struct cache_entry *de;
de = make_cache_entry(istate, S_IFDIR, &tree->oid, sparse_dir, 0, 0);
de->ce_flags |= CE_SKIP_WORKTREE;
return de;
}
For instance, we could at this point assign de->ce_mode to
be S_IFDIR directly. It seems like the wrong place to do that
to me, but I'm open to suggestions.
Thanks,
-Stolee
