On Mon, Oct 25, 2021 at 12:14:07PM -0400, Derrick Stolee wrote: > > On 10/18/2021 7:18 PM, Jonathan Tan wrote: > >> Already during 'git submodule add' we record a pointer to the > >> superproject's gitdir. However, this doesn't help brand-new > >> submodules created with 'git init' and later absorbed with 'git > >> submodule absorbgitdir'. Let's start adding that pointer during 'git > >> submodule absorbgitdir' too. > > > > s/absorbgitdir/absorbgitdirs/ (note the "s" at the end) > > > >> @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path) > >> > >> relocate_gitdir(path, real_old_git_dir, real_new_git_dir); > >> > >> + /* cache pointer to superproject's gitdir */ > >> + /* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */ > >> + strbuf_addf(&config_path, "%s/config", real_new_git_dir); > >> + git_config_set_in_file(config_path.buf, "submodule.superprojectGitdir", > >> + relative_path(absolute_path(get_git_dir()), > >> + real_new_git_dir, &sb)); > >> + > >> + strbuf_release(&config_path); > >> + strbuf_release(&sb); > >> free(old_git_dir); > >> free(real_old_git_dir); > >> free(real_new_git_dir); > > > > Here [1] you mention that you'll delete the NEEDSWORK, but it's still > > there. > > > > Having said that, it might be better to make a test in which we call > > this command while in a worktree of a superproject. The test might > > reveal that (as pointed out to me internally) you might need to use the > > common dir functions instead of the git dir functions to point to the > > directory that you want (git-worktree.txt distinguishes the 2 if you > > search for GIT_COMMON_DIR). > > I came here to say the same thing. It's a bit too direct to compute > the location of a config file this way, so we should expose a method > that can create one for a given Git directory. > > Since you're setting this config value inside the submodule's config, > what does it mean for a submodule to also be a worktree (and hence > require config.worktree)? What happens in this rough scenario? > > 1. git init sub > 2. git init super > 3. git -C sub worktree add super/sub > 4. git -C super submodule absorbgitdir sub > > I haven't actually tried running these things, but it seems unusual > and unexpected. This doesn't even account for cases where the repo > root and a worktree are both submodules within the superproject. > > If we already have protections preventing these worktrees as > submodules, then perhaps there is no need for work here. I'm not > familiar enough with the area to make a claim one way or another. Yeah, I think there is actually a test case covering this in t7412: 137 test_expect_success 'setup a submodule with multiple worktrees' ' 138 # first create another unembedded git dir in a new submodule 139 git init sub3 && 140 test_commit -C sub3 first && 141 git submodule add ./sub3 && 142 test_tick && 143 git commit -m "add another submodule" && 144 git -C sub3 worktree add ../sub3_second_work_tree 145 ' 146 147 test_expect_success 'absorbing fails for a submodule with multiple worktrees' ' 148 test_must_fail git submodule absorbgitdirs sub3 2>error && 149 test_i18ngrep "not supported" error 150 ' That is, I think because 'sub/' in your scenario above has multiple worktrees, the absorbgitdirs will fail. So I won't do additional work here. Thanks. - Emily > > Thanks, > -Stolee
