git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs

From
Emily Shaffer <emilyshaffer@google.com>
Date
Nov 4, 2021, 23:22 UTC
Message-ID
<YYRrH6Hw/C1qj5MV@google.com>
In-Reply-To
<b208eb06-21c7-2199-0bc8-597669a165a1@gmail.com>
On Mon, Oct 25, 2021 at 12:14:07PM -0400, Derrick Stolee wrote:
Show 57 quoted lines
> 
> 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
Previous: Derrick StoleeNext: Derrick Stolee
Message 7 of 31 in “cache parent project's gitdir in submodules”
  1. 0/4 cache parent project's gitdir in submodulesEmily Shaffer, Oct 14, 2021
  2. 1/4 t7400-submodule-basic: modernize inspect() helperEmily Shaffer, Oct 14, 2021
  3. 2/4 introduce submodule.superprojectGitDir recordEmily Shaffer, Oct 14, 2021
  4. 3/4 submodule: record superproject gitdir during absorbgitdirsEmily Shaffer, Oct 14, 2021
  5. Jonathan TanOct 18, 2021
  6. Derrick StoleeOct 25, 2021
  7. Emily ShafferNov 4, 2021
  8. Derrick StoleeNov 8, 2021
  9. Emily ShafferNov 4, 2021
  10. 4/4 submodule: record superproject gitdir during 'update'Emily Shaffer, Oct 14, 2021
  11. Derrick StoleeOct 25, 2021
  12. Derrick StoleeOct 25, 2021
  13. 0/4 cache parent project's gitdir in submodulesEmily Shaffer, Nov 4, 2021
  14. 1/4 t7400-submodule-basic: modernize inspect() helperEmily Shaffer, Nov 4, 2021
  15. 2/4 introduce submodule.superprojectGitDir recordEmily Shaffer, Nov 4, 2021
  16. Junio C HamanoNov 5, 2021
  17. Emily ShafferNov 8, 2021
  18. 3/4 submodule: record superproject gitdir during absorbgitdirsEmily Shaffer, Nov 4, 2021
  19. 4/4 submodule: record superproject gitdir during 'update'Emily Shaffer, Nov 4, 2021
  20. Junio C HamanoNov 5, 2021
  21. Ævar Arnfjörð BjarmasonNov 5, 2021
  22. Emily ShafferNov 8, 2021
  23. Ævar Arnfjörð BjarmasonNov 9, 2021
  24. Emily ShafferNov 9, 2021
  25. Emily ShafferNov 9, 2021
  26. Ævar Arnfjörð BjarmasonNov 5, 2021
  27. Emily ShafferNov 8, 2021
  28. Ævar Arnfjörð BjarmasonNov 9, 2021
  29. Derrick StoleeNov 8, 2021
  30. Emily ShafferNov 8, 2021
  31. Derrick StoleeNov 9, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.