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

[PATCH v4 0/4] cache parent project's gitdir in submodules

From
Emily Shaffer <emilyshaffer@google.com>
Date
Oct 14, 2021, 20:34 UTC
Message-ID
<20211014203416.2802639-1-emilyshaffer@google.com>

For the original cover letter, see https://lore.kernel.org/git/20210611225428.1208973-1-emilyshaffer%40google.com.

The CI run is here: https://github.com/nasamuffin/git/runs/3899227896

It shows some broken Windows tests, but those are broken in the fork-point too: https://github.com/nasamuffin/git/actions/runs/1343120990

Since v3, a pretty major change: the semantics of submodule.superprojectGitDir has changed, to point from the submodule's gitdir to the superproject's gitdir (in v3 and earlier, we kept a path from the submodule's *worktree* to the superproject's gitdir instead). This cleans up some of the confusions about the behavior when a submodule worktree moves around in the superproject's tree, or in a future when we support submodules having multiple worktrees.

I also tried to simplify the tests to use 'test-tool path-utils relative_path' everywhere - I think that makes them much more clear for a test reader, but if you're reviewing and it isn't obvious what we're testing for, please speak up.

I think this is pretty mature and there was a lot of general agreement that the gitdir->gitdir association was the way to go, so please be brutal and look for nits, leaks, etc. this round ;)

 - Emily
Emily Shaffer (4):
  t7400-submodule-basic: modernize inspect() helper
  introduce submodule.superprojectGitDir record
  submodule: record superproject gitdir during absorbgitdirs
  submodule: record superproject gitdir during 'update'
 Documentation/config/submodule.txt | 12 +++++++
 builtin/submodule--helper.c        |  4 +++
 git-submodule.sh                   | 14 ++++++++
 submodule.c                        | 10 ++++++
 t/t7400-submodule-basic.sh         | 54 ++++++++++++++++--------------
 t/t7406-submodule-update.sh        | 12 +++++++
 t/t7412-submodule-absorbgitdirs.sh | 23 +++++++++++--
 7 files changed, 102 insertions(+), 27 deletions(-)
Range-diff against v3:
1:  f1236dc9d7 ! 1:  2ff151aaa2 t7400-submodule-basic: modernize inspect() helper
    @@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submo
     -}
     -
      inspect() {
    - 	dir=$1 &&
    +-	dir=$1 &&
     -	dotdot="${2:-..}" &&
    - 
    +-
     -	(
     -		cd "$dir" &&
     -		listbranches >"$dotdot/heads" &&
    @@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submo
     -		git diff-files --exit-code &&
     -		git clean -n -d -x >"$dotdot/untracked"
     -	)
    -+	git -C "$dir" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&
    -+	{ git -C "$dir" symbolic-ref HEAD || :; } >head &&
    -+	git -C "$dir" rev-parse HEAD >head-sha1 &&
    -+	git -C "$dir" update-index --refresh &&
    -+	git -C "$dir" diff-files --exit-code &&
    -+	git -C "$dir" clean -n -d -x >untracked
    ++	sub_dir=$1 &&
    ++
    ++	git -C "$sub_dir" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&
    ++	{ git -C "$sub_dir" symbolic-ref HEAD || :; } >head &&
    ++	git -C "$sub_dir" rev-parse HEAD >head-sha1 &&
    ++	git -C "$sub_dir" update-index --refresh &&
    ++	git -C "$sub_dir" diff-files --exit-code &&
    ++	git -C "$sub_dir" clean -n -d -x >untracked
      }
      
      test_expect_success 'submodule add' '
