Re: [PATCH 1/3] t6437: run absorbgitdirs on repos

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 8, 2021, 22:20 UTC
Message-ID: <xmqqwnnq1zh9.fsf@gitster.g>
In-Reply-To: <942d3ce2d3cf96192c7e9d5860a18c333dd08acf.1631123754.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 6 quoted lines

> The submodule merge code is being transitioned away from
> add_submodule_odb() to repo_submodule_init(), and the latter does not
> support submodules that have their .git directories in the worktree
> (instead of in .git/modules). Migrate the test code by calling
> absorbgitdirs wherever necessary to place the .git directories of
> submodules in .git/modules of the superproject.

It is wonderful that we can work with the "absorbed" form of the submodule repositories, and having all the submodules in "absorbed" form may be ideal in the longer term, but I fail to convince myself that this particular change to test script is a good idea.

After all, if you are not cloning from somebody else but creating a new submodule in a superproject you have (whether that was locally created or cloned from elsewhere), wouldn't it more natural to have a submodule that is not "absorbed" there? Is it easy to create an "absorbed" submodule locally?

The change to tests in this patch may make things work with the repo_submodule_init() that is incapable of work with submodules with in-worktree .git repository. In theory, the users can do the same, i.e. have an extra "git submodule absorbgitdirs" call, when adding a new submodule, to make things work with repo_submodule_init(), too.

But that sounds like a workaround, not a feature.

Quite honestly, if repo_submodule_init() can only work with the "absorbed" form, isn't it simply a bug?

Two independent (i.e. we can do either or both) things may improve the situation:

 - allow repo_submodule_init() to work on both forms (in place---no
   cheating by calling "absorb" behind user's back).

 - teach "git submodule add" to transition an old-style submodule
   into the "absorbed" form (either with an option, or do so by
   default with an escape hatch to disable).

The latter indirectly attacks the "repo_submodule_init() can only work with absorbed form" issue by making it harder to create a non-absorbed submodule to begin with.

Hmm?