| Message ID | 942d3ce2d3cf96192c7e9d5860a18c333dd08acf.1631123754.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | More add_submodule_odb() cleanup in merge code | expand |
> 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. Upon further work, there seems to be quite a lot of test code to migrate, and it might be easier instead to add support for unabsorbed submodules to repo_submodule_init() (say, following what open_submodule() in submodule.c does). Feel free to comment on this or other aspects of the patch set, and in the meantime, I'll investigate that.
Jonathan Tan <jonathantanmy@google.com> writes: > 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?
> 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. Thanks for taking a look. I took a further look and it doesn't look too difficult to teach repo_submodule_init() to work on both forms (without calling "absorbgitdirs") - I'll send an updated patch set soon. As for "git submodule add" transitioning it into the absorbed form, I'll leave that for someone else to do - although if repo_submodule_init() supports the unabsorbed form now, that is not so urgent to do, I think.
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index e5e89c2045..8efce86b42 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -27,7 +27,8 @@ test_expect_success setup ' git add file && test_tick && git commit -m sub-root) && - git add sub && + git submodule add ./sub sub && + git submodule absorbgitdirs && test_tick && git commit -m root && @@ -82,7 +83,8 @@ test_expect_success 'setup for merge search' ' git branch sub-a) && git commit --allow-empty -m init && git branch init && - git add sub && + git submodule add ./sub sub && + git submodule absorbgitdirs && git commit -m "a" && git branch a && @@ -112,7 +114,8 @@ test_expect_success 'setup for merge search' ' git checkout -b g init && (cd sub && git checkout -b sub-g sub-c) && - git add sub && + git submodule add ./sub sub && + git submodule absorbgitdirs && git commit -a -m "g") '
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. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- t/t6437-submodule-merge.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)