2:  2caf9eb8ee ! 2:  ed5479ad5d introduce submodule.superprojectGitDir record
    @@ Commit message
     
         By using a relative path instead of an absolute path, we can move the
         superproject directory around on the filesystem without breaking the
    -    submodule's cache.
    +    submodule's cache. And by using the path from gitdir to gitdir, we can
    +    move the submodule within the superproject's tree structure without
    +    breaking the submodule's cache, too.
     
         Since this hint value is only introduced during new submodule creation
         via `git submodule add`, though, there is more work to do to allow the
    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::
      	clone proceeds as if no alternate was specified.
     +
     +submodule.superprojectGitDir::
    -+	The relative path from the submodule's worktree to its superproject's
    ++	The relative path from the submodule's gitdir to its superproject's
     +	gitdir. When Git is run in a repository, it usually makes no difference
     +	whether this repository is standalone or a submodule, but if this
     +	configuration variable is present, additional behavior may be possible,
    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::
     +	to be present in every submodule, so only optional value-added behavior
     +	should be linked to it. It is set automatically during
     +	submodule creation.
    -++
    -+	Because of this configuration variable, it is forbidden to use the
    -+	same submodule worktree shared by multiple superprojects.
     
      ## builtin/submodule--helper.c ##
    -@@ builtin/submodule--helper.c: static int module_clone(int argc, const char **argv, const char *prefix)
    +@@ builtin/submodule--helper.c: static int clone_submodule(struct module_clone_data *clone_data)
      		git_config_set_in_file(p, "submodule.alternateErrorStrategy",
    - 					   error_strategy);
    + 				       error_strategy);
      
     +	git_config_set_in_file(p, "submodule.superprojectGitdir",
     +			       relative_path(absolute_path(get_git_dir()),
    -+					     path, &sb));
    ++					     sm_gitdir, &sb));
     +
      	free(sm_alternate);
      	free(error_strategy);
      
     
      ## t/t7400-submodule-basic.sh ##
    -@@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submodules to' '
    - submodurl=$(pwd -P)
    +@@ t/t7400-submodule-basic.sh: submodurl=$(pwd -P)
      
      inspect() {
    --	dir=$1 &&
    --
    --	git -C "$dir" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&
    --	{ git -C "$dir" symbolic-ref HEAD || :; } >head &&
    --	git -C "$dir" rev-parse HEAD >head-sha1 &&
    --	git -C "$dir" update-index --refresh &&
    --	git -C "$dir" diff-files --exit-code &&
    --	git -C "$dir" clean -n -d -x >untracked
    -+	sub_dir=$1 &&
    + 	sub_dir=$1 &&
     +	super_dir=$2 &&
    + 
    + 	git -C "$sub_dir" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&
    + 	{ git -C "$sub_dir" symbolic-ref HEAD || :; } >head &&
    + 	git -C "$sub_dir" rev-parse HEAD >head-sha1 &&
    + 	git -C "$sub_dir" update-index --refresh &&
    + 	git -C "$sub_dir" diff-files --exit-code &&
    ++
    ++	# Ensure that submodule.superprojectGitDir contains the path from the
    ++	# submodule's gitdir to the superproject's gitdir.
    ++
    ++	super_abs_gitdir=$(git -C "$super_dir" rev-parse --absolute-git-dir) &&
    ++	sub_abs_gitdir=$(git -C "$sub_dir" rev-parse --absolute-git-dir) &&
    ++
    ++	[ "$(git -C "$sub_dir" config --get submodule.superprojectGitDir)" = \
    ++	  "$(test-tool path-utils relative_path "$super_abs_gitdir" \
    ++						"$sub_abs_gitdir")" ] &&
     +
    -+	git -C "$sub_dir" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&
    -+	{ git -C "$sub_dir" symbolic-ref HEAD || :; } >head &&
    -+	git -C "$sub_dir" rev-parse HEAD >head-sha1 &&
    -+	git -C "$sub_dir" update-index --refresh &&
    -+	git -C "$sub_dir" diff-files --exit-code &&
    -+	cached_super_dir="$(git -C "$sub_dir" config --get submodule.superprojectGitDir)" &&
    -+	[ "$(git -C "$super_dir" rev-parse --absolute-git-dir)" \
    -+		-ef "$sub_dir/$cached_super_dir" ] &&
    -+	git -C "$sub_dir" clean -n -d -x >untracked
    + 	git -C "$sub_dir" clean -n -d -x >untracked
      }
      
    - test_expect_success 'submodule add' '
     @@ t/t7400-submodule-basic.sh: test_expect_success 'submodule add' '
      	) &&
      
