Re: [PATCH 5/6] t5526: break test submodule differently

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Jan 9, 2024 at 7:17 AM Patrick Steinhardt <ps@pks.im> wrote:
> In 10f5c52656 (submodule: avoid auto-discovery in
> prepare_submodule_repo_env(), 2016-09-01) we fixed a bug when doing a
> recursive fetch with submodule in the case where the submodule is broken
> due to whatever reason. The test to exercise that the fix works breaks
> the submodule by deleting its `HEAD` reference, which will cause us to
> not detect the directory as a Git repository.
>
> While this is perfectly fine in theory, this way of breaking the repo
> becomes problematic with the current efforts to introduce another refdb
> backend into Git. The new reftable backend has a stub HEAD file that
> always contains "ref: refs/heads/.invalid" so that tools continue to be
> able to detect such a repository. But as the reftable backend will never
> delete this file even when asked to delete `HEAD` the current way to
> delete the `HEAD` reference will stop working.

This patch is not the appropriate place to bikeshed (but since this is
the first time I've read or paid attention to it), if I saw "ref:
refs/heads/.invalid", the word ".invalid" would make me think
something was broken in my repository. Would it make sense to use some
less alarming word, such as perhaps ".placeholder", ".stand-in",
".synthesized" or even the name of the non-file-based backend in use?

> Adapt the code to instead delete the objects database. Going back with
> this new way to cuase breakage confirms that it triggers the infinite
> recursion just the same, and there are no equivalent ongoing efforts to
> replace the object database with an alternate backend.

s/cuase/cause/

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> @@ -771,7 +771,7 @@ test_expect_success 'fetching submodule into a broken repository' '
>         # Break the receiving submodule
> -       test-tool -C dst/sub ref-store main delete-refs REF_NO_DEREF msg HEAD &&
> +       rm -r dst/sub/.git/objects &&

If there is no guarantee that .git/objects will exist when any
particular backend is in use, would it be more robust to use -f here,
as well?

    rm -rf dst/sub/.git/objects &&