git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 8/9] archive: add --recurse-submodules to git-archive command

From
Glen Choo <chooglen@google.com>
Date
Oct 27, 2022, 17:30 UTC
Message-ID
<kl6lbkpxqim1.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To
<fa90e7cf-4ddc-573a-1859-9e2b6b203f95@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 48 quoted lines
> Am 27.10.22 um 01:34 schrieb Glen Choo:
>> "Heather Lapointe via GitGitGadget" <gitgitgadget@gmail.com> 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.

Show 6 quoted lines
>
> 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é
Previous: Glen ChooNext: Glen Choo
Message 38 of 48 in “archive: add --recurse-submodules to git-archive command”
  1. archive: add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 12, 2022
  2. 0/2 archive: Add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 13, 2022
  3. 2/2 archive: fix a case of submodule in submodule traversalAlphadelta14 via GitGitGadget, Oct 13, 2022
  4. 1/2 archive: add --recurse-submodules to git-archive commandAlphadelta14 via GitGitGadget, Oct 13, 2022
  5. René ScharfeOct 13, 2022
  6. Heather LapointeOct 13, 2022
  7. René ScharfeOct 13, 2022
  8. Heather LapointeOct 13, 2022
  9. René ScharfeOct 14, 2022
  10. 0/9 archive: Add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 17, 2022
  11. 1/9 tree: do not use the_repository for tree traversal methods.Alphadelta14 via GitGitGadget, Oct 17, 2022
  12. Junio C HamanoOct 17, 2022
  13. Glen ChooOct 26, 2022
  14. Jonathan TanOct 27, 2022
  15. Junio C HamanoOct 27, 2022
  16. 2/9 tree: update cases to use repo_ tree methodsHeather Lapointe via GitGitGadget, Oct 17, 2022
  17. 3/9 tree: increase test coverage for tree.cHeather Lapointe via GitGitGadget, Oct 17, 2022
  18. Phillip WoodOct 17, 2022
  19. Junio C HamanoOct 17, 2022
  20. Jonathan TanOct 27, 2022
  21. 4/9 tree: handle submodule case for read_tree_at properlyHeather Lapointe via GitGitGadget, Oct 17, 2022
  22. Phillip WoodOct 17, 2022
  23. Junio C HamanoOct 17, 2022
  24. Glen ChooOct 26, 2022
  25. Jonathan TanOct 27, 2022
  26. 5/9 tree: add repository parameter to read_tree_fn_tHeather Lapointe via GitGitGadget, Oct 17, 2022
  27. 6/9 archive: pass repo objects to write_archive handlersHeather Lapointe via GitGitGadget, Oct 17, 2022
  28. Phillip WoodOct 17, 2022
  29. 7/9 archive: remove global repository from archive_argsHeather Lapointe via GitGitGadget, Oct 17, 2022
  30. 9/9 archive: add tests for git archive --recurse-submodulesHeather Lapointe via GitGitGadget, Oct 17, 2022
  31. Jonathan TanOct 27, 2022
  32. Glen ChooOct 27, 2022
  33. Ævar Arnfjörð BjarmasonOct 28, 2022
  34. 8/9 archive: add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 17, 2022
  35. Glen ChooOct 26, 2022
  36. René ScharfeOct 27, 2022
  37. Glen ChooOct 27, 2022
  38. Glen ChooOct 27, 2022
  39. Glen ChooOct 27, 2022
  40. Phillip WoodOct 17, 2022
  41. Junio C HamanoOct 18, 2022
  42. Heather LapointeOct 18, 2022
  43. Junio C HamanoOct 19, 2022
  44. Junio C HamanoOct 19, 2022
  45. Junio C HamanoOct 20, 2022
  46. Junio C HamanoOct 21, 2022
  47. Glen ChooOct 26, 2022
  48. Heather LapointeOct 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.