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

[PATCH v2 00/10] Get rid of "git --super-prefix"

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 14, 2022, 10:08 UTC
Message-ID
<cover-v2-00.10-00000000000-20221114T100803Z-avarab@gmail.com>
In-Reply-To
<RFC-cover-0.8-00000000000-20221109T192315Z-avarab@gmail.com>
= Summary

What's "--super-prefix"? The "git" command takes an "internal use only" "--super-prefix" option, which is used to inform processes invoked in submodules what the path prefix to the invoking superproject is.

This is so so that e.g. "git submodule absorbgitdirs" can report "sub-1/sub-2", instead of "sub-2" when being invoked in the "sub-1" submodule.

For this the "--super-prefix" facility has been doing a setenv("GIT_INTERNAL_SUPER_PREFIX", "sub-1/") as soon as it got the "--supre-prefix=sub-1/". We'd then pass that along via the environment when invoking the sub-process.

As this series shows we don't need such a hands-off global facility to do this, we can instead just pass the relevant context directly in each command. E.g. "git submodule absorbgitdirs" can pass the path to the "git submodule absorbgitdirs" sub-process it's about to invoke.

= Relation to other submissions

This is a non-RFC version of my earlier RFC to get rid of "--super-prefix"[1], which itself was an alternate proposal to Glen's [2]. Per [3] he's agreed to go with this approach.

This is on top of "ab/submodule-helper-prep-only" (currently in "next"[4]) and "ab/submodule-no-abspath" (currently in "seen"[5]).

It's also proposing to replace Glen's one-patch[6], which is working around the problem shown in the test added in 1/10 here. Per downthread of [7] I think Glen was aiming for getting a more narrow fix in case we split off 9/10 here into some later fix.

