Thread (105 messages) 105 messages, 4 authors, 2022-03-08

Re: [PATCH v2 7/9] fetch: fetch unpopulated, changed submodules

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-02-16 09:44:19

On Wed, Feb 16 2022, Glen Choo wrote:
Glen Choo [off-list ref] writes:
quoted
Jonathan Tan [off-list ref] writes:
quoted
Glen Choo [off-list ref] writes:
quoted
+	# Create new superproject commit with updated submodules
+	add_upstream_commit &&
+	(
+		cd submodule &&
+		(
+			cd subdir/deepsubmodule &&
+			git fetch &&
+			git checkout -q FETCH_HEAD
+		) &&
+		git add subdir/deepsubmodule &&
+		git commit -m "new deep submodule"
+	) &&
+	git add submodule &&
+	git commit -m "new submodule" &&
I thought add_upstream_commit() would do this, but apparently it just
adds commits to the submodules (which works for the earlier tests that
just tested that the submodules were fetched, but not for this one). I
think that all this should go into its own function.
I'm testing out a function that does exactly what these lines do, i.e.
create a superproject commit containing a submodule change containing a
deepsubmodule change. That works pretty well and it makes sense in the
context of the tests.
quoted
quoted
Also, I understand that "git fetch" is there to pick up the commit we
made in add_upstream_commit, but this indirection is unnecessary in a
test, I think. Can we not use add_upstream_commit and just create our
own in subdir/deepsubmodule? (This might conflict with subsequent tests
that use the old scheme, but I think that it should be fine.)
We can avoid the "git fetch" if we first need to fix an inconsistency in
how the submodules are set up. Right now, we have:

  test_expect_success setup '
    mkdir deepsubmodule &&
    [...]
    mkdir submodule &&
    (
    [...]
      git submodule add "$pwd/deepsubmodule" subdir/deepsubmodule &&
      git commit -a -m new &&
      git branch -M sub
    ) &&
    git submodule add "$pwd/submodule" submodule &&
    [...]
    (
      cd downstream &&
      git submodule update --init --recursive
    )
  '

resulting in a directory structure like:

$pwd
|_submodule
  |_subdir
    |_deepsubmodule
|_deepsubmodule

and upstream/downstream dependencies like:

upstream                             downstream            
--------                             ----------
$pwd/deepsubmodule                   $pwd/downstream/submodule/subdir/deepsubmodule (SUT)
                                     $pwd/submodule/subdir/deepsubmodule


So we can't create the commit in submodule/subdir/deepsubmodule, because
that's not where our SUT would fetch from.

Instead, we could fix this by having a more consistent
upstream/downstream structure:

$pwd
|_submodule
  |_subdir
    |_deepsubmodule

upstream                             downstream            
--------                             ----------
$pwd/submodule/subdir/deepsubmodule  $pwd/downstream/submodule/subdir/deepsubmodule (SUT)

This seems more convenient to test, but before I commit to this, is
there a downside to this that I'm not seeing?
Won't this sort of arrangement create N copies of e.g. a zlib.git or
some other common library that might be used by N submodules.

But I haven't read all the context, I'm assuming you're talking about
how we store in-tree a/b and x/y/b submodules now, we store those in
.git/ both as .git/modules/b.git or whatever? I can't remember ... :)

Whatever we do now I do think the caveat I've noted above is interesting
when it comes to submodule design, e.g. if both git.git and
some-random-thing.git both bring in the same sha1collisiondetection.git
from the same github URL should those be the same in our underlying
storage?

I think the answer to that would ideally be both "yes" and
"no".

I.e. "yes" because it's surely handy for "git fetch", now you don't need to
fetch the same stuff twice in the common case of just updating all our
recursive submodules.

And also "no" because maybe some users would really consider them
different. E.g. the you may want to "cd git/" and adjust the git.git one
and create a branch there for some hotfix it needs, which would not be
needed/wanted by some-random-thing.git.

Hrm...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help