René Scharfe <l.s.r@xxxxxx> writes:
> Am 27.10.22 um 01:34 schrieb Glen Choo:
>> "Heather Lapointe via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>>
>>> index 34549d849f1..f81ef741487 100644
>>> --- a/archive.c
>>> +++ b/archive.c
>>> @@ -213,6 +214,25 @@ static void queue_directory(const struct object_id *oid,
>>> oidcpy(&d->oid, oid);
>>> }
>>>
>>> +static void queue_submodule(
>>> + struct repository *superproject,
>>> + const struct object_id *oid,
>>> + struct strbuf *base, const char *filename,
>>> + unsigned mode, struct archiver_context *c)
>>> +{
>>> + struct repository subrepo;
>>> +
>>> + if (repo_submodule_init(&subrepo, superproject, filename, null_oid()))
>>> + return;
>>> +
>>> + if (repo_read_index(&subrepo) < 0)
>>> + die("index file corrupt");
>>> +
>>> + queue_directory(oid, base, filename, mode, c);
>>> +
>>> + repo_clear(&subrepo);
>>> +}
>>> +
>
>> What's much more surprising is that you can delete the entire function
>> body (even queue_directory()!) and the tests still pass! The tests are
>> definitely testing what they say they are (I've also checked the
>> tarballs), so I'm not sure what's going on.
>>
>> I commented out queue_directory() in the S_ISDIR case, and the only test
>> failures I saw were:
>>
>> - t5000.68, which uses a glob in its pathspec. I tried using a glob for
>> in the archive submodule tests, but I couldn't reproduce the failure.
>> - t5004.11, which is a really big test case that I didn't bother looking
>> deeply into.
>>
>> So I'm at a loss as to what queue_directory() actually does.
> An archive doesn't strictly need directory entries. If it contains a
> file with a deeply nested path then extractors will create the parent
> directory hierarchy regardless. diff(1) won't notice any difference.
> Directory entries are mainly included to specify the permission bits.
Thanks. In that case, we should probably also test the case where there
are empty directories (e.g. when a file is excluded by a pathspec), and
we should also check the permission bits.
>
> t5000.68 checks for the directory entries in the output given by the
> option --verbose of git archive. t5004.11 checks the number of archive
> entries (including directories) using "zipinfo -h".
>
> René
