{"thread":{"id":"56712","subject":"[PATCH v4 0/4] cache parent project's gitdir in submodules","startedAt":"2021-10-14T20:34:34Z","lastAt":"2021-11-09T21:46:37Z","messageCount":31,"participants":["Emily Shaffer","Jonathan Tan","Derrick Stolee","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"438786","messageId":"20211014203416.2802639-1-emilyshaffer@google.com","threadId":"56712","inReplyTo":null,"subject":"[PATCH v4 0/4] cache parent project's gitdir in submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-10-14T20:34:12Z","receivedAt":"2021-10-14T20:34:34Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"For the original cover letter, see\nhttps://lore.kernel.org/git/20210611225428.1208973-1-emilyshaffer%40google.com.\n\nThe CI run is here:\nhttps://github.com/nasamuffin/git/runs/3899227896\n\nIt shows some broken Windows tests, but those are broken in the\nfork-point too:\nhttps://github.com/nasamuffin/git/actions/runs/1343120990\n\nSince v3, a pretty major change: the semantics of\nsubmodule.superprojectGitDir has changed, to point from the submodule's\ngitdir to the superproject's gitdir (in v3 and earlier, we kept a path\nfrom the submodule's *worktree* to the superproject's gitdir instead).\nThis cleans up some of the confusions about the behavior when a\nsubmodule worktree moves around in the superproject's tree, or in a\nfuture when we support submodules having multiple worktrees.\n\nI also tried to simplify the tests to use 'test-tool path-utils\nrelative_path' everywhere - I think that makes them much more clear for\na test reader, but if you're reviewing and it isn't obvious what we're\ntesting for, please speak up.\n\nI think this is pretty mature and there was a lot of general agreement\nthat the gitdir->gitdir association was the way to go, so please be\nbrutal and look for nits, leaks, etc. this round ;)\n\n - Emily\n\nEmily Shaffer (4):\n  t7400-submodule-basic: modernize inspect() helper\n  introduce submodule.superprojectGitDir record\n  submodule: record superproject gitdir during absorbgitdirs\n  submodule: record superproject gitdir during 'update'\n\n Documentation/config/submodule.txt | 12 +++++++\n builtin/submodule--helper.c        |  4 +++\n git-submodule.sh                   | 14 ++++++++\n submodule.c                        | 10 ++++++\n t/t7400-submodule-basic.sh         | 54 ++++++++++++++++--------------\n t/t7406-submodule-update.sh        | 12 +++++++\n t/t7412-submodule-absorbgitdirs.sh | 23 +++++++++++--\n 7 files changed, 102 insertions(+), 27 deletions(-)\n\nRange-diff against v3:\n1:  f1236dc9d7 ! 1:  2ff151aaa2 t7400-submodule-basic: modernize inspect() helper\n    @@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submo\n     -}\n     -\n      inspect() {\n    - \tdir=$1 &&\n    +-\tdir=$1 &&\n     -\tdotdot=\"${2:-..}\" &&\n    - \n    +-\n     -\t(\n     -\t\tcd \"$dir\" &&\n     -\t\tlistbranches >\"$dotdot/heads\" &&\n    @@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submo\n     -\t\tgit diff-files --exit-code &&\n     -\t\tgit clean -n -d -x >\"$dotdot/untracked\"\n     -\t)\n    -+\tgit -C \"$dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n    -+\t{ git -C \"$dir\" symbolic-ref HEAD || :; } >head &&\n    -+\tgit -C \"$dir\" rev-parse HEAD >head-sha1 &&\n    -+\tgit -C \"$dir\" update-index --refresh &&\n    -+\tgit -C \"$dir\" diff-files --exit-code &&\n    -+\tgit -C \"$dir\" clean -n -d -x >untracked\n    ++\tsub_dir=$1 &&\n    ++\n    ++\tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n    ++\t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n    ++\tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n    ++\tgit -C \"$sub_dir\" update-index --refresh &&\n    ++\tgit -C \"$sub_dir\" diff-files --exit-code &&\n    ++\tgit -C \"$sub_dir\" clean -n -d -x >untracked\n      }\n      \n      test_expect_success 'submodule add' '\n2:  2caf9eb8ee ! 2:  ed5479ad5d introduce submodule.superprojectGitDir record\n    @@ Commit message\n     \n         By using a relative path instead of an absolute path, we can move the\n         superproject directory around on the filesystem without breaking the\n    -    submodule's cache.\n    +    submodule's cache. And by using the path from gitdir to gitdir, we can\n    +    move the submodule within the superproject's tree structure without\n    +    breaking the submodule's cache, too.\n     \n         Since this hint value is only introduced during new submodule creation\n         via `git submodule add`, though, there is more work to do to allow the\n    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::\n      \tclone proceeds as if no alternate was specified.\n     +\n     +submodule.superprojectGitDir::\n    -+\tThe relative path from the submodule's worktree to its superproject's\n    ++\tThe relative path from the submodule's gitdir to its superproject's\n     +\tgitdir. When Git is run in a repository, it usually makes no difference\n     +\twhether this repository is standalone or a submodule, but if this\n     +\tconfiguration variable is present, additional behavior may be possible,\n    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::\n     +\tto be present in every submodule, so only optional value-added behavior\n     +\tshould be linked to it. It is set automatically during\n     +\tsubmodule creation.\n    -++\n    -+\tBecause of this configuration variable, it is forbidden to use the\n    -+\tsame submodule worktree shared by multiple superprojects.\n     \n      ## builtin/submodule--helper.c ##\n    -@@ builtin/submodule--helper.c: static int module_clone(int argc, const char **argv, const char *prefix)\n    +@@ builtin/submodule--helper.c: static int clone_submodule(struct module_clone_data *clone_data)\n      \t\tgit_config_set_in_file(p, \"submodule.alternateErrorStrategy\",\n    - \t\t\t\t\t   error_strategy);\n    + \t\t\t\t       error_strategy);\n      \n     +\tgit_config_set_in_file(p, \"submodule.superprojectGitdir\",\n     +\t\t\t       relative_path(absolute_path(get_git_dir()),\n    -+\t\t\t\t\t     path, &sb));\n    ++\t\t\t\t\t     sm_gitdir, &sb));\n     +\n      \tfree(sm_alternate);\n      \tfree(error_strategy);\n      \n     \n      ## t/t7400-submodule-basic.sh ##\n    -@@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository to add submodules to' '\n    - submodurl=$(pwd -P)\n    +@@ t/t7400-submodule-basic.sh: submodurl=$(pwd -P)\n      \n      inspect() {\n    --\tdir=$1 &&\n    --\n    --\tgit -C \"$dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n    --\t{ git -C \"$dir\" symbolic-ref HEAD || :; } >head &&\n    --\tgit -C \"$dir\" rev-parse HEAD >head-sha1 &&\n    --\tgit -C \"$dir\" update-index --refresh &&\n    --\tgit -C \"$dir\" diff-files --exit-code &&\n    --\tgit -C \"$dir\" clean -n -d -x >untracked\n    -+\tsub_dir=$1 &&\n    + \tsub_dir=$1 &&\n     +\tsuper_dir=$2 &&\n    + \n    + \tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n    + \t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n    + \tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n    + \tgit -C \"$sub_dir\" update-index --refresh &&\n    + \tgit -C \"$sub_dir\" diff-files --exit-code &&\n    ++\n    ++\t# Ensure that submodule.superprojectGitDir contains the path from the\n    ++\t# submodule's gitdir to the superproject's gitdir.\n    ++\n    ++\tsuper_abs_gitdir=$(git -C \"$super_dir\" rev-parse --absolute-git-dir) &&\n    ++\tsub_abs_gitdir=$(git -C \"$sub_dir\" rev-parse --absolute-git-dir) &&\n    ++\n    ++\t[ \"$(git -C \"$sub_dir\" config --get submodule.superprojectGitDir)\" = \\\n    ++\t  \"$(test-tool path-utils relative_path \"$super_abs_gitdir\" \\\n    ++\t\t\t\t\t\t\"$sub_abs_gitdir\")\" ] &&\n     +\n    -+\tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n    -+\t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n    -+\tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n    -+\tgit -C \"$sub_dir\" update-index --refresh &&\n    -+\tgit -C \"$sub_dir\" diff-files --exit-code &&\n    -+\tcached_super_dir=\"$(git -C \"$sub_dir\" config --get submodule.superprojectGitDir)\" &&\n    -+\t[ \"$(git -C \"$super_dir\" rev-parse --absolute-git-dir)\" \\\n    -+\t\t-ef \"$sub_dir/$cached_super_dir\" ] &&\n    -+\tgit -C \"$sub_dir\" clean -n -d -x >untracked\n    + \tgit -C \"$sub_dir\" clean -n -d -x >untracked\n      }\n      \n    - test_expect_success 'submodule add' '\n     @@ t/t7400-submodule-basic.sh: test_expect_success 'submodule add' '\n      \t) &&\n      \n3:  d278568a8e ! 3:  60e6cf77c5 submodule: record superproject gitdir during absorbgitdirs\n    @@ Commit message\n      ## submodule.c ##\n     @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *path)\n      \tchar *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;\n    - \tchar *new_git_dir;\n    + \tstruct strbuf new_gitdir = STRBUF_INIT;\n      \tconst struct submodule *sub;\n     +\tstruct strbuf config_path = STRBUF_INIT, sb = STRBUF_INIT;\n      \n    @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *p\n     +\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n     +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n     +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n    -+\t\t\t       relative_path(get_super_prefix_or_empty(),\n    -+\t\t\t\t\t     path, &sb));\n    ++\t\t\t       relative_path(absolute_path(get_git_dir()),\n    ++\t\t\t\t\t     real_new_git_dir, &sb));\n     +\n     +\tstrbuf_release(&config_path);\n     +\tstrbuf_release(&sb);\n    @@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir' '\n     +\ttest_cmp expect.2 actual.2 &&\n     +\n     +\t# make sure the submodule cached the superproject gitdir correctly\n    -+\ttest-tool path-utils real_path . >expect &&\n    -+\ttest-tool path-utils real_path \\\n    -+\t\t\"$(git -C sub1 config submodule.superprojectGitDir)\" >actual &&\n    ++\tsubmodule_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n    ++\tsuperproject_gitdir=\"$(git rev-parse --absolute-git-dir)\" &&\n    ++\n    ++\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n    ++\t\t\"$submodule_gitdir\" >expect &&\n    ++\tgit -C sub1 config submodule.superprojectGitDir >actual &&\n     +\n     +\ttest_cmp expect actual\n      '\n      \n      test_expect_success 'absorbing does not fail for deinitialized submodules' '\n    +@@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir in a nested submodule' '\n    + \tgit status >actual.1 &&\n    + \tgit -C sub1/nested rev-parse HEAD >actual.2 &&\n    + \ttest_cmp expect.1 actual.1 &&\n    +-\ttest_cmp expect.2 actual.2\n    ++\ttest_cmp expect.2 actual.2 &&\n    ++\n    ++\tsub1_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n    ++\tsub1_nested_gitdir=\"$(git -C sub1/nested rev-parse --absolute-git-dir)\" &&\n    ++\n    ++\ttest-tool path-utils relative_path \"$sub1_gitdir\" \"$sub1_nested_gitdir\" \\\n    ++\t\t>expect &&\n    ++\tgit -C sub1/nested config submodule.superprojectGitDir >actual &&\n    ++\n    ++\ttest_cmp expect actual\n    + '\n    + \n    + test_expect_success 're-setup nested submodule' '\n4:  6866c36620 ! 4:  df9879ff93 submodule: record superproject gitdir during 'update'\n    @@ Commit message\n     \n      ## git-submodule.sh ##\n     @@ git-submodule.sh: cmd_update()\n    - \t\t\tfi\n    - \t\tfi\n    + \t\t\t;;\n    + \t\tesac\n      \n     +\t\t# Cache a pointer to the superproject's gitdir. This may have\n    -+\t\t# changed, so rewrite it unconditionally. Writes it to worktree\n    ++\t\t# changed, unless it's a fresh clone. Writes it to worktree\n     +\t\t# if applicable, otherwise to local.\n    -+\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n    -+\t\t\t\t\t\t --prefix \"${sm_path}\" \\\n    -+\t\t\t\t\t\t --git-dir)\"\n    ++\t\tif test -z \"$just_cloned\"\n    ++\t\tthen\n    ++\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n    ++\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n    ++\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n    ++\t\t\t\t\t\t\t --git-dir)\"\n     +\n    -+\t\tgit -C \"$sm_path\" config --worktree \\\n    -+\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n    ++\t\t\tgit -C \"$sm_path\" config --worktree \\\n    ++\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n    ++\t\tfi\n     +\n      \t\tif test -n \"$recursive\"\n      \t\tthen\n    @@ t/t7406-submodule-update.sh: test_expect_success 'submodule update --quiet passe\n     +\t(cd super &&\n     +\t git -C submodule config --unset submodule.superprojectGitdir &&\n     +\t git submodule update &&\n    -+\t echo \"../.git\" >expect &&\n    ++\t test-tool path-utils relative_path \\\n    ++\t\t\"$(git rev-parse --absolute-git-dir)\" \\\n    ++\t\t\"$(git -C submodule rev-parse --absolute-git-dir)\" >expect &&\n     +\t git -C submodule config submodule.superprojectGitdir >actual &&\n     +\t test_cmp expect actual\n     +\t)\n-- \n2.33.0.1079.g6e70778dc9-goog\n\n"},{"id":"438787","messageId":"20211014203416.2802639-2-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"[PATCH v4 1/4] t7400-submodule-basic: modernize inspect() helper","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-10-14T20:34:13Z","receivedAt":"2021-10-14T20:34:36Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Since the inspect() helper in the submodule-basic test suite was\nwritten, 'git -C <dir>' was added. By using -C, we no longer need a\nreference to the base directory for the test. This simplifies callsites,\nand will make the addition of other arguments in later patches more\nreadable.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n t/t7400-submodule-basic.sh | 40 +++++++++++++++-----------------------\n 1 file changed, 16 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex cb1b8e35db..d69a5c0032 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -107,23 +107,15 @@ test_expect_success 'setup - repository to add submodules to' '\n # generates, which will expand symbolic links.\n submodurl=$(pwd -P)\n \n-listbranches() {\n-\tgit for-each-ref --format='%(refname)' 'refs/heads/*'\n-}\n-\n inspect() {\n-\tdir=$1 &&\n-\tdotdot=\"${2:-..}\" &&\n-\n-\t(\n-\t\tcd \"$dir\" &&\n-\t\tlistbranches >\"$dotdot/heads\" &&\n-\t\t{ git symbolic-ref HEAD || :; } >\"$dotdot/head\" &&\n-\t\tgit rev-parse HEAD >\"$dotdot/head-sha1\" &&\n-\t\tgit update-index --refresh &&\n-\t\tgit diff-files --exit-code &&\n-\t\tgit clean -n -d -x >\"$dotdot/untracked\"\n-\t)\n+\tsub_dir=$1 &&\n+\n+\tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n+\t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n+\tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n+\tgit -C \"$sub_dir\" update-index --refresh &&\n+\tgit -C \"$sub_dir\" diff-files --exit-code &&\n+\tgit -C \"$sub_dir\" clean -n -d -x >untracked\n }\n \n test_expect_success 'submodule add' '\n@@ -146,7 +138,7 @@ test_expect_success 'submodule add' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod ../.. &&\n+\tinspect addtest/submod &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -248,7 +240,7 @@ test_expect_success 'submodule add --branch' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod-branch ../.. &&\n+\tinspect addtest/submod-branch &&\n \ttest_cmp expect-heads heads &&\n \ttest_cmp expect-head head &&\n \ttest_must_be_empty untracked\n@@ -264,7 +256,7 @@ test_expect_success 'submodule add with ./ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotsubmod/frotz ../../.. &&\n+\tinspect addtest/dotsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -280,7 +272,7 @@ test_expect_success 'submodule add with /././ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotslashdotsubmod/frotz ../../.. &&\n+\tinspect addtest/dotslashdotsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -296,7 +288,7 @@ test_expect_success 'submodule add with // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/slashslashsubmod/frotz ../../.. &&\n+\tinspect addtest/slashslashsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -312,7 +304,7 @@ test_expect_success 'submodule add with /.. in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod ../.. &&\n+\tinspect addtest/realsubmod &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -328,7 +320,7 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod2 ../.. &&\n+\tinspect addtest/realsubmod2 &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -359,7 +351,7 @@ test_expect_success 'submodule add in subdirectory' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod3 ../.. &&\n+\tinspect addtest/realsubmod3 &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n-- \n2.33.0.1079.g6e70778dc9-goog\n\n"},{"id":"438788","messageId":"20211014203416.2802639-3-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"[PATCH v4 2/4] introduce submodule.superprojectGitDir record","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-10-14T20:34:14Z","receivedAt":"2021-10-14T20:34:41Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Teach submodules a reference to their superproject's gitdir. This allows\nus to A) know that we're running from a submodule, and B) have a\nshortcut to the superproject's vitals, for example, configs.\n\nBy using a relative path instead of an absolute path, we can move the\nsuperproject directory around on the filesystem without breaking the\nsubmodule's cache. And by using the path from gitdir to gitdir, we can\nmove the submodule within the superproject's tree structure without\nbreaking the submodule's cache, too.\n\nSince this hint value is only introduced during new submodule creation\nvia `git submodule add`, though, there is more work to do to allow the\nrecord to be created at other times.\n\nIf the new config is present, we can do some optional value-added\nbehavior, like letting \"git status\" print additional info about the\nsubmodule's status in relation to its superproject, or like letting the\nsuperproject and submodule share an additional config file separate from\neither one's local config.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config/submodule.txt | 12 +++++++++++\n builtin/submodule--helper.c        |  4 ++++\n t/t7400-submodule-basic.sh         | 32 ++++++++++++++++++++----------\n 3 files changed, 38 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt\nindex ee454f8126..c10aeb7fdc 100644\n--- a/Documentation/config/submodule.txt\n+++ b/Documentation/config/submodule.txt\n@@ -91,3 +91,15 @@ submodule.alternateErrorStrategy::\n \t`ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore`\n \tor `info`, and if there is an error with the computed alternate, the\n \tclone proceeds as if no alternate was specified.\n+\n+submodule.superprojectGitDir::\n+\tThe relative path from the submodule's gitdir to its superproject's\n+\tgitdir. When Git is run in a repository, it usually makes no difference\n+\twhether this repository is standalone or a submodule, but if this\n+\tconfiguration variable is present, additional behavior may be possible,\n+\tsuch as \"git status\" printing additional information about this\n+\tsubmodule's status with respect to its superproject. This config should\n+\tonly be present in projects which are submodules, but is not guaranteed\n+\tto be present in every submodule, so only optional value-added behavior\n+\tshould be linked to it. It is set automatically during\n+\tsubmodule creation.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d574d650af..338dcbd364 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1838,6 +1838,10 @@ static int clone_submodule(struct module_clone_data *clone_data)\n \t\tgit_config_set_in_file(p, \"submodule.alternateErrorStrategy\",\n \t\t\t\t       error_strategy);\n \n+\tgit_config_set_in_file(p, \"submodule.superprojectGitdir\",\n+\t\t\t       relative_path(absolute_path(get_git_dir()),\n+\t\t\t\t\t     sm_gitdir, &sb));\n+\n \tfree(sm_alternate);\n \tfree(error_strategy);\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex d69a5c0032..3d146491df 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -109,12 +109,24 @@ submodurl=$(pwd -P)\n \n inspect() {\n \tsub_dir=$1 &&\n+\tsuper_dir=$2 &&\n \n \tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n \t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n \tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n \tgit -C \"$sub_dir\" update-index --refresh &&\n \tgit -C \"$sub_dir\" diff-files --exit-code &&\n+\n+\t# Ensure that submodule.superprojectGitDir contains the path from the\n+\t# submodule's gitdir to the superproject's gitdir.\n+\n+\tsuper_abs_gitdir=$(git -C \"$super_dir\" rev-parse --absolute-git-dir) &&\n+\tsub_abs_gitdir=$(git -C \"$sub_dir\" rev-parse --absolute-git-dir) &&\n+\n+\t[ \"$(git -C \"$sub_dir\" config --get submodule.superprojectGitDir)\" = \\\n+\t  \"$(test-tool path-utils relative_path \"$super_abs_gitdir\" \\\n+\t\t\t\t\t\t\"$sub_abs_gitdir\")\" ] &&\n+\n \tgit -C \"$sub_dir\" clean -n -d -x >untracked\n }\n \n@@ -138,7 +150,7 @@ test_expect_success 'submodule add' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod &&\n+\tinspect addtest/submod addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -240,7 +252,7 @@ test_expect_success 'submodule add --branch' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod-branch &&\n+\tinspect addtest/submod-branch addtest &&\n \ttest_cmp expect-heads heads &&\n \ttest_cmp expect-head head &&\n \ttest_must_be_empty untracked\n@@ -256,7 +268,7 @@ test_expect_success 'submodule add with ./ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotsubmod/frotz &&\n+\tinspect addtest/dotsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -272,7 +284,7 @@ test_expect_success 'submodule add with /././ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotslashdotsubmod/frotz &&\n+\tinspect addtest/dotslashdotsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -288,7 +300,7 @@ test_expect_success 'submodule add with // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/slashslashsubmod/frotz &&\n+\tinspect addtest/slashslashsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -304,7 +316,7 @@ test_expect_success 'submodule add with /.. in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod &&\n+\tinspect addtest/realsubmod addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -320,7 +332,7 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod2 &&\n+\tinspect addtest/realsubmod2 addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -351,7 +363,7 @@ test_expect_success 'submodule add in subdirectory' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod3 &&\n+\tinspect addtest/realsubmod3 addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -492,7 +504,7 @@ test_expect_success 'update should work when path is an empty dir' '\n \tgit submodule update -q >update.out &&\n \ttest_must_be_empty update.out &&\n \n-\tinspect init &&\n+\tinspect init . &&\n \ttest_cmp expect head-sha1\n '\n \n@@ -551,7 +563,7 @@ test_expect_success 'update should checkout rev1' '\n \techo \"$rev1\" >expect &&\n \n \tgit submodule update init &&\n-\tinspect init &&\n+\tinspect init . &&\n \n \ttest_cmp expect head-sha1\n '\n-- \n2.33.0.1079.g6e70778dc9-goog\n\n"},{"id":"438789","messageId":"20211014203416.2802639-4-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"[PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-10-14T20:34:15Z","receivedAt":"2021-10-14T20:34:42Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Already during 'git submodule add' we record a pointer to the\nsuperproject's gitdir. However, this doesn't help brand-new\nsubmodules created with 'git init' and later absorbed with 'git\nsubmodule absorbgitdir'. Let's start adding that pointer during 'git\nsubmodule absorbgitdir' too.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n submodule.c                        | 10 ++++++++++\n t/t7412-submodule-absorbgitdirs.sh | 23 +++++++++++++++++++++--\n 2 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 96487517f9..571b68b66d 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2084,6 +2084,7 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n \tchar *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;\n \tstruct strbuf new_gitdir = STRBUF_INIT;\n \tconst struct submodule *sub;\n+\tstruct strbuf config_path = STRBUF_INIT, sb = STRBUF_INIT;\n \n \tif (submodule_uses_worktrees(path))\n \t\tdie(_(\"relocate_gitdir for submodule '%s' with \"\n@@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n \n \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n \n+\t/* cache pointer to superproject's gitdir */\n+\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n+\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n+\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n+\t\t\t       relative_path(absolute_path(get_git_dir()),\n+\t\t\t\t\t     real_new_git_dir, &sb));\n+\n+\tstrbuf_release(&config_path);\n+\tstrbuf_release(&sb);\n \tfree(old_git_dir);\n \tfree(real_old_git_dir);\n \tfree(real_new_git_dir);\ndiff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh\nindex 1cfa150768..9ee5ccd660 100755\n--- a/t/t7412-submodule-absorbgitdirs.sh\n+++ b/t/t7412-submodule-absorbgitdirs.sh\n@@ -30,7 +30,17 @@ test_expect_success 'absorb the git dir' '\n \tgit status >actual.1 &&\n \tgit -C sub1 rev-parse HEAD >actual.2 &&\n \ttest_cmp expect.1 actual.1 &&\n-\ttest_cmp expect.2 actual.2\n+\ttest_cmp expect.2 actual.2 &&\n+\n+\t# make sure the submodule cached the superproject gitdir correctly\n+\tsubmodule_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n+\tsuperproject_gitdir=\"$(git rev-parse --absolute-git-dir)\" &&\n+\n+\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n+\t\t\"$submodule_gitdir\" >expect &&\n+\tgit -C sub1 config submodule.superprojectGitDir >actual &&\n+\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'absorbing does not fail for deinitialized submodules' '\n@@ -61,7 +71,16 @@ test_expect_success 'absorb the git dir in a nested submodule' '\n \tgit status >actual.1 &&\n \tgit -C sub1/nested rev-parse HEAD >actual.2 &&\n \ttest_cmp expect.1 actual.1 &&\n-\ttest_cmp expect.2 actual.2\n+\ttest_cmp expect.2 actual.2 &&\n+\n+\tsub1_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n+\tsub1_nested_gitdir=\"$(git -C sub1/nested rev-parse --absolute-git-dir)\" &&\n+\n+\ttest-tool path-utils relative_path \"$sub1_gitdir\" \"$sub1_nested_gitdir\" \\\n+\t\t>expect &&\n+\tgit -C sub1/nested config submodule.superprojectGitDir >actual &&\n+\n+\ttest_cmp expect actual\n '\n \n test_expect_success 're-setup nested submodule' '\n-- \n2.33.0.1079.g6e70778dc9-goog\n\n"},{"id":"438790","messageId":"20211014203416.2802639-5-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"[PATCH v4 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-10-14T20:34:16Z","receivedAt":"2021-10-14T20:34:44Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"A recorded hint path to the superproject's gitdir might be added during\n'git submodule add', but in some cases - like submodules which were\ncreated before 'git submodule add' learned to record that info - it might\nbe useful to update the hint. Let's do it during 'git submodule\nupdate', when we already have a handle to the superproject while calling\noperations on the submodules.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n git-submodule.sh            | 14 ++++++++++++++\n t/t7406-submodule-update.sh | 12 ++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 652861aa66..ec02eb5971 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -449,6 +449,20 @@ cmd_update()\n \t\t\t;;\n \t\tesac\n \n+\t\t# Cache a pointer to the superproject's gitdir. This may have\n+\t\t# changed, unless it's a fresh clone. Writes it to worktree\n+\t\t# if applicable, otherwise to local.\n+\t\tif test -z \"$just_cloned\"\n+\t\tthen\n+\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n+\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n+\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n+\t\t\t\t\t\t\t --git-dir)\"\n+\n+\t\t\tgit -C \"$sm_path\" config --worktree \\\n+\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n+\t\tfi\n+\n \t\tif test -n \"$recursive\"\n \t\tthen\n \t\t\t(\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 11cccbb333..7fd5741a5a 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -1061,4 +1061,16 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s\n \t)\n '\n \n+test_expect_success 'submodule update adds superproject gitdir to older repos' '\n+\t(cd super &&\n+\t git -C submodule config --unset submodule.superprojectGitdir &&\n+\t git submodule update &&\n+\t test-tool path-utils relative_path \\\n+\t\t\"$(git rev-parse --absolute-git-dir)\" \\\n+\t\t\"$(git -C submodule rev-parse --absolute-git-dir)\" >expect &&\n+\t git -C submodule config submodule.superprojectGitdir >actual &&\n+\t test_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.33.0.1079.g6e70778dc9-goog\n\n"},{"id":"439010","messageId":"20211018231818.89219-1-jonathantanmy@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-4-emilyshaffer@google.com","subject":"Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2021-10-18T23:18:18Z","receivedAt":"2021-10-18T23:18:25Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> Already during 'git submodule add' we record a pointer to the\n> superproject's gitdir. However, this doesn't help brand-new\n> submodules created with 'git init' and later absorbed with 'git\n> submodule absorbgitdir'. Let's start adding that pointer during 'git\n> submodule absorbgitdir' too.\n\ns/absorbgitdir/absorbgitdirs/ (note the \"s\" at the end)\n\n> @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n>  \n>  \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n>  \n> +\t/* cache pointer to superproject's gitdir */\n> +\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n> +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n> +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n> +\t\t\t       relative_path(absolute_path(get_git_dir()),\n> +\t\t\t\t\t     real_new_git_dir, &sb));\n> +\n> +\tstrbuf_release(&config_path);\n> +\tstrbuf_release(&sb);\n>  \tfree(old_git_dir);\n>  \tfree(real_old_git_dir);\n>  \tfree(real_new_git_dir);\n\nHere [1] you mention that you'll delete the NEEDSWORK, but it's still\nthere.\n\nHaving said that, it might be better to make a test in which we call\nthis command while in a worktree of a superproject. The test might\nreveal that (as pointed out to me internally) you might need to use the\ncommon dir functions instead of the git dir functions to point to the\ndirectory that you want (git-worktree.txt distinguishes the 2 if you\nsearch for GIT_COMMON_DIR).\n\nBesides that, all 4 patches look good (including the description of the\nnew config variable).\n\n[1] https://lore.kernel.org/git/YWc2iJ7FQJYCnQ7w@google.com/\n"},{"id":"439553","messageId":"b208eb06-21c7-2199-0bc8-597669a165a1@gmail.com","threadId":"56712","inReplyTo":"20211018231818.89219-1-jonathantanmy@google.com","subject":"Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-25T16:14:07Z","receivedAt":"2021-10-25T16:14:24Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/18/2021 7:18 PM, Jonathan Tan wrote:\n>> Already during 'git submodule add' we record a pointer to the\n>> superproject's gitdir. However, this doesn't help brand-new\n>> submodules created with 'git init' and later absorbed with 'git\n>> submodule absorbgitdir'. Let's start adding that pointer during 'git\n>> submodule absorbgitdir' too.\n> \n> s/absorbgitdir/absorbgitdirs/ (note the \"s\" at the end)\n> \n>> @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n>>  \n>>  \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n>>  \n>> +\t/* cache pointer to superproject's gitdir */\n>> +\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n>> +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n>> +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n>> +\t\t\t       relative_path(absolute_path(get_git_dir()),\n>> +\t\t\t\t\t     real_new_git_dir, &sb));\n>> +\n>> +\tstrbuf_release(&config_path);\n>> +\tstrbuf_release(&sb);\n>>  \tfree(old_git_dir);\n>>  \tfree(real_old_git_dir);\n>>  \tfree(real_new_git_dir);\n> \n> Here [1] you mention that you'll delete the NEEDSWORK, but it's still\n> there.\n> \n> Having said that, it might be better to make a test in which we call\n> this command while in a worktree of a superproject. The test might\n> reveal that (as pointed out to me internally) you might need to use the\n> common dir functions instead of the git dir functions to point to the\n> directory that you want (git-worktree.txt distinguishes the 2 if you\n> search for GIT_COMMON_DIR).\n\nI came here to say the same thing. It's a bit too direct to compute\nthe location of a config file this way, so we should expose a method\nthat can create one for a given Git directory.\n\nSince you're setting this config value inside the submodule's config,\nwhat does it mean for a submodule to also be a worktree (and hence\nrequire config.worktree)? What happens in this rough scenario?\n\n  1. git init sub\n  2. git init super\n  3. git -C sub worktree add super/sub\n  4. git -C super submodule absorbgitdir sub\n\nI haven't actually tried running these things, but it seems unusual\nand unexpected. This doesn't even account for cases where the repo\nroot and a worktree are both submodules within the superproject.\n\nIf we already have protections preventing these worktrees as\nsubmodules, then perhaps there is no need for work here. I'm not\nfamiliar enough with the area to make a claim one way or another.\n\nThanks,\n-Stolee\n"},{"id":"439554","messageId":"6ad1f4bc-25cc-8c0d-2bf2-900c94d717d9@gmail.com","threadId":"56712","inReplyTo":"20211014203416.2802639-5-emilyshaffer@google.com","subject":"Re: [PATCH v4 4/4] submodule: record superproject gitdir during 'update'","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-25T16:17:27Z","receivedAt":"2021-10-25T16:17:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2021 4:34 PM, Emily Shaffer wrote:\n\n> +\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n> +\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n> +\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n> +\t\t\t\t\t\t\t --git-dir)\"\n> +\n> +\t\t\tgit -C \"$sm_path\" config --worktree \\\n> +\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n\nThis use of `--worktree` is good because it acts the same as `--local` if\nconfig.worktree doesn't exist. If we discover it is too challenging to find\nthe correct config file in-core, then we could call this command as a subprocess\nin patch 3.\n\nThanks,\n-Stolee\n"},{"id":"439555","messageId":"911ab2c1-8a11-d9d0-4b28-fc801112f6da@gmail.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"Re: [PATCH v4 0/4] cache parent project's gitdir in submodules","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-25T16:19:43Z","receivedAt":"2021-10-25T16:19:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2021 4:34 PM, Emily Shaffer wrote:\n> Since v3, a pretty major change: the semantics of\n> submodule.superprojectGitDir has changed, to point from the submodule's\n> gitdir to the superproject's gitdir (in v3 and earlier, we kept a path\n> from the submodule's *worktree* to the superproject's gitdir instead).\n> This cleans up some of the confusions about the behavior when a\n> submodule worktree moves around in the superproject's tree, or in a\n> future when we support submodules having multiple worktrees.\n\nI like the new design!\n \n> I also tried to simplify the tests to use 'test-tool path-utils\n> relative_path' everywhere - I think that makes them much more clear for\n> a test reader, but if you're reviewing and it isn't obvious what we're\n> testing for, please speak up.\n\nI found these changes to be well motivated.\n\n> I think this is pretty mature and there was a lot of general agreement\n> that the gitdir->gitdir association was the way to go, so please be\n> brutal and look for nits, leaks, etc. this round ;)\n\nMy main concern was also brought up by Jonathan: the concerns around\nworktrees are not fully fleshed out. We should avoid a NEEDSWORK, and\nthere is a way forward using a subprocess even if we find it is\ndifficult to find the correct config file in-core. In-core is preferred,\nbut it's good to have options.\n\nThanks,\n-Stolee\n"},{"id":"440487","messageId":"YYRaII8YWVxlBqsF@google.com","threadId":"56712","inReplyTo":"20211018231818.89219-1-jonathantanmy@google.com","subject":"Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T22:09:36Z","receivedAt":"2021-11-04T22:09:45Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Oct 18, 2021 at 04:18:18PM -0700, Jonathan Tan wrote:\n> \n> > Already during 'git submodule add' we record a pointer to the\n> > superproject's gitdir. However, this doesn't help brand-new\n> > submodules created with 'git init' and later absorbed with 'git\n> > submodule absorbgitdir'. Let's start adding that pointer during 'git\n> > submodule absorbgitdir' too.\n> \n> s/absorbgitdir/absorbgitdirs/ (note the \"s\" at the end)\n> \n> > @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n> >  \n> >  \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n> >  \n> > +\t/* cache pointer to superproject's gitdir */\n> > +\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n> > +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n> > +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n> > +\t\t\t       relative_path(absolute_path(get_git_dir()),\n> > +\t\t\t\t\t     real_new_git_dir, &sb));\n> > +\n> > +\tstrbuf_release(&config_path);\n> > +\tstrbuf_release(&sb);\n> >  \tfree(old_git_dir);\n> >  \tfree(real_old_git_dir);\n> >  \tfree(real_new_git_dir);\n> \n> Here [1] you mention that you'll delete the NEEDSWORK, but it's still\n> there.\n> \n> Having said that, it might be better to make a test in which we call\n> this command while in a worktree of a superproject. The test might\n> reveal that (as pointed out to me internally) you might need to use the\n> common dir functions instead of the git dir functions to point to the\n> directory that you want (git-worktree.txt distinguishes the 2 if you\n> search for GIT_COMMON_DIR).\n\nHuh, something interesting happened, actually.\n\nI wrote a little test:\n\n  test_expect_success 'absorbgitdirs works when called from a superproject worktree' '\n          # set up a worktree of the superproject\n          git worktree add wt &&\n          (\n          cd wt &&\n \n          # create a new unembedded git dir\n          git init sub4 &&\n          test_commit -C sub4 first &&\n          git submodule add ./sub4 &&\n          test_tick &&\n \n          # absorb the git dir\n          git submodule absorbgitdirs sub4 &&\n \n          # make sure the submodule cached the superproject gitdir correctly\n          submodule_gitdir=\"$(git -C sub4 rev-parse --absolute-git-dir)\" &&\n          superproject_gitdir=\"$(git rev-parse --absolute-git-dir)\" &&\n \n          test-tool path-utils relative_path \"$superproject_gitdir\" \\\n                  \"$submodule_gitdir\" >expect &&\n          git -C sub4 config submodule.superprojectGitDir >actual &&\n \n          test_pause &&\n          test_cmp expect actual\n          )\n  '\n\nHowever, the `git submodule absorbgitdirs` command didn't do quite what\nI expected.\n\nWhen I made a new worktree, that worktree's gitdir showed up in\n'$TEST_DIR/.git/worktrees/wt/'. That's not very surprising. But what did\nsurprise me was that when I called `git submodule absorbgitdirs sub4`,\nsub4's gitdir ended up in '$TEST_DIR/.git/worktrees/wt/modules/sub4',\nrather than in '$TEST_DIR/.git/modules/sub4'. That's a little\nsurprising!\n\nAnyway, this test has a sort of pernicious mistake too - I'm checking\nrelative path between 'git rev-parse --absolute-git-dir's, but the\nrelative path from .git/worktrees/wt/ to .git/worktrees/wt/modules/sub4\nis the same as the relative path from .git/ to .git/modules/sub4, so\nthis test actually passes.\n\nI'll change the tests here and elsewhere to use 'git rev-parse\n--path-format=absolute --git-common-dir', but I think that still leaves\na kind of dangerous state for people working a lot with worktrees and\nsubmodules - if I like to make throwaway worktrees in my regular\nworkflow, and I create a new submodule in one, and then get rid of the\nworktree when I'm done with the task that added the new submodule, I\nthink it will explode if I try to checkout the branch I was using in\nthat worktree.\n\nI tried it out locally:\n\n # setup\n git init test && cd test\n git commit --allow-empty -m \"first commit\"\n git worktree add wt\n (\n   cd wt\n   git init sub\n   git -C sub commit --allow-empty -m \"first-commit\"\n   git submodule add ./sub\n   git commit -m \"add submodule (as initted)\n   git submodule absorbgitdirs sub\n   # This told me \"Migrating git directory of 'sub' from\n   # test/wt/sub/.git to test/.git/worktrees/wt/modules/sub, oops\n )\n\n # Make it possible to checkout wt branch somewhere else\n git -C wt checkout HEAD --detach\n\n # Try and delete the worktree; presumably my work is done\n git worktree remove wt\n\nAt this point, Git wouldn't let me remove the worktree:\n  fatal: working trees containing submodules cannot be moved or removed\n\nBut wouldn't it be better to just absorb the submodule into the common\ndir instead...?\n\nBy the way, when I tried checking out 'wt' branch from the original\nworktree without deleting the new worktree, and then running 'git\nsubmodule update', Git cloned 'sub' from .git/worktrees/wt/modules/sub\ninto .git/modules/sub. That was pretty surprising too!\n\n\nAnyway, all of that is kind of a tangent. The takeaways I got are\ntwofold:\n\n1) Yes, use common dir, not gitdir, in all the cached\npath-to-superproject stuff.\n2) Someone (me?) should send a patch keeping the \"you can't delete a\nsubmodule with a gitdir in it\" die(), but *also* changing the behavior\nto put the new submodule gitdir into get_common_dir() instead of\nget_git_dir().\n\nI'll try to send (2) separately - I think it will be a pretty small change,\nand by keeping around the die() for deleting a worktree that already has\na submodule gitdir in it, we won't be risking deleting anybody's\nexisting work.\n\n - Emily\n\n> \n> Besides that, all 4 patches look good (including the description of the\n> new config variable).\n> \n> [1] https://lore.kernel.org/git/YWc2iJ7FQJYCnQ7w@google.com/\n"},{"id":"440490","messageId":"YYRrH6Hw/C1qj5MV@google.com","threadId":"56712","inReplyTo":"b208eb06-21c7-2199-0bc8-597669a165a1@gmail.com","subject":"Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:22:07Z","receivedAt":"2021-11-04T23:22:14Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Oct 25, 2021 at 12:14:07PM -0400, Derrick Stolee wrote:\n> \n> On 10/18/2021 7:18 PM, Jonathan Tan wrote:\n> >> Already during 'git submodule add' we record a pointer to the\n> >> superproject's gitdir. However, this doesn't help brand-new\n> >> submodules created with 'git init' and later absorbed with 'git\n> >> submodule absorbgitdir'. Let's start adding that pointer during 'git\n> >> submodule absorbgitdir' too.\n> > \n> > s/absorbgitdir/absorbgitdirs/ (note the \"s\" at the end)\n> > \n> >> @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n> >>  \n> >>  \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n> >>  \n> >> +\t/* cache pointer to superproject's gitdir */\n> >> +\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n> >> +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n> >> +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n> >> +\t\t\t       relative_path(absolute_path(get_git_dir()),\n> >> +\t\t\t\t\t     real_new_git_dir, &sb));\n> >> +\n> >> +\tstrbuf_release(&config_path);\n> >> +\tstrbuf_release(&sb);\n> >>  \tfree(old_git_dir);\n> >>  \tfree(real_old_git_dir);\n> >>  \tfree(real_new_git_dir);\n> > \n> > Here [1] you mention that you'll delete the NEEDSWORK, but it's still\n> > there.\n> > \n> > Having said that, it might be better to make a test in which we call\n> > this command while in a worktree of a superproject. The test might\n> > reveal that (as pointed out to me internally) you might need to use the\n> > common dir functions instead of the git dir functions to point to the\n> > directory that you want (git-worktree.txt distinguishes the 2 if you\n> > search for GIT_COMMON_DIR).\n> \n> I came here to say the same thing. It's a bit too direct to compute\n> the location of a config file this way, so we should expose a method\n> that can create one for a given Git directory.\n> \n> Since you're setting this config value inside the submodule's config,\n> what does it mean for a submodule to also be a worktree (and hence\n> require config.worktree)? What happens in this rough scenario?\n> \n>   1. git init sub\n>   2. git init super\n>   3. git -C sub worktree add super/sub\n>   4. git -C super submodule absorbgitdir sub\n> \n> I haven't actually tried running these things, but it seems unusual\n> and unexpected. This doesn't even account for cases where the repo\n> root and a worktree are both submodules within the superproject.\n> \n> If we already have protections preventing these worktrees as\n> submodules, then perhaps there is no need for work here. I'm not\n> familiar enough with the area to make a claim one way or another.\n\nYeah, I think there is actually a test case covering this in t7412:\n\n137 test_expect_success 'setup a submodule with multiple worktrees' '\n138         # first create another unembedded git dir in a new submodule\n139         git init sub3 &&\n140         test_commit -C sub3 first &&\n141         git submodule add ./sub3 &&\n142         test_tick &&\n143         git commit -m \"add another submodule\" &&\n144         git -C sub3 worktree add ../sub3_second_work_tree\n145 '\n146\n147 test_expect_success 'absorbing fails for a submodule with multiple worktrees' '\n148         test_must_fail git submodule absorbgitdirs sub3 2>error &&\n149         test_i18ngrep \"not supported\" error\n150 '\n\nThat is, I think because 'sub/' in your scenario above has multiple\nworktrees, the absorbgitdirs will fail. So I won't do additional work\nhere. Thanks.\n\n - Emily\n\n> \n> Thanks,\n> -Stolee\n"},{"id":"440491","messageId":"20211104234942.3473650-1-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211014203416.2802639-1-emilyshaffer@google.com","subject":"[PATCH v5 0/4] cache parent project's gitdir in submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:49:38Z","receivedAt":"2021-11-04T23:49:48Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"For the original cover letter, see\nhttps://lore.kernel.org/git/20210611225428.1208973-1-emilyshaffer%40google.com.\n\nThe CI run is here:\nhttps://github.com/nasamuffin/git/actions/runs/1423475715\n\nIt shows a broken (pedantic,fedora) test, but those are known broken:\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2111040007170.56%40tvgsbejvaqbjf.bet\n\n[v4 cover letter]\nSince v3, a pretty major change: the semantics of\nsubmodule.superprojectGitDir has changed, to point from the submodule's\ngitdir to the superproject's gitdir (in v3 and earlier, we kept a path\nfrom the submodule's *worktree* to the superproject's gitdir instead).\nThis cleans up some of the confusions about the behavior when a\nsubmodule worktree moves around in the superproject's tree, or in a\nfuture when we support submodules having multiple worktrees.\n\nI also tried to simplify the tests to use 'test-tool path-utils\nrelative_path' everywhere - I think that makes them much more clear for\na test reader, but if you're reviewing and it isn't obvious what we're\ntesting for, please speak up.\n\nI think this is pretty mature and there was a lot of general agreement\nthat the gitdir->gitdir association was the way to go, so please be\nbrutal and look for nits, leaks, etc. this round ;)\n[/v4 cover letter]\n\nSince v4:\n\nThe only real change here is a slight semantics change to map from\n<submodule gitdir> to <superproject common git dir>. In every case\n*except* for when the superproject has a worktree, this changes nothing.\nFor the case when the superproject has a worktree, this means that now\nsubmodules will refer to the general superproject common dir (e.g. no\nworktree-specific refs or configs or whatnot).\n\nI *think* that because a submodule should exist in the context of the\ncommon dir, not the worktree gitdir, that is ok. However, it does mean\nit would be difficult to do something like sharing a config specific to\nthe worktree (the initial goal of this series).\n\n$ROOT/.git\n$ROOT/.git/config.superproject <- shared by $ROOT/.git/modules/sub\n$ROOT/.git/modules/sub <- points to $ROOT/.git\n$ROOT/.git/worktrees/wt\n$ROOT/.git/worktrees/wt/config.superproject <- contains a certain config-based pre-commit hook\n\nIf the submodule only knows about the common dir, that is tough, because\nthe submodule would basically have to guess which worktree it's in from\nits own path. There would be no way for '$WT/sub' to inherit\n'$ROOT/.git/worktrees/wt/config.superproject'.\n\nThat said... right now, we don't support submodules in worktrees very\nwell at all. A submodule in a worktree will get a brand new gitdir in\n$ROOT/.git/worktrees/modules/ (and that brand new gitdir would point to\nthe super's common dir). So I think we can punt on this entire question\nuntil we teach submodules and worktrees to play more gracefully together\n(it's on my long list...), and at that time we can probably introduce a\npointer from $ROOT/.git/modules/sub/worktrees/wt/ to\n$ROOT/.git/worktrees/wt/....\n\nOr, to summarize the long ramble above: \"this is still kind of weird\nwith worktrees, but let's fix it later when we fix worktrees more\nthoroughly\".\n\n(More rambling about worktree weirdness here:\nhttps://lore.kernel.org/git/YYRaII8YWVxlBqsF%40google.com )\n\n - Emily\n\nEmily Shaffer (4):\n  t7400-submodule-basic: modernize inspect() helper\n  introduce submodule.superprojectGitDir record\n  submodule: record superproject gitdir during absorbgitdirs\n  submodule: record superproject gitdir during 'update'\n\n Documentation/config/submodule.txt | 12 +++++++\n builtin/submodule--helper.c        |  4 +++\n git-submodule.sh                   | 14 ++++++++\n submodule.c                        |  9 +++++\n t/t7400-submodule-basic.sh         | 54 ++++++++++++++++--------------\n t/t7406-submodule-update.sh        | 12 +++++++\n t/t7412-submodule-absorbgitdirs.sh | 50 +++++++++++++++++++++++++--\n 7 files changed, 128 insertions(+), 27 deletions(-)\n\nRange-diff against v4:\n1:  2ff151aaa2 = 1:  6ff10beaf2 t7400-submodule-basic: modernize inspect() helper\n2:  ed5479ad5d ! 2:  d4f4627585 introduce submodule.superprojectGitDir record\n    @@ Commit message\n         superproject directory around on the filesystem without breaking the\n         submodule's cache. And by using the path from gitdir to gitdir, we can\n         move the submodule within the superproject's tree structure without\n    -    breaking the submodule's cache, too.\n    +    breaking the submodule's cache, too. Finally, by pointing at\n    +    \"get_git_common_dir()\" instead of \"get_git_dir()\", we ensure the link\n    +    will refer to the parent repo, not to a specific worktree.\n     \n         Since this hint value is only introduced during new submodule creation\n         via `git submodule add`, though, there is more work to do to allow the\n    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::\n     +\n     +submodule.superprojectGitDir::\n     +\tThe relative path from the submodule's gitdir to its superproject's\n    -+\tgitdir. When Git is run in a repository, it usually makes no difference\n    -+\twhether this repository is standalone or a submodule, but if this\n    -+\tconfiguration variable is present, additional behavior may be possible,\n    -+\tsuch as \"git status\" printing additional information about this\n    -+\tsubmodule's status with respect to its superproject. This config should\n    -+\tonly be present in projects which are submodules, but is not guaranteed\n    -+\tto be present in every submodule, so only optional value-added behavior\n    -+\tshould be linked to it. It is set automatically during\n    -+\tsubmodule creation.\n    ++\tcommon dir. When Git is run in a repository, it usually makes no\n    ++\tdifference whether this repository is standalone or a submodule, but if\n    ++\tthis configuration variable is present, additional behavior may be\n    ++\tpossible, such as \"git status\" printing additional information about\n    ++\tthis submodule's status with respect to its superproject. This config\n    ++\tshould only be present in projects which are submodules, but is not\n    ++\tguaranteed to be present in every submodule, so only optional\n    ++\tvalue-added behavior should be linked to it. It is set automatically\n    ++\tduring submodule creation.\n     \n      ## builtin/submodule--helper.c ##\n     @@ builtin/submodule--helper.c: static int clone_submodule(struct module_clone_data *clone_data)\n    @@ builtin/submodule--helper.c: static int clone_submodule(struct module_clone_data\n      \t\t\t\t       error_strategy);\n      \n     +\tgit_config_set_in_file(p, \"submodule.superprojectGitdir\",\n    -+\t\t\t       relative_path(absolute_path(get_git_dir()),\n    ++\t\t\t       relative_path(absolute_path(get_git_common_dir()),\n     +\t\t\t\t\t     sm_gitdir, &sb));\n     +\n      \tfree(sm_alternate);\n    @@ t/t7400-submodule-basic.sh: submodurl=$(pwd -P)\n     +\t# Ensure that submodule.superprojectGitDir contains the path from the\n     +\t# submodule's gitdir to the superproject's gitdir.\n     +\n    -+\tsuper_abs_gitdir=$(git -C \"$super_dir\" rev-parse --absolute-git-dir) &&\n    -+\tsub_abs_gitdir=$(git -C \"$sub_dir\" rev-parse --absolute-git-dir) &&\n    ++\tsuper_abs_gitdir=$(git -C \"$super_dir\" rev-parse --path-format=absolute --git-common-dir) &&\n    ++\tsub_abs_gitdir=$(git -C \"$sub_dir\" rev-parse --path-format=absolute --git-common-dir) &&\n     +\n     +\t[ \"$(git -C \"$sub_dir\" config --get submodule.superprojectGitDir)\" = \\\n     +\t  \"$(test-tool path-utils relative_path \"$super_abs_gitdir\" \\\n3:  60e6cf77c5 ! 3:  2dae297943 submodule: record superproject gitdir during absorbgitdirs\n    @@ Commit message\n         Already during 'git submodule add' we record a pointer to the\n         superproject's gitdir. However, this doesn't help brand-new\n         submodules created with 'git init' and later absorbed with 'git\n    -    submodule absorbgitdir'. Let's start adding that pointer during 'git\n    -    submodule absorbgitdir' too.\n    +    submodule absorbgitdirs'. Let's start adding that pointer during 'git\n    +    submodule absorbgitdirs' too.\n     \n         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n     \n    @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *p\n      \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n      \n     +\t/* cache pointer to superproject's gitdir */\n    -+\t/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */\n     +\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n     +\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n    -+\t\t\t       relative_path(absolute_path(get_git_dir()),\n    ++\t\t\t       relative_path(absolute_path(get_git_common_dir()),\n     +\t\t\t\t\t     real_new_git_dir, &sb));\n     +\n     +\tstrbuf_release(&config_path);\n    @@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir' '\n     +\ttest_cmp expect.2 actual.2 &&\n     +\n     +\t# make sure the submodule cached the superproject gitdir correctly\n    -+\tsubmodule_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n    -+\tsuperproject_gitdir=\"$(git rev-parse --absolute-git-dir)\" &&\n    ++\tsubmodule_gitdir=\"$(git -C sub1 rev-parse --path-format=absolute --git-common-dir)\" &&\n    ++\tsuperproject_gitdir=\"$(git rev-parse --path-format=absolute --git-common-dir)\" &&\n     +\n     +\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n     +\t\t\"$submodule_gitdir\" >expect &&\n    @@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir in a\n     -\ttest_cmp expect.2 actual.2\n     +\ttest_cmp expect.2 actual.2 &&\n     +\n    -+\tsub1_gitdir=\"$(git -C sub1 rev-parse --absolute-git-dir)\" &&\n    -+\tsub1_nested_gitdir=\"$(git -C sub1/nested rev-parse --absolute-git-dir)\" &&\n    ++\tsub1_gitdir=\"$(git -C sub1 rev-parse --path-format=absolute --git-common-dir)\" &&\n    ++\tsub1_nested_gitdir=\"$(git -C sub1/nested rev-parse --path-format=absolute --git-common-dir)\" &&\n     +\n     +\ttest-tool path-utils relative_path \"$sub1_gitdir\" \"$sub1_nested_gitdir\" \\\n     +\t\t>expect &&\n    @@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir in a\n      '\n      \n      test_expect_success 're-setup nested submodule' '\n    +@@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorbing fails for a submodule with multiple worktrees' '\n    + \ttest_i18ngrep \"not supported\" error\n    + '\n    + \n    ++test_expect_success 'absorbgitdirs works when called from a superproject worktree' '\n    ++\t# set up a worktree of the superproject\n    ++\tgit worktree add wt &&\n    ++\t(\n    ++\tcd wt &&\n    ++\n    ++\t# create a new unembedded git dir\n    ++\tgit init sub4 &&\n    ++\ttest_commit -C sub4 first &&\n    ++\tgit submodule add ./sub4 &&\n    ++\ttest_tick &&\n    ++\n    ++\t# absorb the git dir\n    ++\tgit submodule absorbgitdirs sub4 &&\n    ++\n    ++\t# make sure the submodule cached the superproject gitdir correctly\n    ++\tsubmodule_gitdir=\"$(git -C sub4 rev-parse --path-format=absolute --git-common-dir)\" &&\n    ++\tsuperproject_gitdir=\"$(git rev-parse --path-format=absolute --git-common-dir)\" &&\n    ++\n    ++\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n    ++\t\t\"$submodule_gitdir\" >expect &&\n    ++\tgit -C sub4 config submodule.superprojectGitDir >actual &&\n    ++\n    ++\ttest_cmp expect actual\n    ++\t)\n    ++'\n    ++\n    + test_done\n4:  df9879ff93 ! 4:  f0412d6d34 submodule: record superproject gitdir during 'update'\n    @@ git-submodule.sh: cmd_update()\n      \t\t\t;;\n      \t\tesac\n      \n    -+\t\t# Cache a pointer to the superproject's gitdir. This may have\n    ++\t\t# Cache a pointer to the superproject's common dir. This may have\n     +\t\t# changed, unless it's a fresh clone. Writes it to worktree\n     +\t\t# if applicable, otherwise to local.\n     +\t\tif test -z \"$just_cloned\"\n    @@ git-submodule.sh: cmd_update()\n     +\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n     +\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n     +\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n    -+\t\t\t\t\t\t\t --git-dir)\"\n    ++\t\t\t\t\t\t\t --git-common-dir)\"\n     +\n     +\t\t\tgit -C \"$sm_path\" config --worktree \\\n     +\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n    @@ t/t7406-submodule-update.sh: test_expect_success 'submodule update --quiet passe\n     +\t git -C submodule config --unset submodule.superprojectGitdir &&\n     +\t git submodule update &&\n     +\t test-tool path-utils relative_path \\\n    -+\t\t\"$(git rev-parse --absolute-git-dir)\" \\\n    -+\t\t\"$(git -C submodule rev-parse --absolute-git-dir)\" >expect &&\n    ++\t\t\"$(git rev-parse --path-format=absolute --git-common-dir)\" \\\n    ++\t\t\"$(git -C submodule rev-parse --path-format=absolute --git-common-dir)\" >expect &&\n     +\t git -C submodule config submodule.superprojectGitdir >actual &&\n     +\t test_cmp expect actual\n     +\t)\n-- \n2.34.0.rc0.344.g81b53c2807-goog\n\n"},{"id":"440492","messageId":"20211104234942.3473650-2-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211104234942.3473650-1-emilyshaffer@google.com","subject":"[PATCH v5 1/4] t7400-submodule-basic: modernize inspect() helper","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:49:39Z","receivedAt":"2021-11-04T23:49:50Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Since the inspect() helper in the submodule-basic test suite was\nwritten, 'git -C <dir>' was added. By using -C, we no longer need a\nreference to the base directory for the test. This simplifies callsites,\nand will make the addition of other arguments in later patches more\nreadable.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n t/t7400-submodule-basic.sh | 40 +++++++++++++++-----------------------\n 1 file changed, 16 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex cb1b8e35db..d69a5c0032 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -107,23 +107,15 @@ test_expect_success 'setup - repository to add submodules to' '\n # generates, which will expand symbolic links.\n submodurl=$(pwd -P)\n \n-listbranches() {\n-\tgit for-each-ref --format='%(refname)' 'refs/heads/*'\n-}\n-\n inspect() {\n-\tdir=$1 &&\n-\tdotdot=\"${2:-..}\" &&\n-\n-\t(\n-\t\tcd \"$dir\" &&\n-\t\tlistbranches >\"$dotdot/heads\" &&\n-\t\t{ git symbolic-ref HEAD || :; } >\"$dotdot/head\" &&\n-\t\tgit rev-parse HEAD >\"$dotdot/head-sha1\" &&\n-\t\tgit update-index --refresh &&\n-\t\tgit diff-files --exit-code &&\n-\t\tgit clean -n -d -x >\"$dotdot/untracked\"\n-\t)\n+\tsub_dir=$1 &&\n+\n+\tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n+\t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n+\tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n+\tgit -C \"$sub_dir\" update-index --refresh &&\n+\tgit -C \"$sub_dir\" diff-files --exit-code &&\n+\tgit -C \"$sub_dir\" clean -n -d -x >untracked\n }\n \n test_expect_success 'submodule add' '\n@@ -146,7 +138,7 @@ test_expect_success 'submodule add' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod ../.. &&\n+\tinspect addtest/submod &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -248,7 +240,7 @@ test_expect_success 'submodule add --branch' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod-branch ../.. &&\n+\tinspect addtest/submod-branch &&\n \ttest_cmp expect-heads heads &&\n \ttest_cmp expect-head head &&\n \ttest_must_be_empty untracked\n@@ -264,7 +256,7 @@ test_expect_success 'submodule add with ./ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotsubmod/frotz ../../.. &&\n+\tinspect addtest/dotsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -280,7 +272,7 @@ test_expect_success 'submodule add with /././ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotslashdotsubmod/frotz ../../.. &&\n+\tinspect addtest/dotslashdotsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -296,7 +288,7 @@ test_expect_success 'submodule add with // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/slashslashsubmod/frotz ../../.. &&\n+\tinspect addtest/slashslashsubmod/frotz &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -312,7 +304,7 @@ test_expect_success 'submodule add with /.. in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod ../.. &&\n+\tinspect addtest/realsubmod &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -328,7 +320,7 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod2 ../.. &&\n+\tinspect addtest/realsubmod2 &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -359,7 +351,7 @@ test_expect_success 'submodule add in subdirectory' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod3 ../.. &&\n+\tinspect addtest/realsubmod3 &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n-- \n2.34.0.rc0.344.g81b53c2807-goog\n\n"},{"id":"440493","messageId":"20211104234942.3473650-3-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211104234942.3473650-1-emilyshaffer@google.com","subject":"[PATCH v5 2/4] introduce submodule.superprojectGitDir record","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:49:40Z","receivedAt":"2021-11-04T23:49:52Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Teach submodules a reference to their superproject's gitdir. This allows\nus to A) know that we're running from a submodule, and B) have a\nshortcut to the superproject's vitals, for example, configs.\n\nBy using a relative path instead of an absolute path, we can move the\nsuperproject directory around on the filesystem without breaking the\nsubmodule's cache. And by using the path from gitdir to gitdir, we can\nmove the submodule within the superproject's tree structure without\nbreaking the submodule's cache, too. Finally, by pointing at\n\"get_git_common_dir()\" instead of \"get_git_dir()\", we ensure the link\nwill refer to the parent repo, not to a specific worktree.\n\nSince this hint value is only introduced during new submodule creation\nvia `git submodule add`, though, there is more work to do to allow the\nrecord to be created at other times.\n\nIf the new config is present, we can do some optional value-added\nbehavior, like letting \"git status\" print additional info about the\nsubmodule's status in relation to its superproject, or like letting the\nsuperproject and submodule share an additional config file separate from\neither one's local config.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config/submodule.txt | 12 +++++++++++\n builtin/submodule--helper.c        |  4 ++++\n t/t7400-submodule-basic.sh         | 32 ++++++++++++++++++++----------\n 3 files changed, 38 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt\nindex ee454f8126..8cc57fe1c1 100644\n--- a/Documentation/config/submodule.txt\n+++ b/Documentation/config/submodule.txt\n@@ -91,3 +91,15 @@ submodule.alternateErrorStrategy::\n \t`ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore`\n \tor `info`, and if there is an error with the computed alternate, the\n \tclone proceeds as if no alternate was specified.\n+\n+submodule.superprojectGitDir::\n+\tThe relative path from the submodule's gitdir to its superproject's\n+\tcommon dir. When Git is run in a repository, it usually makes no\n+\tdifference whether this repository is standalone or a submodule, but if\n+\tthis configuration variable is present, additional behavior may be\n+\tpossible, such as \"git status\" printing additional information about\n+\tthis submodule's status with respect to its superproject. This config\n+\tshould only be present in projects which are submodules, but is not\n+\tguaranteed to be present in every submodule, so only optional\n+\tvalue-added behavior should be linked to it. It is set automatically\n+\tduring submodule creation.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6298cbdd4e..f803812225 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1838,6 +1838,10 @@ static int clone_submodule(struct module_clone_data *clone_data)\n \t\tgit_config_set_in_file(p, \"submodule.alternateErrorStrategy\",\n \t\t\t\t       error_strategy);\n \n+\tgit_config_set_in_file(p, \"submodule.superprojectGitdir\",\n+\t\t\t       relative_path(absolute_path(get_git_common_dir()),\n+\t\t\t\t\t     sm_gitdir, &sb));\n+\n \tfree(sm_alternate);\n \tfree(error_strategy);\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex d69a5c0032..3c20d42fbe 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -109,12 +109,24 @@ submodurl=$(pwd -P)\n \n inspect() {\n \tsub_dir=$1 &&\n+\tsuper_dir=$2 &&\n \n \tgit -C \"$sub_dir\" for-each-ref --format='%(refname)' 'refs/heads/*' >heads &&\n \t{ git -C \"$sub_dir\" symbolic-ref HEAD || :; } >head &&\n \tgit -C \"$sub_dir\" rev-parse HEAD >head-sha1 &&\n \tgit -C \"$sub_dir\" update-index --refresh &&\n \tgit -C \"$sub_dir\" diff-files --exit-code &&\n+\n+\t# Ensure that submodule.superprojectGitDir contains the path from the\n+\t# submodule's gitdir to the superproject's gitdir.\n+\n+\tsuper_abs_gitdir=$(git -C \"$super_dir\" rev-parse --path-format=absolute --git-common-dir) &&\n+\tsub_abs_gitdir=$(git -C \"$sub_dir\" rev-parse --path-format=absolute --git-common-dir) &&\n+\n+\t[ \"$(git -C \"$sub_dir\" config --get submodule.superprojectGitDir)\" = \\\n+\t  \"$(test-tool path-utils relative_path \"$super_abs_gitdir\" \\\n+\t\t\t\t\t\t\"$sub_abs_gitdir\")\" ] &&\n+\n \tgit -C \"$sub_dir\" clean -n -d -x >untracked\n }\n \n@@ -138,7 +150,7 @@ test_expect_success 'submodule add' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod &&\n+\tinspect addtest/submod addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -240,7 +252,7 @@ test_expect_success 'submodule add --branch' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/submod-branch &&\n+\tinspect addtest/submod-branch addtest &&\n \ttest_cmp expect-heads heads &&\n \ttest_cmp expect-head head &&\n \ttest_must_be_empty untracked\n@@ -256,7 +268,7 @@ test_expect_success 'submodule add with ./ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotsubmod/frotz &&\n+\tinspect addtest/dotsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -272,7 +284,7 @@ test_expect_success 'submodule add with /././ in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/dotslashdotsubmod/frotz &&\n+\tinspect addtest/dotslashdotsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -288,7 +300,7 @@ test_expect_success 'submodule add with // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/slashslashsubmod/frotz &&\n+\tinspect addtest/slashslashsubmod/frotz addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -304,7 +316,7 @@ test_expect_success 'submodule add with /.. in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod &&\n+\tinspect addtest/realsubmod addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -320,7 +332,7 @@ test_expect_success 'submodule add with ./, /.. and // in path' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod2 &&\n+\tinspect addtest/realsubmod2 addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -351,7 +363,7 @@ test_expect_success 'submodule add in subdirectory' '\n \t) &&\n \n \trm -f heads head untracked &&\n-\tinspect addtest/realsubmod3 &&\n+\tinspect addtest/realsubmod3 addtest &&\n \ttest_cmp expect heads &&\n \ttest_cmp expect head &&\n \ttest_must_be_empty untracked\n@@ -492,7 +504,7 @@ test_expect_success 'update should work when path is an empty dir' '\n \tgit submodule update -q >update.out &&\n \ttest_must_be_empty update.out &&\n \n-\tinspect init &&\n+\tinspect init . &&\n \ttest_cmp expect head-sha1\n '\n \n@@ -551,7 +563,7 @@ test_expect_success 'update should checkout rev1' '\n \techo \"$rev1\" >expect &&\n \n \tgit submodule update init &&\n-\tinspect init &&\n+\tinspect init . &&\n \n \ttest_cmp expect head-sha1\n '\n-- \n2.34.0.rc0.344.g81b53c2807-goog\n\n"},{"id":"440494","messageId":"20211104234942.3473650-4-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211104234942.3473650-1-emilyshaffer@google.com","subject":"[PATCH v5 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:49:41Z","receivedAt":"2021-11-04T23:49:55Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Already during 'git submodule add' we record a pointer to the\nsuperproject's gitdir. However, this doesn't help brand-new\nsubmodules created with 'git init' and later absorbed with 'git\nsubmodule absorbgitdirs'. Let's start adding that pointer during 'git\nsubmodule absorbgitdirs' too.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n submodule.c                        |  9 ++++++\n t/t7412-submodule-absorbgitdirs.sh | 50 ++++++++++++++++++++++++++++--\n 2 files changed, 57 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex c689070524..2e54dcf1a2 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2097,6 +2097,7 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n \tchar *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;\n \tstruct strbuf new_gitdir = STRBUF_INIT;\n \tconst struct submodule *sub;\n+\tstruct strbuf config_path = STRBUF_INIT, sb = STRBUF_INIT;\n \n \tif (submodule_uses_worktrees(path))\n \t\tdie(_(\"relocate_gitdir for submodule '%s' with \"\n@@ -2127,6 +2128,14 @@ static void relocate_single_git_dir_into_superproject(const char *path)\n \n \trelocate_gitdir(path, real_old_git_dir, real_new_git_dir);\n \n+\t/* cache pointer to superproject's gitdir */\n+\tstrbuf_addf(&config_path, \"%s/config\", real_new_git_dir);\n+\tgit_config_set_in_file(config_path.buf, \"submodule.superprojectGitdir\",\n+\t\t\t       relative_path(absolute_path(get_git_common_dir()),\n+\t\t\t\t\t     real_new_git_dir, &sb));\n+\n+\tstrbuf_release(&config_path);\n+\tstrbuf_release(&sb);\n \tfree(old_git_dir);\n \tfree(real_old_git_dir);\n \tfree(real_new_git_dir);\ndiff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh\nindex 1cfa150768..b6f229043d 100755\n--- a/t/t7412-submodule-absorbgitdirs.sh\n+++ b/t/t7412-submodule-absorbgitdirs.sh\n@@ -30,7 +30,17 @@ test_expect_success 'absorb the git dir' '\n \tgit status >actual.1 &&\n \tgit -C sub1 rev-parse HEAD >actual.2 &&\n \ttest_cmp expect.1 actual.1 &&\n-\ttest_cmp expect.2 actual.2\n+\ttest_cmp expect.2 actual.2 &&\n+\n+\t# make sure the submodule cached the superproject gitdir correctly\n+\tsubmodule_gitdir=\"$(git -C sub1 rev-parse --path-format=absolute --git-common-dir)\" &&\n+\tsuperproject_gitdir=\"$(git rev-parse --path-format=absolute --git-common-dir)\" &&\n+\n+\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n+\t\t\"$submodule_gitdir\" >expect &&\n+\tgit -C sub1 config submodule.superprojectGitDir >actual &&\n+\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'absorbing does not fail for deinitialized submodules' '\n@@ -61,7 +71,16 @@ test_expect_success 'absorb the git dir in a nested submodule' '\n \tgit status >actual.1 &&\n \tgit -C sub1/nested rev-parse HEAD >actual.2 &&\n \ttest_cmp expect.1 actual.1 &&\n-\ttest_cmp expect.2 actual.2\n+\ttest_cmp expect.2 actual.2 &&\n+\n+\tsub1_gitdir=\"$(git -C sub1 rev-parse --path-format=absolute --git-common-dir)\" &&\n+\tsub1_nested_gitdir=\"$(git -C sub1/nested rev-parse --path-format=absolute --git-common-dir)\" &&\n+\n+\ttest-tool path-utils relative_path \"$sub1_gitdir\" \"$sub1_nested_gitdir\" \\\n+\t\t>expect &&\n+\tgit -C sub1/nested config submodule.superprojectGitDir >actual &&\n+\n+\ttest_cmp expect actual\n '\n \n test_expect_success 're-setup nested submodule' '\n@@ -130,4 +149,31 @@ test_expect_success 'absorbing fails for a submodule with multiple worktrees' '\n \ttest_i18ngrep \"not supported\" error\n '\n \n+test_expect_success 'absorbgitdirs works when called from a superproject worktree' '\n+\t# set up a worktree of the superproject\n+\tgit worktree add wt &&\n+\t(\n+\tcd wt &&\n+\n+\t# create a new unembedded git dir\n+\tgit init sub4 &&\n+\ttest_commit -C sub4 first &&\n+\tgit submodule add ./sub4 &&\n+\ttest_tick &&\n+\n+\t# absorb the git dir\n+\tgit submodule absorbgitdirs sub4 &&\n+\n+\t# make sure the submodule cached the superproject gitdir correctly\n+\tsubmodule_gitdir=\"$(git -C sub4 rev-parse --path-format=absolute --git-common-dir)\" &&\n+\tsuperproject_gitdir=\"$(git rev-parse --path-format=absolute --git-common-dir)\" &&\n+\n+\ttest-tool path-utils relative_path \"$superproject_gitdir\" \\\n+\t\t\"$submodule_gitdir\" >expect &&\n+\tgit -C sub4 config submodule.superprojectGitDir >actual &&\n+\n+\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.34.0.rc0.344.g81b53c2807-goog\n\n"},{"id":"440495","messageId":"20211104234942.3473650-5-emilyshaffer@google.com","threadId":"56712","inReplyTo":"20211104234942.3473650-1-emilyshaffer@google.com","subject":"[PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-04T23:49:42Z","receivedAt":"2021-11-04T23:49:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"A recorded hint path to the superproject's gitdir might be added during\n'git submodule add', but in some cases - like submodules which were\ncreated before 'git submodule add' learned to record that info - it might\nbe useful to update the hint. Let's do it during 'git submodule\nupdate', when we already have a handle to the superproject while calling\noperations on the submodules.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n git-submodule.sh            | 14 ++++++++++++++\n t/t7406-submodule-update.sh | 12 ++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 652861aa66..873d64eb99 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -449,6 +449,20 @@ cmd_update()\n \t\t\t;;\n \t\tesac\n \n+\t\t# Cache a pointer to the superproject's common dir. This may have\n+\t\t# changed, unless it's a fresh clone. Writes it to worktree\n+\t\t# if applicable, otherwise to local.\n+\t\tif test -z \"$just_cloned\"\n+\t\tthen\n+\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n+\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n+\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n+\t\t\t\t\t\t\t --git-common-dir)\"\n+\n+\t\t\tgit -C \"$sm_path\" config --worktree \\\n+\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n+\t\tfi\n+\n \t\tif test -n \"$recursive\"\n \t\tthen\n \t\t\t(\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 11cccbb333..5146237abc 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -1061,4 +1061,16 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s\n \t)\n '\n \n+test_expect_success 'submodule update adds superproject gitdir to older repos' '\n+\t(cd super &&\n+\t git -C submodule config --unset submodule.superprojectGitdir &&\n+\t git submodule update &&\n+\t test-tool path-utils relative_path \\\n+\t\t\"$(git rev-parse --path-format=absolute --git-common-dir)\" \\\n+\t\t\"$(git -C submodule rev-parse --path-format=absolute --git-common-dir)\" >expect &&\n+\t git -C submodule config submodule.superprojectGitdir >actual &&\n+\t test_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.34.0.rc0.344.g81b53c2807-goog\n\n"},{"id":"440502","messageId":"xmqqzgqjxl4e.fsf@gitster.g","threadId":"56712","inReplyTo":"20211104234942.3473650-5-emilyshaffer@google.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-05T04:49:21Z","receivedAt":"2021-11-05T04:49:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> A recorded hint path to the superproject's gitdir might be added during\n> 'git submodule add', but in some cases - like submodules which were\n> created before 'git submodule add' learned to record that info - it might\n> be useful to update the hint. Let's do it during 'git submodule\n> update', when we already have a handle to the superproject while calling\n> operations on the submodules.\n\nWe are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\ninvalidate it, or if we have such a record, do we blindly trust it\nand use it without verifying if it is still fresh?\n\nAlso, this step and the previous step both say we record gitdir on\ntheir title, but we instead record common dir.  Whichever is the\nright choice to record, let's be consistent.\n\nThanks.\n"},{"id":"440508","messageId":"xmqqo86zxcqp.fsf@gitster.g","threadId":"56712","inReplyTo":"20211104234942.3473650-3-emilyshaffer@google.com","subject":"Re: [PATCH v5 2/4] introduce submodule.superprojectGitDir record","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-05T07:50:22Z","receivedAt":"2021-11-05T07:50:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> By using a relative path instead of an absolute path, we can move the\n> superproject directory around on the filesystem without breaking the\n> submodule's cache. And by using the path from gitdir to gitdir, we can\n> move the submodule within the superproject's tree structure without\n> breaking the submodule's cache, too.\n\nAll of the above explains what the design choice is, and what\nbenefit the chosen design brings us.  But this last one ...\n\n> Finally, by pointing at \"get_git_common_dir()\" instead of\n> \"get_git_dir()\", we ensure the link will refer to the parent repo,\n> not to a specific worktree.\n\n... only says that we choose to point at the common one, not a\nspecific worktree (i.e. what behaviour was chosen by the design),\nbut it is left unclear what benefit it is trying to bring us.\n\nThinking aloud, imagine that there are two worktrees for the\nsuperproject.  For simplicity, let's further imagine that these\nworktrees have a clean check-out of the same commit, hence, these\ntwo worktrees have the same commit from the same submodule checked\nout at the same location relative to the project root.\n\nThe subdirectory in each of these two superproject worktrees that\nchecks out the submodule has .git file (as we \"absorb\" the gitdir in\nthe modern submodule layout) pointing at somewhere.  It probably is\nOK if they point at the same place, but it might be more natural if\nthese two submodule checkouts are two worktrees for a single\nsubmodule repository (this part I am not very clear, and that is why\nI labeled the discussion \"thinking aloud\").\n\nIt seems to me that both design choices would have equally valid\narguments for them.  If both of these submodule worktrees point at\nthe \"common\" dir of the superproject, because the \"common\" one is\npart of the primary worktree, which is harder to lose than secondary\nworktrees, of the superproject, it is presumably harder to go stale\nwhen \"worktree rm\" is done at superproject, which may be a plus.\nBut then from the \"cached\" pointer, each of these submodule\nworktrees cannot tell which worktree of the superproject they are\nchecked out as a part of.  Of course we can go to the root level of\nthe submodule worktree and do the usual repository discovery to\nlearn where the root level of the superproject worktree it belongs\nto, but it somehow feels that it defeats half the point of having\nthis \"cache\" information.\n\nIf we instead point at the git-dir, from there, it is just one level\nof indirection to find out where the common dir of the superproject\nis.\n\n> If the new config is present, we can do some optional value-added\n> behavior, like letting \"git status\" print additional info about the\n> submodule's status in relation to its superproject, or like letting the\n> superproject and submodule share an additional config file separate from\n> either one's local config.\n\nAnd one value-add that I would think of off the top of my head is to\nshow \"we have commit X checked out, which is 4 commits on top of\nwhat the superproject's index records for this submodule\" when \"git\nstatus\" is run in the submodule's worktree.  I do not know that is\nan operation to specifically optimize for, but by choosing to\n\"cache\" the common dir, not the one specific to the worktree of the\nsuperporject our submodule checkout is a part of, the chosen design\nseems to make it harder to implement?\n\nThanks.\n"},{"id":"440513","messageId":"211105.861r3vhtot.gmgdl@evledraar.gmail.com","threadId":"56712","inReplyTo":"xmqqzgqjxl4e.fsf@gitster.g","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T08:43:56Z","receivedAt":"2021-11-05T08:51:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 04 2021, Junio C Hamano wrote:\n\n> Emily Shaffer <emilyshaffer@google.com> writes:\n>\n>> A recorded hint path to the superproject's gitdir might be added during\n>> 'git submodule add', but in some cases - like submodules which were\n>> created before 'git submodule add' learned to record that info - it might\n>> be useful to update the hint. Let's do it during 'git submodule\n>> update', when we already have a handle to the superproject while calling\n>> operations on the submodules.\n>\n> We are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\n> invalidate it, or if we have such a record, do we blindly trust it\n> and use it without verifying if it is still fresh?\n>\n> Also, this step and the previous step both say we record gitdir on\n> their title, but we instead record common dir.  Whichever is the\n> right choice to record, let's be consistent.\n\nI had similar (AFAICT still unaddressed) feedback on the v2[1]. I'd lost\ntrack of this series, and see one reason is that the In-Reply-Chain was\nbroken between v3..v4.\n\nI.e. it seems to me that this whole thing started as a way to avoid\nshellscript overhead by calling git-rev-parse from git-submodule.sh, but\nnow that the relevant bits are moved to C we could just call some\nslightly adjusted code in setup.c.\n\nMaybe I'm entirely wrong, but I think if I am that this series has a\ncommit message gap between the \"hint\" and \"cache\" and this really *does*\nneed to be written at clone/init/update time in some way that I've\nmissed.\n\nOtherwise I still don't really get it, sorry. I.e. maybe the relevant\ncode in setup.c really does need caching, or maybe it doesn't anymore,\nand this cache-via-config is a hold-over from when figuring out the\nparent repo implied needing to shell out somewhere for every operation.\n\n1. https://lore.kernel.org/git/87r1cnfkqx.fsf@evledraar.gmail.com/\n"},{"id":"440516","messageId":"211105.86wnlngebr.gmgdl@evledraar.gmail.com","threadId":"56712","inReplyTo":"20211104234942.3473650-5-emilyshaffer@google.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-05T08:51:12Z","receivedAt":"2021-11-05T09:08:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 04 2021, Emily Shaffer wrote:\n\n> A recorded hint path to the superproject's gitdir might be added during\n> 'git submodule add', but in some cases - like submodules which were\n> created before 'git submodule add' learned to record that info - it might\n> be useful to update the hint. Let's do it during 'git submodule\n> update', when we already have a handle to the superproject while calling\n> operations on the submodules.\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>  git-submodule.sh            | 14 ++++++++++++++\n>  t/t7406-submodule-update.sh | 12 ++++++++++++\n>  2 files changed, 26 insertions(+)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 652861aa66..873d64eb99 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -449,6 +449,20 @@ cmd_update()\n>  \t\t\t;;\n>  \t\tesac\n>  \n> +\t\t# Cache a pointer to the superproject's common dir. This may have\n> +\t\t# changed, unless it's a fresh clone. Writes it to worktree\n> +\t\t# if applicable, otherwise to local.\n> +\t\tif test -z \"$just_cloned\"\n> +\t\tthen\n> +\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n> +\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n> +\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n> +\t\t\t\t\t\t\t --git-common-dir)\"\n> +\n> +\t\t\tgit -C \"$sm_path\" config --worktree \\\n> +\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n> +\t\tfi\n> +\n\nAside from the \"is this really needed anymore?\" comment on this caching\nstrategy in general I had in [1] does this really need to be adding new\nshell code to git-submodule.sh given that we're actively trying to get\nrid of it entirely and move it over to C.\n\nI.e. here we've just called \"git submodule--helper\nrun-update-procedure\", and we pass it \"$sm_path\" (but not \"$recursive\",\nbut could easily do so).\n\nIt needs to clone this new submodule, so presumably it already has a\n\"$sm_gitdir\" equivalent, and we can turn that into the same relative\npath, no?\n\nCan't we call this with a git_config_set*() somewhere in that code?\n\n*investigates a bit*\n\nOkey, so I see that [2] is part of a series that Atharva Raykar had a\nversion of (including this new git-submodule.sh code above) [3] rebased\non top of an older version of this topic. I.e. this bit is that part of that patch:\n\n+       git_config_set_in_file(config_path, \"submodule.superprojectGitdir\",\n+                              relative_path(get_git_dir(),\n+                                            update_data->sm_path, &sb));\n\nI also vaguely recall that Junio ejected his topic to make room for this\ntopic first.\n\nMaybe I've missed some update on this but is his [2][3] broken in\ncombination with your topic here? And re[1] is whatever \"caching\" is\nbeing done here still beneficial once this is all moved to C?\n\nIn your [4] there seems to be an agreement to do it the other way\naround, but as noted in the mail linked from [1] maybe the caching isn't\nneeded anymore then?\n\n1. https://lore.kernel.org/git/211105.861r3vhtot.gmgdl@evledraar.gmail.com/\n2. https://lore.kernel.org/git/20211013051805.45662-8-raykar.ath@gmail.com/\n3. https://lore.kernel.org/git/20211013051805.45662-1-raykar.ath@gmail.com/\n4. https://lore.kernel.org/git/YWiXL+plA7GHfuVv@google.com/\n"},{"id":"440656","messageId":"b42a3c00-2c6c-5cb6-2a3c-0aad095e6fef@gmail.com","threadId":"56712","inReplyTo":"YYRrH6Hw/C1qj5MV@google.com","subject":"Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-08T01:07:00Z","receivedAt":"2021-11-08T01:07:06Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/4/2021 7:22 PM, Emily Shaffer wrote:\n> On Mon, Oct 25, 2021 at 12:14:07PM -0400, Derrick Stolee wrote:\n>> Since you're setting this config value inside the submodule's config,\n>> what does it mean for a submodule to also be a worktree (and hence\n>> require config.worktree)? What happens in this rough scenario?\n>>\n>>   1. git init sub\n>>   2. git init super\n>>   3. git -C sub worktree add super/sub\n>>   4. git -C super submodule absorbgitdir sub\n>>\n>> I haven't actually tried running these things, but it seems unusual\n>> and unexpected. This doesn't even account for cases where the repo\n>> root and a worktree are both submodules within the superproject.\n>>\n>> If we already have protections preventing these worktrees as\n>> submodules, then perhaps there is no need for work here. I'm not\n>> familiar enough with the area to make a claim one way or another.\n> \n> Yeah, I think there is actually a test case covering this in t7412:\n...\n> 147 test_expect_success 'absorbing fails for a submodule with multiple worktrees' '\n> 148         test_must_fail git submodule absorbgitdirs sub3 2>error &&\n> 149         test_i18ngrep \"not supported\" error\n> 150 '\n> \n> That is, I think because 'sub/' in your scenario above has multiple\n> worktrees, the absorbgitdirs will fail. So I won't do additional work\n> here. Thanks.\n\nGood to hear. Thanks for tracking that down.\n\n-Stolee\n"},{"id":"440657","messageId":"920c3133-b6ee-b82c-0876-f06713e9337b@gmail.com","threadId":"56712","inReplyTo":"20211104234942.3473650-1-emilyshaffer@google.com","subject":"Re: [PATCH v5 0/4] cache parent project's gitdir in submodules","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-08T01:24:43Z","receivedAt":"2021-11-08T01:24:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/4/2021 7:49 PM, Emily Shaffer wrote:\n\n> The only real change here is a slight semantics change to map from\n> <submodule gitdir> to <superproject common git dir>. In every case\n> *except* for when the superproject has a worktree, this changes nothing.\n> For the case when the superproject has a worktree, this means that now\n> submodules will refer to the general superproject common dir (e.g. no\n> worktree-specific refs or configs or whatnot).\n> \n> I *think* that because a submodule should exist in the context of the\n> common dir, not the worktree gitdir, that is ok. However, it does mean\n> it would be difficult to do something like sharing a config specific to\n> the worktree (the initial goal of this series).\n> \n> $ROOT/.git\n> $ROOT/.git/config.superproject <- shared by $ROOT/.git/modules/sub\n> $ROOT/.git/modules/sub <- points to $ROOT/.git\n> $ROOT/.git/worktrees/wt\n> $ROOT/.git/worktrees/wt/config.superproject <- contains a certain config-based pre-commit hook\n> \n> If the submodule only knows about the common dir, that is tough, because\n> the submodule would basically have to guess which worktree it's in from\n> its own path. There would be no way for '$WT/sub' to inherit\n> '$ROOT/.git/worktrees/wt/config.superproject'.\n> \n> That said... right now, we don't support submodules in worktrees very\n> well at all. A submodule in a worktree will get a brand new gitdir in\n> $ROOT/.git/worktrees/modules/ (and that brand new gitdir would point to\n> the super's common dir). So I think we can punt on this entire question\n> until we teach submodules and worktrees to play more gracefully together\n> (it's on my long list...),\n\n(I omit a portion that will be discussed later.)\n\n> Or, to summarize the long ramble above: \"this is still kind of weird\n> with worktrees, but let's fix it later when we fix worktrees more\n> thoroughly\".\n\nI'm concerned about punting here, because making a messy situation worse\nis unlikely to have a clean way out. Could we set up a design that works\nwith superproject worktrees?\n\nYou mentioned that submodules cannot have worktrees. At least, you said\nthat 'absorbgitdirs' does not allow them. Could those subprojects still\nexist and be registered as submodules without using that command?\n\nWhat I'm trying to hint at is that if the submodules can't have\nworktrees, then maybe we could make their 'config.worktree' files be\nrelative to the superproject worktrees. Then, these submodules could\npoint to the commondir in their base config and _also_ to the worktree\ngitdir in their config.worktree.\n\nThe issue that is immediately obvious here is that my definition is\ncircular: we need to know the superproject worktree in order to discover\nthe config.worktree which contains the information about the superproject\nworktree.\n\n> and at that time we can probably introduce a\n> pointer from $ROOT/.git/modules/sub/worktrees/wt/ to\n> $ROOT/.git/worktrees/wt/....\n\nYour idea here appears to assume that if the superproject has worktrees,\nthen the submodule is divided into worktrees in an exact correspondence.\nThis would allow the submodule's config.worktree to point to the\nsuperproject's worktree (or possibly it could be inferred from the\nsubmodule's worktree relative to the submodule's commondir).\n\nThis seems like an interesting way forward, but requires changing how\n'git absorbgitdirs' works, along with changes to 'git worktree' or other\nsubmodule commands when the submodule first appears during a 'git checkout'\nin a worktree. I imagine there are a lot of \"gotchas\" here. It is worth\nspending some time imagining how to create this setup and/or enforce it\nas submodules are added in the lifecycle of a repository, if only to\nvalidate the config design presented by this series.\n\nThanks,\n-Stolee\n"},{"id":"440700","messageId":"YYmuqEQUaB1a8Gs1@google.com","threadId":"56712","inReplyTo":"920c3133-b6ee-b82c-0876-f06713e9337b@gmail.com","subject":"Re: [PATCH v5 0/4] cache parent project's gitdir in submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-08T23:11:36Z","receivedAt":"2021-11-08T23:11:44Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Sun, Nov 07, 2021 at 08:24:43PM -0500, Derrick Stolee wrote:\n> \n> On 11/4/2021 7:49 PM, Emily Shaffer wrote:\n> \n> > The only real change here is a slight semantics change to map from\n> > <submodule gitdir> to <superproject common git dir>. In every case\n> > *except* for when the superproject has a worktree, this changes nothing.\n> > For the case when the superproject has a worktree, this means that now\n> > submodules will refer to the general superproject common dir (e.g. no\n> > worktree-specific refs or configs or whatnot).\n> > \n> > I *think* that because a submodule should exist in the context of the\n> > common dir, not the worktree gitdir, that is ok. However, it does mean\n> > it would be difficult to do something like sharing a config specific to\n> > the worktree (the initial goal of this series).\n> > \n> > $ROOT/.git\n> > $ROOT/.git/config.superproject <- shared by $ROOT/.git/modules/sub\n> > $ROOT/.git/modules/sub <- points to $ROOT/.git\n> > $ROOT/.git/worktrees/wt\n> > $ROOT/.git/worktrees/wt/config.superproject <- contains a certain config-based pre-commit hook\n> > \n> > If the submodule only knows about the common dir, that is tough, because\n> > the submodule would basically have to guess which worktree it's in from\n> > its own path. There would be no way for '$WT/sub' to inherit\n> > '$ROOT/.git/worktrees/wt/config.superproject'.\n> > \n> > That said... right now, we don't support submodules in worktrees very\n> > well at all. A submodule in a worktree will get a brand new gitdir in\n> > $ROOT/.git/worktrees/modules/ (and that brand new gitdir would point to\n> > the super's common dir). So I think we can punt on this entire question\n> > until we teach submodules and worktrees to play more gracefully together\n> > (it's on my long list...),\n> \n> (I omit a portion that will be discussed later.)\n> \n> > Or, to summarize the long ramble above: \"this is still kind of weird\n> > with worktrees, but let's fix it later when we fix worktrees more\n> > thoroughly\".\n> \n> I'm concerned about punting here, because making a messy situation worse\n> is unlikely to have a clean way out. Could we set up a design that works\n> with superproject worktrees?\n> \n> You mentioned that submodules cannot have worktrees. At least, you said\n> that 'absorbgitdirs' does not allow them. Could those subprojects still\n> exist and be registered as submodules without using that command?\n> \n> What I'm trying to hint at is that if the submodules can't have\n> worktrees, then maybe we could make their 'config.worktree' files be\n> relative to the superproject worktrees. Then, these submodules could\n> point to the commondir in their base config and _also_ to the worktree\n> gitdir in their config.worktree.\n> \n> The issue that is immediately obvious here is that my definition is\n> circular: we need to know the superproject worktree in order to discover\n> the config.worktree which contains the information about the superproject\n> worktree.\n> \n> > and at that time we can probably introduce a\n> > pointer from $ROOT/.git/modules/sub/worktrees/wt/ to\n> > $ROOT/.git/worktrees/wt/....\n> \n> Your idea here appears to assume that if the superproject has worktrees,\n> then the submodule is divided into worktrees in an exact correspondence.\n> This would allow the submodule's config.worktree to point to the\n> superproject's worktree (or possibly it could be inferred from the\n> submodule's worktree relative to the submodule's commondir).\n> \n> This seems like an interesting way forward, but requires changing how\n> 'git absorbgitdirs' works, along with changes to 'git worktree' or other\n> submodule commands when the submodule first appears during a 'git checkout'\n> in a worktree. I imagine there are a lot of \"gotchas\" here. It is worth\n> spending some time imagining how to create this setup and/or enforce it\n> as submodules are added in the lifecycle of a repository, if only to\n> validate the config design presented by this series.\n\nYeah, I think we may be overthinking it, especially with the concerns\nabout common dir vs. gitdir. More specifically - I think we accidentally\ndid the right thing in the previous iteration by using the gitdir :)\n\nI think we can probably put it pretty simply:\nsubmodule.superprojectGitDir should point from the most local gitdir of\nthe submodule to the most local gitdir of the superproject.\n\nLuckily there are not so many permutations to worry about here.\n\nSuper doesn't have worktrees, sub doesn't have worktrees:\n.git/\n  modules/\n    sub/\n      config <- contains a pointer to \".git/\"\n  config\n\nSuper doesn't have worktrees, sub does have worktrees (and as you\nsuggest above, right now this would have to be created carefully and\nmanually, but later we probably want this to Just Work):\n.git/\n  modules/\n    sub/\n      config\n      config.worktree <- contains a pointer to \".git/\"\n      worktrees/\n        sub-wt/\n\tconfig <- contains a pointer to \".git/\"\n  config\n\nSuper has worktrees, sub doesn't have worktrees:\nActually, I think in the future this might not be possible, if we want\nto make `git worktree add --recurse-submodules` work gracefully (and I\ndo want that). But in the interim, in practice it looks like this:\n.git/\n  modules/\n    sub/\n      config <- contains a pointer to \".git/\"\n  worktrees/\n    super-wt/\n      modules/\n        sub/\n\t  config <- contains a pointer to \".git/worktrees/super-wt\"\n      config\n  config\nThis case is pretty weird anyway, because in order for a submodule to be\npresent in multiple worktrees of the superproject, the submodule itself\nneeds to either have multiple worktrees or multiple repos. But on the\nflip side, today it's impossible for a single submodule gitdir to need\nto know about more superproject worktrees than the one it was\ninitted/whatever into.\n\nBoth super and sub have worktrees:\nAnd this won't exist until we have graceful support of `git worktree add\n--recurse-submodules` or with some manual effort, now.\n.git/\n  modules/\n    sub/\n      worktrees/\n        sub-wt/\n\t  config <- contains a pointer to \".git/worktrees/super-wt/\"\n      config\n      config.worktree <- contains a pointer to \".git/\"\n  worktrees/\n    super-wt/\n      config\n  config\n  config.worktree\n\nI think this will give us access to both the worktree gitdir *and* the\ncommon gitdir:\n\n  ~/git/.git/worktrees/git-second [GIT_DIR!]$ git rev-parse --git-common-dir\n  /usr/local/google/home/emilyshaffer/git/.git\n\nSo that means from any submodule, we can determine:\n - submodule's gitdir (from the .git link in the submodule wt)\n - submodule's common dir (from existing commands)\n - gitdir of superproject which submodule inhabits (from the config in\n   the submodule's gitdir, or the submodule's config.worktree)\n - common dir of superproject (from existing commands + prior config)\n\nThe upshot to me, then, means that we should be 1) making sure to get\nthe path to the gitdir, not the common dir, of the superproject; and 2)\nusing helpers to write to the worktree config, not to the local config,\nof the submodule. In other words, we want to avoid the following:\n\n.git/\n  modules/\n    sub/\n      worktrees/\n        wt/\n\t  config\n      config <- \"submodule.superprojectGitDir = ../../../..\" as written by the worktree\n\nWill take a look at the rest of the comments too, but this sounds like a\nreasonable approach to me.\n\n - Emily\n\n> \n> Thanks,\n> -Stolee\n"},{"id":"440701","messageId":"YYmvt+wFQ97QLaO8@google.com","threadId":"56712","inReplyTo":"xmqqo86zxcqp.fsf@gitster.g","subject":"Re: [PATCH v5 2/4] introduce submodule.superprojectGitDir record","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-08T23:16:07Z","receivedAt":"2021-11-08T23:16:16Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Nov 05, 2021 at 12:50:22AM -0700, Junio C Hamano wrote:\n> \n> Emily Shaffer <emilyshaffer@google.com> writes:\n> \n> > By using a relative path instead of an absolute path, we can move the\n> > superproject directory around on the filesystem without breaking the\n> > submodule's cache. And by using the path from gitdir to gitdir, we can\n> > move the submodule within the superproject's tree structure without\n> > breaking the submodule's cache, too.\n> \n> All of the above explains what the design choice is, and what\n> benefit the chosen design brings us.  But this last one ...\n> \n> > Finally, by pointing at \"get_git_common_dir()\" instead of\n> > \"get_git_dir()\", we ensure the link will refer to the parent repo,\n> > not to a specific worktree.\n> \n> ... only says that we choose to point at the common one, not a\n> specific worktree (i.e. what behaviour was chosen by the design),\n> but it is left unclear what benefit it is trying to bring us.\n> \n> Thinking aloud, imagine that there are two worktrees for the\n> superproject.  For simplicity, let's further imagine that these\n> worktrees have a clean check-out of the same commit, hence, these\n> two worktrees have the same commit from the same submodule checked\n> out at the same location relative to the project root.\n> \n> The subdirectory in each of these two superproject worktrees that\n> checks out the submodule has .git file (as we \"absorb\" the gitdir in\n> the modern submodule layout) pointing at somewhere.  It probably is\n> OK if they point at the same place, but it might be more natural if\n> these two submodule checkouts are two worktrees for a single\n> submodule repository (this part I am not very clear, and that is why\n> I labeled the discussion \"thinking aloud\").\n> \n> It seems to me that both design choices would have equally valid\n> arguments for them.  If both of these submodule worktrees point at\n> the \"common\" dir of the superproject, because the \"common\" one is\n> part of the primary worktree, which is harder to lose than secondary\n> worktrees, of the superproject, it is presumably harder to go stale\n> when \"worktree rm\" is done at superproject, which may be a plus.\n> But then from the \"cached\" pointer, each of these submodule\n> worktrees cannot tell which worktree of the superproject they are\n> checked out as a part of.  Of course we can go to the root level of\n> the submodule worktree and do the usual repository discovery to\n> learn where the root level of the superproject worktree it belongs\n> to, but it somehow feels that it defeats half the point of having\n> this \"cache\" information.\n> \n> If we instead point at the git-dir, from there, it is just one level\n> of indirection to find out where the common dir of the superproject\n> is.\n> \n> > If the new config is present, we can do some optional value-added\n> > behavior, like letting \"git status\" print additional info about the\n> > submodule's status in relation to its superproject, or like letting the\n> > superproject and submodule share an additional config file separate from\n> > either one's local config.\n> \n> And one value-add that I would think of off the top of my head is to\n> show \"we have commit X checked out, which is 4 commits on top of\n> what the superproject's index records for this submodule\" when \"git\n> status\" is run in the submodule's worktree.  I do not know that is\n> an operation to specifically optimize for, but by choosing to\n> \"cache\" the common dir, not the one specific to the worktree of the\n> superporject our submodule checkout is a part of, the chosen design\n> seems to make it harder to implement?\n\nYeah, that is compelling. Sorry not to reply in depth, but I am\nconvinced.\n\n - Emily\n"},{"id":"440702","messageId":"YYmw8vEyFnQpe58z@google.com","threadId":"56712","inReplyTo":"211105.861r3vhtot.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-08T23:21:22Z","receivedAt":"2021-11-08T23:21:30Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Nov 05, 2021 at 09:43:56AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> \n> On Thu, Nov 04 2021, Junio C Hamano wrote:\n> \n> > Emily Shaffer <emilyshaffer@google.com> writes:\n> >\n> >> A recorded hint path to the superproject's gitdir might be added during\n> >> 'git submodule add', but in some cases - like submodules which were\n> >> created before 'git submodule add' learned to record that info - it might\n> >> be useful to update the hint. Let's do it during 'git submodule\n> >> update', when we already have a handle to the superproject while calling\n> >> operations on the submodules.\n> >\n> > We are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\n> > invalidate it, or if we have such a record, do we blindly trust it\n> > and use it without verifying if it is still fresh?\n> >\n> > Also, this step and the previous step both say we record gitdir on\n> > their title, but we instead record common dir.  Whichever is the\n> > right choice to record, let's be consistent.\n> \n> I had similar (AFAICT still unaddressed) feedback on the v2[1]. I'd lost\n> track of this series, and see one reason is that the In-Reply-Chain was\n> broken between v3..v4.\n> \n> I.e. it seems to me that this whole thing started as a way to avoid\n> shellscript overhead by calling git-rev-parse from git-submodule.sh, but\n> now that the relevant bits are moved to C we could just call some\n> slightly adjusted code in setup.c.\n\nNo, that is not the case. It is the case that `git -C .. rev-parse\n--git-dir` is *very* expensive in the case where `../` is not, in fact,\na gitdir; when I attempted another series which relied on finding the\nparent superproject's gitdir in this way, our testsuite took something\nlike 5x longer to run than before. In other words, the expensive part is\nnot the shelling out overhead - the expensive part is searching up the\nentire filesystem directory structure in the worst-case (\"we are not a\nsubmodule\") scenario. This is still needed, even with 'git-submodule.sh'\nmoving to C.\n\n> \n> Maybe I'm entirely wrong, but I think if I am that this series has a\n> commit message gap between the \"hint\" and \"cache\" and this really *does*\n> need to be written at clone/init/update time in some way that I've\n> missed.\n> \n> Otherwise I still don't really get it, sorry. I.e. maybe the relevant\n> code in setup.c really does need caching, or maybe it doesn't anymore,\n> and this cache-via-config is a hold-over from when figuring out the\n> parent repo implied needing to shell out somewhere for every operation.\n> \n> 1. https://lore.kernel.org/git/87r1cnfkqx.fsf@evledraar.gmail.com/\n"},{"id":"440703","messageId":"YYmxSptSDRPWJJ3t@google.com","threadId":"56712","inReplyTo":"211105.86wnlngebr.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-08T23:22:50Z","receivedAt":"2021-11-08T23:22:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Nov 05, 2021 at 09:51:12AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> \n> On Thu, Nov 04 2021, Emily Shaffer wrote:\n> \n> > A recorded hint path to the superproject's gitdir might be added during\n> > 'git submodule add', but in some cases - like submodules which were\n> > created before 'git submodule add' learned to record that info - it might\n> > be useful to update the hint. Let's do it during 'git submodule\n> > update', when we already have a handle to the superproject while calling\n> > operations on the submodules.\n> >\n> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> > ---\n> >  git-submodule.sh            | 14 ++++++++++++++\n> >  t/t7406-submodule-update.sh | 12 ++++++++++++\n> >  2 files changed, 26 insertions(+)\n> >\n> > diff --git a/git-submodule.sh b/git-submodule.sh\n> > index 652861aa66..873d64eb99 100755\n> > --- a/git-submodule.sh\n> > +++ b/git-submodule.sh\n> > @@ -449,6 +449,20 @@ cmd_update()\n> >  \t\t\t;;\n> >  \t\tesac\n> >  \n> > +\t\t# Cache a pointer to the superproject's common dir. This may have\n> > +\t\t# changed, unless it's a fresh clone. Writes it to worktree\n> > +\t\t# if applicable, otherwise to local.\n> > +\t\tif test -z \"$just_cloned\"\n> > +\t\tthen\n> > +\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n> > +\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n> > +\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n> > +\t\t\t\t\t\t\t --git-common-dir)\"\n> > +\n> > +\t\t\tgit -C \"$sm_path\" config --worktree \\\n> > +\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n> > +\t\tfi\n> > +\n> \n> Aside from the \"is this really needed anymore?\" comment on this caching\n> strategy in general I had in [1] does this really need to be adding new\n> shell code to git-submodule.sh given that we're actively trying to get\n> rid of it entirely and move it over to C.\n> \n> I.e. here we've just called \"git submodule--helper\n> run-update-procedure\", and we pass it \"$sm_path\" (but not \"$recursive\",\n> but could easily do so).\n> \n> It needs to clone this new submodule, so presumably it already has a\n> \"$sm_gitdir\" equivalent, and we can turn that into the same relative\n> path, no?\n> \n> Can't we call this with a git_config_set*() somewhere in that code?\n> \n> *investigates a bit*\n> \n> Okey, so I see that [2] is part of a series that Atharva Raykar had a\n> version of (including this new git-submodule.sh code above) [3] rebased\n> on top of an older version of this topic. I.e. this bit is that part of that patch:\n> \n> +       git_config_set_in_file(config_path, \"submodule.superprojectGitdir\",\n> +                              relative_path(get_git_dir(),\n> +                                            update_data->sm_path, &sb));\n> \n> I also vaguely recall that Junio ejected his topic to make room for this\n> topic first.\n> \n> Maybe I've missed some update on this but is his [2][3] broken in\n> combination with your topic here? And re[1] is whatever \"caching\" is\n> being done here still beneficial once this is all moved to C?\n> \n> In your [4] there seems to be an agreement to do it the other way\n> around, but as noted in the mail linked from [1] maybe the caching isn't\n> needed anymore then?\n\nI answered vs. your other mail; yes, it's still needed, and last I\nchecked Atharva's series had been kicked out to make room for this one.\n\n> \n> 1. https://lore.kernel.org/git/211105.861r3vhtot.gmgdl@evledraar.gmail.com/\n> 2. https://lore.kernel.org/git/20211013051805.45662-8-raykar.ath@gmail.com/\n> 3. https://lore.kernel.org/git/20211013051805.45662-1-raykar.ath@gmail.com/\n> 4. https://lore.kernel.org/git/YWiXL+plA7GHfuVv@google.com/\n"},{"id":"440713","messageId":"211109.86v912dtfw.gmgdl@evledraar.gmail.com","threadId":"56712","inReplyTo":"YYmw8vEyFnQpe58z@google.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-09T00:42:03Z","receivedAt":"2021-11-09T01:37:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 08 2021, Emily Shaffer wrote:\n\n> On Fri, Nov 05, 2021 at 09:43:56AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> \n>> On Thu, Nov 04 2021, Junio C Hamano wrote:\n>> \n>> > Emily Shaffer <emilyshaffer@google.com> writes:\n>> >\n>> >> A recorded hint path to the superproject's gitdir might be added during\n>> >> 'git submodule add', but in some cases - like submodules which were\n>> >> created before 'git submodule add' learned to record that info - it might\n>> >> be useful to update the hint. Let's do it during 'git submodule\n>> >> update', when we already have a handle to the superproject while calling\n>> >> operations on the submodules.\n>> >\n>> > We are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\n>> > invalidate it, or if we have such a record, do we blindly trust it\n>> > and use it without verifying if it is still fresh?\n>> >\n>> > Also, this step and the previous step both say we record gitdir on\n>> > their title, but we instead record common dir.  Whichever is the\n>> > right choice to record, let's be consistent.\n>> \n>> I had similar (AFAICT still unaddressed) feedback on the v2[1]. I'd lost\n>> track of this series, and see one reason is that the In-Reply-Chain was\n>> broken between v3..v4.\n>> \n>> I.e. it seems to me that this whole thing started as a way to avoid\n>> shellscript overhead by calling git-rev-parse from git-submodule.sh, but\n>> now that the relevant bits are moved to C we could just call some\n>> slightly adjusted code in setup.c.\n>\n> No, that is not the case. It is the case that `git -C .. rev-parse\n> --git-dir` is *very* expensive in the case where `../` is not, in fact,\n> a gitdir; when I attempted another series which relied on finding the\n> parent superproject's gitdir in this way, our testsuite took something\n> like 5x longer to run than before. In other words, the expensive part is\n> not the shelling out overhead - the expensive part is searching up the\n> entire filesystem directory structure in the worst-case (\"we are not a\n> submodule\") scenario. This is still needed, even with 'git-submodule.sh'\n> moving to C.\n\nDo you have that test code somewhere? I tried to reproduce this &\ncan't. I run my tests in /home/avar/*, and just created this:\n\n    $ find /tmp/some/ -name '.git' -type d\n    /tmp/some/dir/.git\n    /tmp/some/dir/a/b/c/d/e/f/g/i/j/k/.git\n\nI.e. a deeply nested structure in /tmp, if you ask for the git-dir in\n/tmp/some/**/k you'll need to search several levels up.\n\nThen with the patch below we'll instrument almost all git commands to\noptionally do that search, i.e. anything that does setup_git_directory()\nat all:\n\n    $ GIT_TEST_SETUP=true GIT_TEST_SETUP_PRINT=true ~/g/git/git rev-parse HEAD\n    warning: from '/tmp/some/dir/a/b/c/d/e/f/g/i/j' found '/tmp/some/dir' ('/tmp/some/dir/.git')\n    <some hash here>\n\nAnd as a quick test to run a few tests I tried:\n\n    rm -rf test-results/; GIT_TEST_SETUP=true GIT_TEST_SETUP_PRINT=true prove -j8 t741*submod*.sh :: -V\n\nWhich runs quickly enough for a tight test loop, and does that work >600 times:\n    \n    $ cat test-results/*.out|grep -c warning.*from.*found\n    662\n\nI can't get that to show me any meaningful difference, just to pick on\none test (since it was easier to run repeatedly):\n    \n    $ hyperfine -L v true,false \"GIT_TEST_SETUP={v} ./t7416-submodule-dash-url.sh --root=/dev/shm/git\"\n    Benchmark #1: GIT_TEST_SETUP=true ./t7416-submodule-dash-url.sh --root=/dev/shm/git\n      Time (mean ± σ):     527.5 ms ±   7.2 ms    [User: 431.6 ms, System: 125.9 ms]\n      Range (min … max):   522.6 ms … 542.5 ms    10 runs\n     \n    Benchmark #2: GIT_TEST_SETUP=false ./t7416-submodule-dash-url.sh --root=/dev/shm/git\n      Time (mean ± σ):     526.7 ms ±  10.8 ms    [User: 421.1 ms, System: 131.6 ms]\n      Range (min … max):   518.2 ms … 546.8 ms    10 runs\n     \n    Summary\n      'GIT_TEST_SETUP=false ./t7416-submodule-dash-url.sh --root=/dev/shm/git' ran\n        1.00 ± 0.02 times faster than 'GIT_TEST_SETUP=true ./t7416-submodule-dash-url.sh --root=/dev/shm/git'\n\nI.e. it's all fuzzy and within the error margins.\n\nNow if we do e.g.:\n\n    GIT_TEST_SETUP=false strace -f -c -U calls,name,time -S calls ./t7416-submodule-dash-url.sh 2>&1 >/dev/null | grep -A9000 calls >a\n    GIT_TEST_SETUP=true strace -f -c -U calls,name,time -S calls ./t7416-submodule-dash-url.sh 2>&1 >/dev/null | grep -A9000 calls >b\n\nWe'll see the syscall difference, in summary:\n\n    -   110086 total            100.00\n    +   114765 total            100.00\n\nAnd some of the real big differences are:\n    \n    $ diff -u <(head -n 12 a) <(head -n 12 b)\n    --- /dev/fd/63  2021-11-09 01:57:16.023991556 +0100\n    +++ /dev/fd/62  2021-11-09 01:57:16.019991593 +0100\n    @@ -1,12 +1,12 @@\n         calls syscall          % time\n     --------- ---------------- ------\n    -    11504 openat             2.15\n    -    11496 close              2.07\n    -    10672 read               4.15\n    -    10465 rt_sigaction       0.32\n    -     9913 lstat              1.50\n    -     9456 mmap               0.67\n    -     7545 stat               0.81\n    -     6349 fstat              0.62\n    -     3896 access             0.61\n    -     3490 mprotect           0.26\n    +    11783 lstat              1.80\n    +    11600 close              1.89\n    +    11599 openat             2.19\n    +    10887 read               4.36\n    +    10465 rt_sigaction       0.33\n    +     9742 stat               1.07\n    +     9455 mmap               0.75\n    +     6346 fstat              0.57\n    +     4113 access             0.65\n    +     3490 mprotect           0.28\n\nBut syscalls are fast, so it doesn't show up in real results.\n\nNow, of course a real implementation could be less stupid, e.g. even if\nwe think we need *a cache* if these are the performance numbers why do\nwe need to risk the cache being incorrect v.s. say just writing \"I am a\nsubmodule\" somewhere in the config (if we don't have that).\n\nThen we'll only do that work for submodules, so not all git invocations\nwill pay the cost (and it this point we'll usually have read config\nanyway).\n\nBut I really just don't think it's that expensive at all. I can see how\nit would be for actually shelling out, but we don't need to do that.\n\nThis could also just be that I'm running this on a really fast FS, which\nis true. So I went and tested on an AIX machine I have access to.\n\nI/O on AIX is slow, *really slow*, so slow that if you \"rm -rfv\"\nsomething you'll have time to read individual lines scrolling by.\n\nThat ~500ms t7416-submodule-dash-url.sh test runs in around 50s on that\nAIX box (power-aix.osuosl.org), most of which is I/O overhead, I created\nthe same /tmp/ directory structure and tried with\nGIT_TEST_SETUP=[false|true] and it's ~55s with/without the env variable,\nwith no clear winner.\n\nI don't have access to hyperfine on that box, or the patience to wait\nfor AIX I/O to wait for meaningful results, but to a first approximation\nthat seems to indicate that it doesn't really matter there either.\n\ndiff --git a/setup.c b/setup.c\nindex 347d7181ae9..8453d397676 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1209,6 +1209,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n        struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n        const char *prefix = NULL;\n        struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n+       const char *str = \"/tmp/some/dir/a/b/c/d/e/f/g/i/j\";\n+       struct strbuf a = STRBUF_INIT, b = STRBUF_INIT;\n \n        /*\n         * We may have read an incomplete configuration before\n@@ -1231,6 +1233,14 @@ const char *setup_git_directory_gently(int *nongit_ok)\n                die_errno(_(\"Unable to read current working directory\"));\n        strbuf_addbuf(&dir, &cwd);\n \n+       if (git_env_bool(\"GIT_TEST_SETUP\", 0)) {\n+               strbuf_addstr(&a, str);\n+               setup_git_directory_gently_1(&a, &b, 0);\n+\n+               if (strcmp(a.buf, str) && git_env_bool(\"GIT_TEST_SETUP_PRINT\", 0))\n+                       warning(\"from '%s' found '%s' ('%s/%s')\", str, a.buf, a.buf, b.buf);\n+       }\n+\n        switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {\n        case GIT_DIR_EXPLICIT:\n                prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n\n"},{"id":"440714","messageId":"211109.86r1bqdsmu.gmgdl@evledraar.gmail.com","threadId":"56712","inReplyTo":"YYmxSptSDRPWJJ3t@google.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-09T01:12:24Z","receivedAt":"2021-11-09T01:40:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 08 2021, Emily Shaffer wrote:\n\n> On Fri, Nov 05, 2021 at 09:51:12AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> \n>> On Thu, Nov 04 2021, Emily Shaffer wrote:\n>> \n>> > A recorded hint path to the superproject's gitdir might be added during\n>> > 'git submodule add', but in some cases - like submodules which were\n>> > created before 'git submodule add' learned to record that info - it might\n>> > be useful to update the hint. Let's do it during 'git submodule\n>> > update', when we already have a handle to the superproject while calling\n>> > operations on the submodules.\n>> >\n>> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n>> > ---\n>> >  git-submodule.sh            | 14 ++++++++++++++\n>> >  t/t7406-submodule-update.sh | 12 ++++++++++++\n>> >  2 files changed, 26 insertions(+)\n>> >\n>> > diff --git a/git-submodule.sh b/git-submodule.sh\n>> > index 652861aa66..873d64eb99 100755\n>> > --- a/git-submodule.sh\n>> > +++ b/git-submodule.sh\n>> > @@ -449,6 +449,20 @@ cmd_update()\n>> >  \t\t\t;;\n>> >  \t\tesac\n>> >  \n>> > +\t\t# Cache a pointer to the superproject's common dir. This may have\n>> > +\t\t# changed, unless it's a fresh clone. Writes it to worktree\n>> > +\t\t# if applicable, otherwise to local.\n>> > +\t\tif test -z \"$just_cloned\"\n>> > +\t\tthen\n>> > +\t\t\tsm_gitdir=\"$(git -C \"$sm_path\" rev-parse --absolute-git-dir)\"\n>> > +\t\t\trelative_gitdir=\"$(git rev-parse --path-format=relative \\\n>> > +\t\t\t\t\t\t\t --prefix \"${sm_gitdir}\" \\\n>> > +\t\t\t\t\t\t\t --git-common-dir)\"\n>> > +\n>> > +\t\t\tgit -C \"$sm_path\" config --worktree \\\n>> > +\t\t\t\tsubmodule.superprojectgitdir \"$relative_gitdir\"\n>> > +\t\tfi\n>> > +\n>> \n>> Aside from the \"is this really needed anymore?\" comment on this caching\n>> strategy in general I had in [1] does this really need to be adding new\n>> shell code to git-submodule.sh given that we're actively trying to get\n>> rid of it entirely and move it over to C.\n>> \n>> I.e. here we've just called \"git submodule--helper\n>> run-update-procedure\", and we pass it \"$sm_path\" (but not \"$recursive\",\n>> but could easily do so).\n>> \n>> It needs to clone this new submodule, so presumably it already has a\n>> \"$sm_gitdir\" equivalent, and we can turn that into the same relative\n>> path, no?\n>> \n>> Can't we call this with a git_config_set*() somewhere in that code?\n>> \n>> *investigates a bit*\n>> \n>> Okey, so I see that [2] is part of a series that Atharva Raykar had a\n>> version of (including this new git-submodule.sh code above) [3] rebased\n>> on top of an older version of this topic. I.e. this bit is that part of that patch:\n>> \n>> +       git_config_set_in_file(config_path, \"submodule.superprojectGitdir\",\n>> +                              relative_path(get_git_dir(),\n>> +                                            update_data->sm_path, &sb));\n>> \n>> I also vaguely recall that Junio ejected his topic to make room for this\n>> topic first.\n>> \n>> Maybe I've missed some update on this but is his [2][3] broken in\n>> combination with your topic here? And re[1] is whatever \"caching\" is\n>> being done here still beneficial once this is all moved to C?\n>> \n>> In your [4] there seems to be an agreement to do it the other way\n>> around, but as noted in the mail linked from [1] maybe the caching isn't\n>> needed anymore then?\n>\n> I answered vs. your other mail; yes, it's still needed, [...]\n\nI replied just now (in [1], but it's not on lore yet, maybe vger ate my\nmail again).\n\ntl;dr: Ran some quick performance numbers, and can't reproduce any\nslowdown at all when instrumenting the test suite to run another\nsetup_git_directory() that'll do a nested lookup on pretty much every\ngit command, and running the test suite (you mentioned a ~5x overall\nslowdown).\n\nAnyway, I think the two replies you've got here only partially address\nwhat I pointed out, in particular in [2]:\n    \n    But I'm a bit iffy on a series that's structured in a way as to not\n    start by asserting that we should have given semantics without the\n    cache, and then adds the cache later as an optional optimization.\n\nI.e. even if it was 10x as slow I think it should still be structured in\nsuch a way as to at least have some GIT_TEST_* knob to turn it off in\nfavor of a slow path.\n\nE.g. we've got commit-graph paths that are probably 100x or 1000x faster\nthan the slow path, but having the cache-less ones means we can validate\ntheir correctness.\n    \n> and last I checked Atharva's series had been kicked out to make room\n> for this one.\n\nIndeed, but as a comment on this proposed series that doesn't really\naddress this in my above-quoted ([3])\n\n    I.e. here we've just called \"git submodule--helper\n    run-update-procedure\", and we pass it \"$sm_path\" (but not \"$recursive\",\n    but could easily do so).\n\nI.e. my understanding is that Junio ejected Atharva's because this\nseemed like the smaller change, but perhaps it wasn't realized that we'd\nbe adding shellscript only to remove it again?\n\nBut in any case, even if we're patching git-submodule.sh not having\nAtharva's go first in its entirety doesn't answer why we can't extract\nthe bits he/you came up with in C for this series.\n\nI.e. that git-submodule.sh wouldn't need that shelling out (since it\njust called a helper, that helper could just return this data too),\nthat's just a few lines above the hunk in this series.\n\nI.e. if some remaining performance issue I couldn't reproduce in [1] is\ndue to the shellscript version of this v.s. calling a C function in\nlibgit isn't it better to focus on closing that gap than having the\ncaching mechanism?\n\n(Per the above & linked mails I'm still not 100% sure this even *is* a\ncaching mechanism, or if we store primary data in it, but I'm just\ntrying to go by your commit message descriptions).\n\n1. https://lore.kernel.org/git/211109.86v912dtfw.gmgdl@evledraar.gmail.com/\n2. https://lore.kernel.org/git/87r1cnfkqx.fsf@evledraar.gmail.com/\n3. https://lore.kernel.org/git/211105.86wnlngebr.gmgdl@evledraar.gmail.com/\n\n>> \n>> 1. https://lore.kernel.org/git/211105.861r3vhtot.gmgdl@evledraar.gmail.com/\n>> 2. https://lore.kernel.org/git/20211013051805.45662-8-raykar.ath@gmail.com/\n>> 3. https://lore.kernel.org/git/20211013051805.45662-1-raykar.ath@gmail.com/\n>> 4. https://lore.kernel.org/git/YWiXL+plA7GHfuVv@google.com/\n\n"},{"id":"440742","messageId":"82efc900-8518-3bb3-2524-8997475f1a45@gmail.com","threadId":"56712","inReplyTo":"YYmuqEQUaB1a8Gs1@google.com","subject":"Re: [PATCH v5 0/4] cache parent project's gitdir in submodules","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-09T15:58:57Z","receivedAt":"2021-11-09T15:59:02Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/8/2021 6:11 PM, Emily Shaffer wrote:\n\n> Yeah, I think we may be overthinking it, especially with the concerns\n> about common dir vs. gitdir. More specifically - I think we accidentally\n> did the right thing in the previous iteration by using the gitdir :)\n> \n> I think we can probably put it pretty simply:\n> submodule.superprojectGitDir should point from the most local gitdir of\n> the submodule to the most local gitdir of the superproject.\n> \n> Luckily there are not so many permutations to worry about here.\n> \n> Super doesn't have worktrees, sub doesn't have worktrees:\n\n> Super doesn't have worktrees, sub does have worktrees (and as you\n> suggest above, right now this would have to be created carefully and\n> manually, but later we probably want this to Just Work):\n\n> Super has worktrees, sub doesn't have worktrees:\n> Actually, I think in the future this might not be possible, if we want\n> to make `git worktree add --recurse-submodules` work gracefully (and I\n> do want that). But in the interim, in practice it looks like this:\n\n> Both super and sub have worktrees:\n> And this won't exist until we have graceful support of `git worktree add\n> --recurse-submodules` or with some manual effort, now.\n\n> I think this will give us access to both the worktree gitdir *and* the\n> common gitdir:\n> \n>   ~/git/.git/worktrees/git-second [GIT_DIR!]$ git rev-parse --git-common-dir\n>   /usr/local/google/home/emilyshaffer/git/.git\n> \n> So that means from any submodule, we can determine:\n>  - submodule's gitdir (from the .git link in the submodule wt)\n>  - submodule's common dir (from existing commands)\n>  - gitdir of superproject which submodule inhabits (from the config in\n>    the submodule's gitdir, or the submodule's config.worktree)\n>  - common dir of superproject (from existing commands + prior config)\n> \n> The upshot to me, then, means that we should be 1) making sure to get\n> the path to the gitdir, not the common dir, of the superproject; and 2)\n> using helpers to write to the worktree config, not to the local config,\n> of the submodule. In other words, we want to avoid the following:\n> \n> .git/\n>   modules/\n>     sub/\n>       worktrees/\n>         wt/\n> \t  config\n>       config <- \"submodule.superprojectGitDir = ../../../..\" as written by the worktree\n> \n> Will take a look at the rest of the comments too, but this sounds like a\n> reasonable approach to me.\n\nI agree, that this seems reasonable. Spelling it out carefully like this,\nalong with your list of possibilities, clarifies where the data is located\nand how we can construct any information we need from that.\n\nYou point out that there are cases that can be a bit tricky to get into\nwith current features, but this config approach won't make that any worse\nright now.\n\nThanks,\n-Stolee\n"},{"id":"440765","messageId":"YYrb4oCaIXIwN/bF@google.com","threadId":"56712","inReplyTo":"211109.86v912dtfw.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-09T20:36:50Z","receivedAt":"2021-11-09T20:37:00Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Tue, Nov 09, 2021 at 01:42:03AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> \n> On Mon, Nov 08 2021, Emily Shaffer wrote:\n> \n> > On Fri, Nov 05, 2021 at 09:43:56AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >> \n> >> \n> >> On Thu, Nov 04 2021, Junio C Hamano wrote:\n> >> \n> >> > Emily Shaffer <emilyshaffer@google.com> writes:\n> >> >\n> >> >> A recorded hint path to the superproject's gitdir might be added during\n> >> >> 'git submodule add', but in some cases - like submodules which were\n> >> >> created before 'git submodule add' learned to record that info - it might\n> >> >> be useful to update the hint. Let's do it during 'git submodule\n> >> >> update', when we already have a handle to the superproject while calling\n> >> >> operations on the submodules.\n> >> >\n> >> > We are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\n> >> > invalidate it, or if we have such a record, do we blindly trust it\n> >> > and use it without verifying if it is still fresh?\n> >> >\n> >> > Also, this step and the previous step both say we record gitdir on\n> >> > their title, but we instead record common dir.  Whichever is the\n> >> > right choice to record, let's be consistent.\n> >> \n> >> I had similar (AFAICT still unaddressed) feedback on the v2[1]. I'd lost\n> >> track of this series, and see one reason is that the In-Reply-Chain was\n> >> broken between v3..v4.\n> >> \n> >> I.e. it seems to me that this whole thing started as a way to avoid\n> >> shellscript overhead by calling git-rev-parse from git-submodule.sh, but\n> >> now that the relevant bits are moved to C we could just call some\n> >> slightly adjusted code in setup.c.\n> >\n> > No, that is not the case. It is the case that `git -C .. rev-parse\n> > --git-dir` is *very* expensive in the case where `../` is not, in fact,\n> > a gitdir; when I attempted another series which relied on finding the\n> > parent superproject's gitdir in this way, our testsuite took something\n> > like 5x longer to run than before. In other words, the expensive part is\n> > not the shelling out overhead - the expensive part is searching up the\n> > entire filesystem directory structure in the worst-case (\"we are not a\n> > submodule\") scenario. This is still needed, even with 'git-submodule.sh'\n> > moving to C.\n> \n> Do you have that test code somewhere?\n\nI messed around with it a little more, rebasing the no-caches-involved\nolder implementation and using an in-process lookup with\nsetup_git_directory_gently_1.\nhttps://github.com/nasamuffin/git/tree/config-inheritance-no-cache\n\nThe recent experiments are in the tip commit, and the original series is\nin the two commits prior if you're interested.\n\nThe upshot, though, is that I think there is still not a way around a\nsecond subprocess. Before, we determined the superproject's gitdir like\nso:\n\n  # Does a git project at .. think I belong to it?\n  git -C .. ls-files <args> -- path/to/submodule\n  # Where does that git project's gitdir live?\n  git -C .. rev-parse --absolute-git-dir\n\nEven if we can do the second call in-process, we still will be\nperforming this ls-files call to ensure that the parent repo is actually\nour superproject. (One good example of a time when the parent repo is\n*not*: the entire Git test suite, where '/path/to/git/t/trash directory.t1234-abcd'\nis not a submodule of '/path/to/git/.git'.)\n\nWe could reverse the checks, which will make this much less painful in\nthe real world, but will still slow down our test suites (and hopefully\nyou'll forgive me for combining C and bash so brazenly, but it's for\nillustration purposes only):\n\n  # Is there a git project above us?\n  setup_git_directory_gently_1(\"..\", out, 0);\n  # Does it think we're its submodule?\n  git -C $out rev-parse --absolute-git-dir\n\nThat will still result in an extra out-of-process call for every line in\nthe Git test suite, though, because of the trash directory layout.\n\nI looked briefly at `git ls-files` // `cmd_ls_files()` and it's fairly\nclose to being callable on an arbitrary 'struct repository', but not\nquite there. But I am pretty afraid of the rabbit hole ;)\n\n\nAnd anyway, even with those possible changes, it turns out that\n`setup_git_directory_gently_1` - which I had to munge a bit to make\nnon-private, anyway - wants to take shortcuts like looking at\ngetenv(GIT_DIR_ENVIRONMENT), which means it's will notice the\nsubmodule's envvar before it will notice the path passed to it. I\nactually am a little surprised that your experiment worked at all,\nbecause of this wrinkle, but your printf lines show that it did somehow.\n\n\nTaken as a whole, I'm not quite certain which is worse.\n\nApproach \"cache a pointer from the submodule\":\n- all the normal caching gotchas - creation, invalidation, staleness,\n  etc\n+ very easy lookup without need for significant refactoring\n- since it's a config, if we do it wrong we're stuck supporting it\n  forever anyway\n\nApproach \"do lots of in-process heuristics\":\n- need to refactor code areas unrelated to submodules or configs, like\n  setup.c\n- performance might differ based on filesystem speed\n+ still pretty fast (compared to subprocess calls)\n+ avoid all cache correctness issues\n- still kind of based on heuristics; will someone envision a wonky way\n  of organizing a submodule that breaks the heuristic?\n\nI'll think on it more....\n\n - Emily\n\n> I tried to reproduce this &\n> can't. I run my tests in /home/avar/*, and just created this:\n> \n>     $ find /tmp/some/ -name '.git' -type d\n>     /tmp/some/dir/.git\n>     /tmp/some/dir/a/b/c/d/e/f/g/i/j/k/.git\n> \n> I.e. a deeply nested structure in /tmp, if you ask for the git-dir in\n> /tmp/some/**/k you'll need to search several levels up.\n> \n> Then with the patch below we'll instrument almost all git commands to\n> optionally do that search, i.e. anything that does setup_git_directory()\n> at all:\n> \n>     $ GIT_TEST_SETUP=true GIT_TEST_SETUP_PRINT=true ~/g/git/git rev-parse HEAD\n>     warning: from '/tmp/some/dir/a/b/c/d/e/f/g/i/j' found '/tmp/some/dir' ('/tmp/some/dir/.git')\n>     <some hash here>\n> \n> And as a quick test to run a few tests I tried:\n> \n>     rm -rf test-results/; GIT_TEST_SETUP=true GIT_TEST_SETUP_PRINT=true prove -j8 t741*submod*.sh :: -V\n> \n> Which runs quickly enough for a tight test loop, and does that work >600 times:\n>     \n>     $ cat test-results/*.out|grep -c warning.*from.*found\n>     662\n> \n> I can't get that to show me any meaningful difference, just to pick on\n> one test (since it was easier to run repeatedly):\n>     \n>     $ hyperfine -L v true,false \"GIT_TEST_SETUP={v} ./t7416-submodule-dash-url.sh --root=/dev/shm/git\"\n>     Benchmark #1: GIT_TEST_SETUP=true ./t7416-submodule-dash-url.sh --root=/dev/shm/git\n>       Time (mean ± σ):     527.5 ms ±   7.2 ms    [User: 431.6 ms, System: 125.9 ms]\n>       Range (min … max):   522.6 ms … 542.5 ms    10 runs\n>      \n>     Benchmark #2: GIT_TEST_SETUP=false ./t7416-submodule-dash-url.sh --root=/dev/shm/git\n>       Time (mean ± σ):     526.7 ms ±  10.8 ms    [User: 421.1 ms, System: 131.6 ms]\n>       Range (min … max):   518.2 ms … 546.8 ms    10 runs\n>      \n>     Summary\n>       'GIT_TEST_SETUP=false ./t7416-submodule-dash-url.sh --root=/dev/shm/git' ran\n>         1.00 ± 0.02 times faster than 'GIT_TEST_SETUP=true ./t7416-submodule-dash-url.sh --root=/dev/shm/git'\n> \n> I.e. it's all fuzzy and within the error margins.\n> \n> Now if we do e.g.:\n> \n>     GIT_TEST_SETUP=false strace -f -c -U calls,name,time -S calls ./t7416-submodule-dash-url.sh 2>&1 >/dev/null | grep -A9000 calls >a\n>     GIT_TEST_SETUP=true strace -f -c -U calls,name,time -S calls ./t7416-submodule-dash-url.sh 2>&1 >/dev/null | grep -A9000 calls >b\n> \n> We'll see the syscall difference, in summary:\n> \n>     -   110086 total            100.00\n>     +   114765 total            100.00\n> \n> And some of the real big differences are:\n>     \n>     $ diff -u <(head -n 12 a) <(head -n 12 b)\n>     --- /dev/fd/63  2021-11-09 01:57:16.023991556 +0100\n>     +++ /dev/fd/62  2021-11-09 01:57:16.019991593 +0100\n>     @@ -1,12 +1,12 @@\n>          calls syscall          % time\n>      --------- ---------------- ------\n>     -    11504 openat             2.15\n>     -    11496 close              2.07\n>     -    10672 read               4.15\n>     -    10465 rt_sigaction       0.32\n>     -     9913 lstat              1.50\n>     -     9456 mmap               0.67\n>     -     7545 stat               0.81\n>     -     6349 fstat              0.62\n>     -     3896 access             0.61\n>     -     3490 mprotect           0.26\n>     +    11783 lstat              1.80\n>     +    11600 close              1.89\n>     +    11599 openat             2.19\n>     +    10887 read               4.36\n>     +    10465 rt_sigaction       0.33\n>     +     9742 stat               1.07\n>     +     9455 mmap               0.75\n>     +     6346 fstat              0.57\n>     +     4113 access             0.65\n>     +     3490 mprotect           0.28\n> \n> But syscalls are fast, so it doesn't show up in real results.\n> \n> Now, of course a real implementation could be less stupid, e.g. even if\n> we think we need *a cache* if these are the performance numbers why do\n> we need to risk the cache being incorrect v.s. say just writing \"I am a\n> submodule\" somewhere in the config (if we don't have that).\n> \n> Then we'll only do that work for submodules, so not all git invocations\n> will pay the cost (and it this point we'll usually have read config\n> anyway).\n> \n> But I really just don't think it's that expensive at all. I can see how\n> it would be for actually shelling out, but we don't need to do that.\n> \n> This could also just be that I'm running this on a really fast FS, which\n> is true. So I went and tested on an AIX machine I have access to.\n> \n> I/O on AIX is slow, *really slow*, so slow that if you \"rm -rfv\"\n> something you'll have time to read individual lines scrolling by.\n> \n> That ~500ms t7416-submodule-dash-url.sh test runs in around 50s on that\n> AIX box (power-aix.osuosl.org), most of which is I/O overhead, I created\n> the same /tmp/ directory structure and tried with\n> GIT_TEST_SETUP=[false|true] and it's ~55s with/without the env variable,\n> with no clear winner.\n> \n> I don't have access to hyperfine on that box, or the patience to wait\n> for AIX I/O to wait for meaningful results, but to a first approximation\n> that seems to indicate that it doesn't really matter there either.\n> \n> diff --git a/setup.c b/setup.c\n> index 347d7181ae9..8453d397676 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -1209,6 +1209,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>         struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n>         const char *prefix = NULL;\n>         struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n> +       const char *str = \"/tmp/some/dir/a/b/c/d/e/f/g/i/j\";\n> +       struct strbuf a = STRBUF_INIT, b = STRBUF_INIT;\n>  \n>         /*\n>          * We may have read an incomplete configuration before\n> @@ -1231,6 +1233,14 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>                 die_errno(_(\"Unable to read current working directory\"));\n>         strbuf_addbuf(&dir, &cwd);\n>  \n> +       if (git_env_bool(\"GIT_TEST_SETUP\", 0)) {\n> +               strbuf_addstr(&a, str);\n> +               setup_git_directory_gently_1(&a, &b, 0);\n> +\n> +               if (strcmp(a.buf, str) && git_env_bool(\"GIT_TEST_SETUP_PRINT\", 0))\n> +                       warning(\"from '%s' found '%s' ('%s/%s')\", str, a.buf, a.buf, b.buf);\n> +       }\n> +\n>         switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {\n>         case GIT_DIR_EXPLICIT:\n>                 prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n> \n"},{"id":"440770","messageId":"YYrsNagtHssb7jBF@google.com","threadId":"56712","inReplyTo":"YYrb4oCaIXIwN/bF@google.com","subject":"Re: [PATCH v5 4/4] submodule: record superproject gitdir during 'update'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-11-09T21:46:29Z","receivedAt":"2021-11-09T21:46:37Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Tue, Nov 09, 2021 at 12:36:50PM -0800, Emily Shaffer wrote:\n> \n> On Tue, Nov 09, 2021 at 01:42:03AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > \n> > \n> > On Mon, Nov 08 2021, Emily Shaffer wrote:\n> > \n> > > On Fri, Nov 05, 2021 at 09:43:56AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > >> \n> > >> \n> > >> On Thu, Nov 04 2021, Junio C Hamano wrote:\n> > >> \n> > >> > Emily Shaffer <emilyshaffer@google.com> writes:\n> > >> >\n> > >> >> A recorded hint path to the superproject's gitdir might be added during\n> > >> >> 'git submodule add', but in some cases - like submodules which were\n> > >> >> created before 'git submodule add' learned to record that info - it might\n> > >> >> be useful to update the hint. Let's do it during 'git submodule\n> > >> >> update', when we already have a handle to the superproject while calling\n> > >> >> operations on the submodules.\n> > >> >\n> > >> > We are hearing repeated mention of \"cache\" and \"hint\".  Do we ever\n> > >> > invalidate it, or if we have such a record, do we blindly trust it\n> > >> > and use it without verifying if it is still fresh?\n> > >> >\n> > >> > Also, this step and the previous step both say we record gitdir on\n> > >> > their title, but we instead record common dir.  Whichever is the\n> > >> > right choice to record, let's be consistent.\n> > >> \n> > >> I had similar (AFAICT still unaddressed) feedback on the v2[1]. I'd lost\n> > >> track of this series, and see one reason is that the In-Reply-Chain was\n> > >> broken between v3..v4.\n> > >> \n> > >> I.e. it seems to me that this whole thing started as a way to avoid\n> > >> shellscript overhead by calling git-rev-parse from git-submodule.sh, but\n> > >> now that the relevant bits are moved to C we could just call some\n> > >> slightly adjusted code in setup.c.\n> > >\n> > > No, that is not the case. It is the case that `git -C .. rev-parse\n> > > --git-dir` is *very* expensive in the case where `../` is not, in fact,\n> > > a gitdir; when I attempted another series which relied on finding the\n> > > parent superproject's gitdir in this way, our testsuite took something\n> > > like 5x longer to run than before. In other words, the expensive part is\n> > > not the shelling out overhead - the expensive part is searching up the\n> > > entire filesystem directory structure in the worst-case (\"we are not a\n> > > submodule\") scenario. This is still needed, even with 'git-submodule.sh'\n> > > moving to C.\n> > \n> > Do you have that test code somewhere?\n> \n> I messed around with it a little more, rebasing the no-caches-involved\n> older implementation and using an in-process lookup with\n> setup_git_directory_gently_1.\n> https://github.com/nasamuffin/git/tree/config-inheritance-no-cache\n> \n> The recent experiments are in the tip commit, and the original series is\n> in the two commits prior if you're interested.\n> \n> The upshot, though, is that I think there is still not a way around a\n> second subprocess. Before, we determined the superproject's gitdir like\n> so:\n> \n>   # Does a git project at .. think I belong to it?\n>   git -C .. ls-files <args> -- path/to/submodule\n>   # Where does that git project's gitdir live?\n>   git -C .. rev-parse --absolute-git-dir\n> \n> Even if we can do the second call in-process, we still will be\n> performing this ls-files call to ensure that the parent repo is actually\n> our superproject. (One good example of a time when the parent repo is\n> *not*: the entire Git test suite, where '/path/to/git/t/trash directory.t1234-abcd'\n> is not a submodule of '/path/to/git/.git'.)\n> \n> We could reverse the checks, which will make this much less painful in\n> the real world, but will still slow down our test suites (and hopefully\n> you'll forgive me for combining C and bash so brazenly, but it's for\n> illustration purposes only):\n> \n>   # Is there a git project above us?\n>   setup_git_directory_gently_1(\"..\", out, 0);\n>   # Does it think we're its submodule?\n>   git -C $out rev-parse --absolute-git-dir\n> \n> That will still result in an extra out-of-process call for every line in\n> the Git test suite, though, because of the trash directory layout.\n> \n> I looked briefly at `git ls-files` // `cmd_ls_files()` and it's fairly\n> close to being callable on an arbitrary 'struct repository', but not\n> quite there. But I am pretty afraid of the rabbit hole ;)\n\nJonathan Nieder and Glen Choo pointed out that I can read in the index\nto an arbitrary struct index_state from a path, and then call\n'index_dir_exists()', so this part of it is not as scary as I thought.\n\nI'll mess around today and see if I can come up with an in-process\nversion of 'get_superproject_working_tree()' and\n'get_superproject_gitdir()'.\n\nThanks for the lead - a fine example of how useful it is to receive\nhigh-level input from someone removed from the problem ;)\n\n - Emily\n"}]}