Re: [PATCH 9/9] builtin/fetch: check for submodule updates for non branch fetches

[Jonathan Tan]

> @@ -887,11 +887,14 @@ static int store_updated_refs(const char *raw_url, 
> const char *remote_name,
>                               rc |= update_local_ref(ref, what, rm, &note,
>                                                      summary_width);
>                               free(ref);
> -                     } else
> +                     } else {
> +                             check_for_new_submodule_commits(&rm->old_oid);

Does this need to be guarded with a recurse_submodules check, just like
in update_local_ref()?

Also, this warrants a comment - this is here because there is some code
later that requires the new submodule commits to be registered, and the
other branch does not require it only because update_local_ref() calls
it.

> @@ -615,7 +615,7 @@ test_expect_success "fetch new commits on-demand when 
> they are not reachable" '
>       git update-ref refs/changes/2 $D &&
>       (
>               cd downstream &&
> -             git fetch --recurse-submodules --recurse-submodules-default 
> on-demand origin refs/changes/2:refs/heads/my_branch &&
> +             git fetch --recurse-submodules origin refs/changes/2 &&
>               git -C submodule cat-file -t $C &&
>               git checkout --recurse-submodules FETCH_HEAD
>       )

I think there should be a new test - we can tell from the code that just
because fetching to FETCH_HEAD works doesn't mean that fetching to a ref
works, and vice versa.

Also, can you make the test fetch 2 refs? So that we know that it works
with more than one.