Re: [PATCH v2 0/2] archive: Add --recurse-submodules to git-archive command

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

 



Am 13.10.22 um 13:35 schrieb Heather Lapointe via GitGitGadget:
> This makes it possible to include submodule contents in an archive command.

Great!

> The inspiration for this change comes from this Github thread,
> https://github.com/dear-github/dear-github/issues/214, with at least 160
> 👍🏻 's at the time of writing. (I stumbled upon it because I wanted it as
> well).
>
> I figured the underlying implementation wouldn't be too difficult with most
> of the plumbing already in place, so I decided to add the relevant logic to
> the client git-archive command.
>
> One of the trickier parts of this implementation involved teaching read_tree
> about submodules. Some of the troublesome areas were still using the
> the_repository references to look up commit or tree or oid information. I
> ended up deciding that read_tree_fn_t would probably be best off having a
> concrete repo reference since it allows changing the context to a subrepo
> where needed (even though some of the usages did not need it specifically).
>
> I am open to feedback since this is all quite new to me :)
>
> TODO:

This list confuses me:

>  * working implementation

What exactly is not working, yet?

>  * valgrind

What's up with it?  Does is report errors or leaks?

>  * add regression tests

This series adds a new test script.  Do you plan to add more checks?

>  * update documentation with new flag

That I can understand: Indeed Documentation/git-archive.txt would need
an update.

>  * submit to mailing list

But you already did submit two iterations of this series to the Git
mailing list!?

>
> Alphadelta14 (2):
>   archive: add --recurse-submodules to git-archive command
>   archive: fix a case of submodule in submodule traversal

We prefer to keep known bugs out of the repo.  It helps when bisecting,
for example.  So it would be better to squash the fix into the patch
that adds the feature.  But...

>  archive-tar.c                 | 14 +++--
>  archive-zip.c                 | 14 ++---
>  archive.c                     | 99 ++++++++++++++++++++++++-----------
>  archive.h                     |  8 +--
>  builtin/checkout.c            |  2 +-
>  builtin/log.c                 |  2 +-
>  builtin/ls-files.c            | 10 ++--
>  builtin/ls-tree.c             | 16 +++---
>  list-objects.c                |  2 +-
>  merge-recursive.c             |  2 +-
>  revision.c                    |  4 +-
>  sparse-index.c                |  2 +-
>  t/t5005-archive-submodules.sh | 84 +++++++++++++++++++++++++++++
>  tree.c                        | 93 ++++++++++++++++++++++----------
>  tree.h                        | 11 ++--
>  wt-status.c                   |  2 +-
>  16 files changed, 269 insertions(+), 96 deletions(-)
>  create mode 100755 t/t5005-archive-submodules.sh

... this is all a bit much for a single patch, I feel.  Giving
parse_tree_gently() a repo parameter, adding repo_parse_tree(), using
it in read_tree_at(), adding a repo parameter to read_tree_fn_t,
letting read_tree_at() recurse into submodules and adding the new
option to git archive all seem like topics worth their own patch and
rationale.

You probably have all of that in your head right now, but at least my
attention span and working memory capacity requires smaller morsels.

>
>
> base-commit: e85701b4af5b7c2a9f3a1b07858703318dce365d
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1359%2FAlphadelta14%2Farchive-recurse-submodules-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1359/Alphadelta14/archive-recurse-submodules-v2
> Pull-Request: https://github.com/git/git/pull/1359
>
> Range-diff vs v1:
>
>  1:  41664a59029 = 1:  41664a59029 archive: add --recurse-submodules to git-archive command
>  -:  ----------- > 2:  68f7830c6d9 archive: fix a case of submodule in submodule traversal
>





[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