Re: [PATCH v5] submodule: fetch missing objects from default remote

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Mar 3, 2026, 20:47 UTC
Message-ID: <2e62dc94-b821-4815-8dd2-f806580d2027@ramsayjones.plus.com>
In-Reply-To: <20260303200906.4118348-1-nasser.grainawi@oss.qualcomm.com>

On 03/03/2026 8:09 pm, Nasser Grainawi wrote:

Show 19 quoted lines

> When be76c21282 (fetch: ensure submodule objects fetched, 2018-12-06)
> added support for fetching a missing submodule object by id, it
> hardcoded the remote name as "origin" and deferred anything more
> complicated for a later patch. Implement the NEEDSWORK item to remove
> the hardcoded assumption by adding and using a submodule helper subcmd
> 'get-default-remote'. Fixing this lets 'git fetch --recurse-submodules'
> succeed when the fetched commit(s) in the superproject trigger a
> submodule fetch, and that submodule's default remote name is not
> "origin".
> 
> Add non-"origin" remote tests to t5526-fetch-submodules.sh and
> t5572-pull-submodule.sh demonstrating this works as expected and add
> dedicated tests for get-default-remote.
> 
> Signed-off-by: Nasser Grainawi <nasser.grainawi@oss.qualcomm.com>
> Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> Fixes for test_when_finished usage within a subshell.
>

The 't5526-fetch-submodules.sh' test failed for me tonight. Having seen an earlier email about the test_when_finished failures I wasn't too surprised but, once I looked at the failure, it was obviously not the cause of this failure. Indeed, when I ran the test by hand, it passed ... :)

It seems the 'seen' branch (@764d09c9ce) has this v5 version of the patch and (in this test anyway) has some flakiness:

  $ cd t
  $ ./t5526-fetch-submodules.sh --stress >out 2>&1
  $ cat out
  FAIL  4.1
  FAIL  3.1
  FAIL  5.1
  OK    0.1 
  OK    1.1
  OK    6.1 
  OK    7.1
  OK    2.1 
  Log(s) of failed test run(s):
  Contents of '/home/ramsay/git/t/test-results/t5526-fetch-submodules.stress-3.out':
  Initialized empty Git repository in /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-3/.git/
  expecting success of 5526.1 'setup':

...

  expecting success of 5526.44 'fetch new submodule commits on-demand outside standard refspec with custom remote name':

...

  + diff -u expect_fetch_custom actual_fetch_warnings_removed
  --- expect_fetch_custom 2026-03-03 20:35:13.949600802 +0000
  +++ actual_fetch_warnings_removed       2026-03-03 20:35:14.150601532 +0000
  @@ -4,9 +4,9 @@
   Fetching submodule sub1/subdir/deepsubmodule
   Fetching submodule submodule
   Fetching submodule submodule/subdir/deepsubmodule
  -From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
  - * branch            43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
  -Fetching submodule sub1/subdir/deepsubmodule
   From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/submodule
    * branch            e38933e027ee8a2000f603124aa899302a09a51f -> FETCH_HEAD
   Fetching submodule submodule/subdir/deepsubmodule
  +From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
  + * branch            43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
  +Fetching submodule sub1/subdir/deepsubmodule
  error: last command exited with $?=1
  not ok 44 - fetch new submodule commits on-demand outside standard refspec with custom remote name

...

From which I guess that the order of the output is somewhat unpredictable.

Also, other test files in that patch didn't fail for me with 'make test', but it could be possible that they are also flaky. I didn't look.

Thanks.

ATB, Ramsay Jones