Re: [PATCH] submodule recursion in git-archive

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <l.s.r@web.de> writes:

> Thanks for the patches!  Please send only one per message (the second
> one as a reply to the first one, or both as replies to a cover letter),
> though -- that makes commenting on them much easier.
>
> Side note: Documentation/SubmittingPatches doesn't mention that (yet),
> AFAICS.

OK, how about doing this then?

 Documentation/SubmittingPatches | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 7055576..304b3c0 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -140,7 +140,12 @@ comment on the changes you are submitting.  It is important for
 a developer to be able to "quote" your changes, using standard
 e-mail tools, so that they may comment on specific portions of
 your code.  For this reason, all patches should be submitted
-"inline".  If your log message (including your name on the
+"inline".  A patch series that consists of N commits is sent as N
+separate e-mail messages, or a cover letter message (see below) with
+N separate e-mail messages, each being a response to the cover
+letter.
+
+If your log message (including your name on the
 Signed-off-by line) is not writable in ASCII, make sure that
 you send off a message in the correct encoding.
 

>> The feature is disabled for remote repositories as
>> the git_work_tree fails. This is a possible future
>> enhancement.
>
> Hmm, curious.  Why does it fail?  I guess that happens with bare
> repositories, only, right?  (Which are the most likely kind of remote
> repos to encounter, of course.)

Yeah, I do not think of a reason why it should fail in a bare
repository, either. "git archive" is about writing out the contents
of an already recorded tree, so there shouldn't be a reason to even
call get_git_work_tree() in the first place.

Even if the code is run inside a repository with a working tree,
when producing a tarball out of an ancient commit that had a
submodule not at its current location, --recurse-submodules option
should do the right thing, so asking for working tree location of
that submodule to find its repository is wrong, I think.  It may
happen to find one if the archived revision is close enough to what
is currently checked out, but that may not necessarily be the case.

At that point when the code discovers an S_ISGITLINK entry, it
should have both a pathname to the submodule relative to the
toplevel and the commit object name bound to that submodule
location.  What it should do, when it does not find the repository
at the given path (maybe because there is no working tree, or the
sudmodule directory has moved over time) is roughly:

 - Read from .gitmodules at the top-level from the tree it is
   creating the tarball out of;

 - Find "submodule.$name.path" entry that records that path to the
   submodule; and then

 - Using that $name, find the stashed-away location of the submodule
   repository in $GIT_DIR/modules/$name.

or something like that.

This is a related tangent, but when used in a repository that people
often use as their remote, the repository discovery may have to
interact with the relative URL.  People often ship .gitmodules with

	[submodule "bar"]
        	URL = ../bar.git
		path = barDir

for a top-level project "foo" that can be cloned thusly:

	git clone git://site.xz/foo.git

and host bar.git to be clonable with

	git clone git://site.xz/bar.git barDir/

inside the working tree of the foo project.  In such a case, when
"archive --recurse-submodules" is running, it would find the
repository for the "bar" submodule at "../bar.git", I would think.

So this part needs a bit more thought, I am afraid.

>>  'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]
>>  	      [-o <file> | --output=<file>] [--worktree-attributes]
>> +	      [--recursive|--recurse-submodules]
>
> I'd expect git archive --recurse to add subdirectories and their
> contents, which it does right now, and --no-recurse to only archive the
> specified objects, which is not implemented.  IAW: I wouldn't normally
> associate an option with that name with submodules.  Would
> --recurse-submodules alone suffice?

Jens already commented on this, and I agree that --recursive should
be dropped from this patch.