[PATCH 0/5] submodule: remove "--recursive-prefix"
- From
Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jun 27, 2022, 23:20 UTC
- Message-ID
- <pull.1282.git.git.1656372017.gitgitgadget@gmail.com>
= Description
This series is a refactor of "git submodule--helper update" that replaces "--recursive-prefix" with "--super-prefix" (see Background). This was initially motivated by:
* Junio's suggestion to simplify the code [1] (in response to a memory leak found by Johannes Schindelin [2]). * A desire to use the module_list API + get_submodule_displaypath() outside of builtin/submodule--helper.c (I expect to use this to check out branches in each submodule).
But it also happens to remove some overly complicated/duplicated code that was literally converted from shell :)
= Background
When recursing into nested submodules, Git commands keep track of the path from the superproject to the submodule in order to print a "display path" to the user, e.g.
Submodule path '../super/sub/nested-sub': checked out 'abcdef'
For historical reasons, "git submodule--helper update" uses "--recursive-prefix" for this purpose, but it should use "--super-prefix" instead because:
* That's what every other command uses (not just submodule--helper subcommands). * Using the "--super-prefix" helper functions makes the "git submodule--helper update" code simpler
= Patch organization
* Patch 1/5 makes sure that display paths are only computed using display path helper functions ([do_]get_submodule_displaypath()) and fixes some display paths that we never realized were broken. * Patches 2-3/5 simplify and deduplicate some display path computations. * Patch 4/5 replaces "--recursive-prefix" with "--super-prefix", making do_get_submodule_displaypath() obsolete. * Patch 5/5 removes do_get_submodule_displaypath().
= Interactions with other series
This would have been rebased onto ab/submodule-cleanup, but it's not yet clear to me if that series will be merged first. That series is almost done, but Ævar is still doing some digging on SUPPORT_SUPER_PREFIX [3] and may come back with findings that affect this series.
Fortunately, this series is only tangentially related to ab/submodule-cleanup, and the conflicts are quite simple:
| this | ab/submodule-cleanup | resolution | |-----------------------------+-------------------------------+---------------------------| | push --super-prefix arg | add new "ud" var | keep both | |-----------------------------+-------------------------------+---------------------------| | remove "--recursive-prefix" | add "--checkout", "--rebase", | keep both | | | "--merge" | | |-----------------------------+-------------------------------+---------------------------| | add SUPPORT_SUPER_PREFIX | remove SUPPORT_SUPER_PREFIX | keep ab/submodule-cleanup | | to "git submodule--helper | from "git submodule--helper" | | | update" | | |
= Future work
At the end of this series, get_submodule_displaypath() can be moved to submodule.h, which would make submodule.c:get_super_prefix_or_empty() obsolete. I have a patch that demonstrates this (CI run: [4]), but I decided to keep this series more focused on "git submodule--helper update" so that it's easier to review.
[1] https://lore.kernel.org/git/xmqq35g5xmmv.fsf@gitster.g [2] https://lore.kernel.org/git/877a45867ae368bf9e053caedcb6cf421e02344d.1655336146.git.gitgitgadget@gmail.com [3] https://lore.kernel.org/git/patch-v3-02.12-082e015781e-20220622T142012Z-avarab@gmail.com [4] https://github.com/chooglen/git/actions/runs/2572557584
Glen Choo (5): submodule--helper update: use display path helper submodule--helper: don't recreate recursive prefix submodule--helper: use correct display path helper submodule--helper update: use --super-prefix submodule--helper: remove display path helper
builtin/submodule--helper.c | 82 +++++++++---------------------------- t/t7406-submodule-update.sh | 59 ++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 62 deletions(-)
base-commit: 39c15e485575089eb77c769f6da02f98a55905e0 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1282%2Fchooglen%2Fsubmodule%2Fremove-recursive-prefix-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1282/chooglen/submodule/remove-recursive-prefix-v1 Pull-Request: https://github.com/git/git/pull/1282
-- gitgitgadget