3:  d278568a8e ! 3:  60e6cf77c5 submodule: record superproject gitdir during absorbgitdirs
    @@ Commit message
      ## submodule.c ##
     @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *path)
      	char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
    - 	char *new_git_dir;
    + 	struct strbuf new_gitdir = STRBUF_INIT;
      	const struct submodule *sub;
     +	struct strbuf config_path = STRBUF_INIT, sb = STRBUF_INIT;
      
    @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *p
     +	/* 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(get_super_prefix_or_empty(),
    -+					     path, &sb));
    ++			       relative_path(absolute_path(get_git_dir()),
    ++					     real_new_git_dir, &sb));
     +
     +	strbuf_release(&config_path);
     +	strbuf_release(&sb);
    @@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir' '
     +	test_cmp expect.2 actual.2 &&
     +
     +	# make sure the submodule cached the superproject gitdir correctly
    -+	test-tool path-utils real_path . >expect &&
    -+	test-tool path-utils real_path \
    -+		"$(git -C sub1 config submodule.superprojectGitDir)" >actual &&
    ++	submodule_gitdir="$(git -C sub1 rev-parse --absolute-git-dir)" &&
    ++	superproject_gitdir="$(git rev-parse --absolute-git-dir)" &&
    ++
    ++	test-tool path-utils relative_path "$superproject_gitdir" \
    ++		"$submodule_gitdir" >expect &&
    ++	git -C sub1 config submodule.superprojectGitDir >actual &&
     +
     +	test_cmp expect actual
      '
      
      test_expect_success 'absorbing does not fail for deinitialized submodules' '
    +@@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir in a nested submodule' '
    + 	git status >actual.1 &&
    + 	git -C sub1/nested rev-parse HEAD >actual.2 &&
    + 	test_cmp expect.1 actual.1 &&
    +-	test_cmp expect.2 actual.2
    ++	test_cmp expect.2 actual.2 &&
    ++
    ++	sub1_gitdir="$(git -C sub1 rev-parse --absolute-git-dir)" &&
    ++	sub1_nested_gitdir="$(git -C sub1/nested rev-parse --absolute-git-dir)" &&
    ++
    ++	test-tool path-utils relative_path "$sub1_gitdir" "$sub1_nested_gitdir" \
    ++		>expect &&
    ++	git -C sub1/nested config submodule.superprojectGitDir >actual &&
    ++
    ++	test_cmp expect actual
    + '
    + 
    + test_expect_success 're-setup nested submodule' '
4:  6866c36620 ! 4:  df9879ff93 submodule: record superproject gitdir during 'update'
    @@ Commit message
     
      ## git-submodule.sh ##
     @@ git-submodule.sh: cmd_update()
    - 			fi
    - 		fi
    + 			;;
    + 		esac
      
     +		# Cache a pointer to the superproject's gitdir. This may have
    -+		# changed, so rewrite it unconditionally. Writes it to worktree
    ++		# changed, unless it's a fresh clone. Writes it to worktree
     +		# if applicable, otherwise to local.
    -+		relative_gitdir="$(git rev-parse --path-format=relative \
    -+						 --prefix "${sm_path}" \
    -+						 --git-dir)"
    ++		if test -z "$just_cloned"
    ++		then
    ++			sm_gitdir="$(git -C "$sm_path" rev-parse --absolute-git-dir)"
    ++			relative_gitdir="$(git rev-parse --path-format=relative \
    ++							 --prefix "${sm_gitdir}" \
    ++							 --git-dir)"
     +
    -+		git -C "$sm_path" config --worktree \
    -+			submodule.superprojectgitdir "$relative_gitdir"
    ++			git -C "$sm_path" config --worktree \
    ++				submodule.superprojectgitdir "$relative_gitdir"
    ++		fi
     +
      		if test -n "$recursive"
      		then
    @@ t/t7406-submodule-update.sh: test_expect_success 'submodule update --quiet passe
     +	(cd super &&
     +	 git -C submodule config --unset submodule.superprojectGitdir &&
     +	 git submodule update &&
    -+	 echo "../.git" >expect &&
    ++	 test-tool path-utils relative_path \
    ++		"$(git rev-parse --absolute-git-dir)" \
    ++		"$(git -C submodule rev-parse --absolute-git-dir)" >expect &&
     +	 git -C submodule config submodule.superprojectGitdir >actual &&
     +	 test_cmp expect actual
     +	)
-- 
2.33.0.1079.g6e70778dc9-goog
Next: Emily Shaffer
Message 1 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.