As we're fixing an edge case in something that's always been broken (and thus wouldn't backport) I think it's better to just fix the problem directly, rather than introducing new "--super-prefix" use, just to take it away later.

= Changes since the RFC
* Added Glen's "git fetch" test as a 1/10, with an updated commit message
* Updated 2/10's commit message for the non-RFC, and adjusted and
  incorporated a variant of Glen's fsmonitor test change.
* 9/10: Correctly re-arrange bitfield define's when removing one, make
  the test added in 1/10 pass.
* 10/10: New commit to make "git fetch" use our own "--super-prefix"
  instead of its "--submodule-prefix", which makes it consistent with
  the other command-level "--super-prefix" introduced here.
  Personally I'm rather "meh" on this. It's not actually needed by the
  main body of the series, but Glen seems to prefer it in[9], and
  doing it is easy enough.
  That change is a pure refactoring clean-up for consistency. The only
  reason it's 10/10 and not 1/10 is because it uses the
  "OPT__SUPER_PREFIX()" introduced in 2/10.
= CI & fetch URL
Passing at: https://github.com/avar/git/tree/avar/nuke-global-super-prefix-use-local-2
1. https://lore.kernel.org/git/RFC-cover-0.8-00000000000-20221109T192315Z-avarab@gmail.com/
2. https://lore.kernel.org/git/20221109004708.97668-1-chooglen@google.com/
3. https://lore.kernel.org/git/kl6l5yfm2taf.fsf@chooglen-macbookpro.roam.corp.google.com/
4. c0c4f4d1c33 (Merge branch 'ab/submodule-helper-prep-only' into next, 2022-11-08)
5. https://lore.kernel.org/git/patch-1.1-34b54fdd9bb-20221109T020347Z-avarab@gmail.com/
6. https://lore.kernel.org/git/pull.1378.git.git.1668210935360.gitgitgadget@gmail.com/
7. https://lore.kernel.org/git/221111.86fsepmbhe.gmgdl@evledraar.gmail.com/
8. https://lore.kernel.org/git/20221109004708.97668-3-chooglen@google.com/
9. https://lore.kernel.org/git/kl6lsfip0yfx.fsf@chooglen-macbookpro.roam.corp.google.com/
Glen Choo (1):
  read-tree + fetch tests: test failing "--super-prefix" interaction
Ævar Arnfjörð Bjarmason (9):
  submodule--helper: don't use global --super-prefix in "absorbgitdirs"
  submodule--helper: "deinit" has never used "--super-prefix"
  submodule--helper: convert "foreach" to its own "--super-prefix"
  submodule--helper: convert "sync" to its own "--super-prefix"
  submodule--helper: convert "status" to its own "--super-prefix"
  submodule--helper: convert "{update,clone}" to their own
    "--super-prefix"
  submodule tests: test "git branch -t" output and stderr
  read-tree: add "--super-prefix" option, eliminate global
  fetch: rename "--submodule-prefix" to "--super-prefix"
 Documentation/fetch-options.txt |  5 --
 Documentation/git.txt           |  8 +--
 builtin.h                       |  4 --
 builtin/checkout.c              |  2 +-
 builtin/fetch.c                 |  7 +--
 builtin/read-tree.c             |  1 +
 builtin/submodule--helper.c     | 95 ++++++++++++++++++--------------
 cache.h                         |  2 -
 entry.c                         | 12 ++--
 entry.h                         |  6 +-
 environment.c                   | 13 -----
 git.c                           | 41 +++-----------
 parse-options.h                 |  4 ++
 submodule.c                     | 70 +++++++++++------------
 submodule.h                     | 12 ++--
 t/lib-submodule-update.sh       | 98 ++++++++++++++++++---------------
 t/t1001-read-tree-m-2way.sh     |  2 +-
 t/t5616-partial-clone.sh        | 43 +++++++++++++++
 t/t7527-builtin-fsmonitor.sh    | 33 +++--------
 unpack-trees.c                  | 23 ++++----
 unpack-trees.h                  |  1 +
 21 files changed, 244 insertions(+), 238 deletions(-)
Range-diff against v1:
 -:  ----------- >  1:  1114a4ff666 read-tree + fetch tests: test failing "--super-prefix" interaction
 1:  ad0356b596f !  2:  5a35f7b75b3 submodule--helper: don't use global --super-prefix in "absorbgitdirs"
    @@ Commit message
         declare "fsmonitor--daemon" as accepting it too, even though it
         doesn't care about it.
     
    -    There's a parallel proposal to remove "--super-prefix" as an option to
    -    "git" in [3], and some of the approach might be the easiest route in
    -    some cases.
    -
         But in the case of "absorbgitdirs" it only needed "--super-prefix" to
         invoke itself recursively, and we'd never have another "in-between"
         process in the chain. So we didn't need the bigger hammer of "git
    @@ Commit message
         stone makes such an eventual change easier, as we'll need to deal with
         less global state at that point.
     
    +    The "fsmonitor--daemon" test adjusted here was added in [3]. The
    +    comment added in that commit has been out-of-date from the beginning,
    +    and the "have_t2_error_event()" was being overly specific in testing
    +    for a bug that we *don't* have. Let's instead test for the stdout and
    +    stderr that we *do have*.
    +
         1. 74866d75793 (git: make super-prefix option, 2016-10-07)
         2. 53fcfbc84f6 (fsmonitor--daemon: allow --super-prefix argument,
            2022-05-26)
    -    3. https://lore.kernel.org/git/20221109004708.97668-1-chooglen@google.com/
    +    3. 53fcfbc84f6 (fsmonitor--daemon: allow --super-prefix argument,
    +       2022-05-26)
     
    +    Signed-off-by: Glen Choo <chooglen@google.com>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/submodule--helper.c ##
    @@ submodule.h: void submodule_unset_core_worktree(const struct submodule *sub);
      
      /*
       * Return the absolute path of the working tree of the superproject, which this
    +
    + ## t/t7527-builtin-fsmonitor.sh ##
    +@@ t/t7527-builtin-fsmonitor.sh: test_expect_success 'submodule always visited' '
    + # the submodule, and someone does a `git submodule absorbgitdirs`
    + # in the super, Git will recursively invoke `git submodule--helper`
    + # to do the work and this may try to read the index.  This will
    +-# try to start the daemon in the submodule *and* pass (either
    +-# directly or via inheritance) the `--super-prefix` arg to the
    +-# `git fsmonitor--daemon start` command inside the submodule.
    +-# This causes a warning because fsmonitor--daemon does take that
    +-# global arg (see the table in git.c)
    +-#
    +-# This causes a warning when trying to start the daemon that is
    +-# somewhat confusing.  It does not seem to hurt anything because
    +-# the fsmonitor code maps the query failure into a trivial response
    +-# and does the work anyway.
    +-#
    +-# It would be nice to silence the warning, however.
    +-
    +-have_t2_error_event () {
    +-	log=$1
    +-	msg="fsmonitor--daemon doesnQt support --super-prefix" &&
    +-
    +-	tr '\047' Q <$1 | grep -e "$msg"
    +-}
    ++# try to start the daemon in the submodule.
    + 
    + test_expect_success "stray submodule super-prefix warning" '
    + 	test_when_finished "rm -rf super; \
    +-			    rm -rf sub;   \
    +-			    rm super-sub.trace" &&
    ++			    rm -rf sub" &&
    + 
    + 	create_super super &&
    + 	create_sub sub &&
    +@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "stray submodule super-prefix warning" '
    + 
    + 	test_path_is_dir super/dir_1/dir_2/sub/.git &&
    + 
    +-	GIT_TRACE2_EVENT="$PWD/super-sub.trace" \
    +-		git -C super submodule absorbgitdirs &&
    +-
    +-	! have_t2_error_event super-sub.trace
    ++	cat >expect <<-\EOF &&
    ++	Migrating git directory of '\''dir_1/dir_2/sub'\'' from '\''dir_1/dir_2/sub/.git'\'' to '\''.git/modules/dir_1/dir_2/sub'\''
    ++	EOF
    ++	git -C super submodule absorbgitdirs >out 2>actual &&
    ++	test_cmp expect actual &&
    ++	test_must_be_empty out
    + '
    + 
    + # On a case-insensitive file system, confirm that the daemon
 2:  87a780eb9bf =  3:  a7a1f9487dc submodule--helper: "deinit" has never used "--super-prefix"
 3:  4858e2ad0ed =  4:  935d8070834 submodule--helper: convert "foreach" to its own "--super-prefix"
 4:  5ffe4407e46 =  5:  933c752513d submodule--helper: convert "sync" to its own "--super-prefix"
 5:  a46540b63c2 =  6:  67273f729e0 submodule--helper: convert "status" to its own "--super-prefix"
 6:  78ebf0e2abf =  7:  eaa73f5b1e4 submodule--helper: convert "{update,clone}" to their own "--super-prefix"
 7:  00a9e789be7 =  8:  172b5865811 submodule tests: test "git branch -t" output and stderr
 8:  3ba894a6698 !  9:  9fdeab60773 read-tree: add "--super-prefix" option, eliminate global
    @@ git.c
       */
      #define NEED_WORK_TREE		(1<<3)
     -#define SUPPORT_SUPER_PREFIX	(1<<4)
    - #define DELAY_PAGER_CONFIG	(1<<5)
    - #define NO_PARSEOPT		(1<<6) /* parse-options is not used */
    +-#define DELAY_PAGER_CONFIG	(1<<5)
    +-#define NO_PARSEOPT		(1<<6) /* parse-options is not used */
    ++#define DELAY_PAGER_CONFIG	(1<<4)
    ++#define NO_PARSEOPT		(1<<5) /* parse-options is not used */
      
    + struct cmd_struct {
    + 	const char *cmd;
     @@ git.c: const char git_usage_string[] =
      	   "           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
      	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]\n"
    @@ t/t1001-read-tree-m-2way.sh: test_expect_success 'read-tree supports the super-p
      '
      
     
    + ## t/t5616-partial-clone.sh ##
    +@@ t/t5616-partial-clone.sh: test_expect_success 'repack does not loosen promisor objects' '
    + 	grep "loosen_unused_packed_objects/loosened:0" trace
    + '
    + 
    +-test_expect_failure 'lazy-fetch in submodule succeeds' '
    ++test_expect_success 'lazy-fetch in submodule succeeds' '
    + 	# setup
    + 	test_config_global protocol.file.allow always &&
    + 
    +
      ## unpack-trees.c ##
     @@ unpack-trees.c: static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
      	  ? ((o)->msgs[(type)])      \
 -:  ----------- > 10:  100ba36dfb7 fetch: rename "--submodule-prefix" to "--super-prefix"
-- 
2.38.0.1471.ge4d8947e7aa
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 34 of 105 in “git: remove --super-prefix”
  1. 0/4 git: remove --super-prefixGlen Choo, Nov 9, 2022
  2. 1/4 submodule--helper: teach --toplevel-cwd-prefixGlen Choo, Nov 9, 2022
  3. Ævar Arnfjörð BjarmasonNov 9, 2022
  4. 2/4 fetch: refactor --submodule-prefixGlen Choo, Nov 9, 2022
  5. Ævar Arnfjörð BjarmasonNov 9, 2022
  6. 3/4 read-tree: teach --submodule-prefixGlen Choo, Nov 9, 2022
  7. Ævar Arnfjörð BjarmasonNov 9, 2022
  8. 4/4 git: remove --super-prefixGlen Choo, Nov 9, 2022
  9. 0/8 Get rid of "git --super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  10. 1/8 submodule--helper: don't use global --super-prefix in "absorbgitdirs"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  11. Glen ChooNov 11, 2022
  12. 2/8 submodule--helper: "deinit" has never used "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  13. 3/8 submodule--helper: convert "foreach" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  14. 4/8 submodule--helper: convert "sync" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  15. 5/8 submodule--helper: convert "status" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  16. 6/8 submodule--helper: convert "{update,clone}" to their own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 9, 2022
  17. 7/8 submodule tests: test "git branch -t" output and stderrÆvar Arnfjörð Bjarmason, Nov 9, 2022
  18. 8/8 read-tree: add "--super-prefix" option, eliminate globalÆvar Arnfjörð Bjarmason, Nov 9, 2022
  19. Glen ChooNov 11, 2022
  20. Taylor BlauNov 9, 2022
  21. Ævar Arnfjörð BjarmasonNov 9, 2022
  22. Taylor BlauNov 9, 2022
  23. Ævar Arnfjörð BjarmasonNov 9, 2022
  24. Glen ChooNov 10, 2022
  25. Ævar Arnfjörð BjarmasonNov 10, 2022
  26. Glen ChooNov 11, 2022
  27. Glen ChooNov 11, 2022
  28. Ævar Arnfjörð BjarmasonNov 11, 2022
  29. Taylor BlauNov 11, 2022
  30. Glen ChooNov 12, 2022
  31. Ævar Arnfjörð BjarmasonNov 14, 2022
  32. Glen ChooNov 14, 2022
  33. Ævar Arnfjörð BjarmasonNov 15, 2022
  34. 00/10 Get rid of "git --super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  35. 01/10 read-tree + fetch tests: test failing "--super-prefix" interactionÆvar Arnfjörð Bjarmason, Nov 14, 2022
  36. Glen ChooNov 14, 2022
  37. Ævar Arnfjörð BjarmasonNov 14, 2022
  38. Glen ChooNov 14, 2022
  39. 02/10 submodule--helper: don't use global --super-prefix in "absorbgitdirs"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  40. Glen ChooNov 14, 2022
  41. Ævar Arnfjörð BjarmasonNov 17, 2022
  42. 03/10 submodule--helper: "deinit" has never used "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  43. 04/10 submodule--helper: convert "foreach" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  44. Glen ChooNov 14, 2022
  45. Ævar Arnfjörð BjarmasonNov 17, 2022
  46. 05/10 submodule--helper: convert "sync" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  47. 06/10 submodule--helper: convert "status" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  48. 07/10 submodule--helper: convert "{update,clone}" to their own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  49. Glen ChooNov 14, 2022
  50. 08/10 submodule tests: test "git branch -t" output and stderrÆvar Arnfjörð Bjarmason, Nov 14, 2022
  51. Glen ChooNov 14, 2022
  52. 10/10 fetch: rename "--submodule-prefix" to "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 14, 2022
  53. Glen ChooNov 14, 2022
  54. 09/10 read-tree: add "--super-prefix" option, eliminate globalÆvar Arnfjörð Bjarmason, Nov 14, 2022
  55. Glen ChooNov 14, 2022
  56. Taylor BlauNov 14, 2022
  57. Glen ChooNov 14, 2022
  58. Glen ChooNov 14, 2022
  59. Ævar Arnfjörð BjarmasonNov 15, 2022
  60. Glen ChooNov 16, 2022
  61. Ævar Arnfjörð BjarmasonNov 17, 2022
  62. Glen ChooNov 21, 2022
  63. 0/9 Get rid of "git --super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  64. 1/9 read-tree + fetch tests: test failing "--super-prefix" interactionÆvar Arnfjörð Bjarmason, Nov 19, 2022
  65. 3/9 submodule--helper: don't use global --super-prefix in "absorbgitdirs"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  66. Glen ChooNov 22, 2022
  67. 2/9 submodule.c & submodule--helper: pass along "super_prefix" paramÆvar Arnfjörð Bjarmason, Nov 19, 2022
  68. 4/9 submodule--helper: convert "foreach" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  69. 6/9 submodule--helper: convert "status" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  70. 5/9 submodule--helper: convert "sync" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  71. 7/9 submodule--helper: convert "{update,clone}" to their own "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  72. 9/9 fetch: rename "--submodule-prefix" to "--super-prefix"Ævar Arnfjörð Bjarmason, Nov 19, 2022
  73. 8/9 read-tree: add "--super-prefix" option, eliminate globalÆvar Arnfjörð Bjarmason, Nov 19, 2022
  74. Glen ChooNov 22, 2022
  75. Glen ChooNov 22, 2022
  76. 0/9 Get rid of "git --super-prefix"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  77. 2/9 read-tree + fetch tests: test failing "--super-prefix" interactionÆvar Arnfjörð Bjarmason, Dec 15, 2022
  78. 1/9 submodule absorbgitdirs tests: add missing "Migrating git..." testsÆvar Arnfjörð Bjarmason, Dec 15, 2022
  79. Glen ChooDec 15, 2022
  80. Ævar Arnfjörð BjarmasonDec 20, 2022
  81. 3/9 submodule.c & submodule--helper: pass along "super_prefix" paramÆvar Arnfjörð Bjarmason, Dec 15, 2022
  82. 5/9 submodule--helper: convert "foreach" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  83. 4/9 submodule--helper: don't use global --super-prefix in "absorbgitdirs"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  84. Glen ChooDec 15, 2022
  85. 6/9 submodule--helper: convert "sync" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  86. 7/9 submodule--helper: convert "status" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  87. 8/9 submodule--helper: convert "{update,clone}" to their own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 15, 2022
  88. 9/9 read-tree: add "--super-prefix" option, eliminate globalÆvar Arnfjörð Bjarmason, Dec 15, 2022
  89. Glen ChooDec 15, 2022
  90. Junio C HamanoDec 15, 2022
  91. Junio C HamanoDec 15, 2022
  92. 0/9 Get rid of "git --super-prefix"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  93. 2/9 read-tree + fetch tests: test failing "--super-prefix" interactionÆvar Arnfjörð Bjarmason, Dec 20, 2022
  94. 1/9 submodule absorbgitdirs tests: add missing "Migrating git..." testsÆvar Arnfjörð Bjarmason, Dec 20, 2022
  95. 3/9 submodule.c & submodule--helper: pass along "super_prefix" paramÆvar Arnfjörð Bjarmason, Dec 20, 2022
  96. 5/9 submodule--helper: convert "foreach" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  97. 4/9 submodule--helper: don't use global --super-prefix in "absorbgitdirs"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  98. 6/9 submodule--helper: convert "sync" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  99. 7/9 submodule--helper: convert "status" to its own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  100. 8/9 submodule--helper: convert "{update,clone}" to their own "--super-prefix"Ævar Arnfjörð Bjarmason, Dec 20, 2022
  101. 9/9 read-tree: add "--super-prefix" option, eliminate globalÆvar Arnfjörð Bjarmason, Dec 20, 2022
  102. Taylor BlauNov 9, 2022
  103. Glen ChooNov 9, 2022
  104. Taylor BlauNov 10, 2022
  105. Glen ChooNov 10, 2022

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

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