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

[Glen Choo <chooglen@google.com>]

Glen Choo <chooglen@google.com> writes:

> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Glen Choo <chooglen@google.com> writes:
>>> +	# 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.

>> 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?