All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Atharva Raykar" <raykar.ath@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Glen Choo" <chooglen@google.com>
Subject: [PATCH 0/5] submodule: remove "--recursive-prefix"
Date: Mon, 27 Jun 2022 23:20:12 +0000	[thread overview]
Message-ID: <pull.1282.git.git.1656372017.gitgitgadget@gmail.com> (raw)

= 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

             reply	other threads:[~2022-06-27 23:20 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27 23:20 Glen Choo via GitGitGadget [this message]
2022-06-27 23:20 ` [PATCH 1/5] submodule--helper update: use display path helper Glen Choo via GitGitGadget
2022-06-28  8:23   ` Ævar Arnfjörð Bjarmason
2022-06-28 17:34     ` Glen Choo
2022-06-27 23:20 ` [PATCH 2/5] submodule--helper: don't recreate recursive prefix Glen Choo via GitGitGadget
2022-06-27 23:20 ` [PATCH 3/5] submodule--helper: use correct display path helper Glen Choo via GitGitGadget
2022-06-27 23:20 ` [PATCH 4/5] submodule--helper update: use --super-prefix Glen Choo via GitGitGadget
2022-06-28  8:47   ` Ævar Arnfjörð Bjarmason
2022-06-28 18:15     ` Glen Choo
2022-06-27 23:20 ` [PATCH 5/5] submodule--helper: remove display path helper Glen Choo via GitGitGadget
2022-06-28 16:34 ` [PATCH 0/5] submodule: remove "--recursive-prefix" Glen Choo
2022-06-30 21:19 ` [PATCH v2 00/18] " Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 01/18] git-submodule.sh: remove unused sanitize_submodule_env() Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 02/18] git-submodule.sh: remove unused $prefix variable Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 03/18] git-submodule.sh: make the "$cached" variable a boolean Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 04/18] git-submodule.sh: remove unused top-level "--branch" argument Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 05/18] submodule--helper: have --require-init imply --init Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 06/18] submodule update: remove "-v" option Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 07/18] submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs" Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 08/18] submodule--helper: report "submodule" as our name in some "-h" output Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 09/18] submodule--helper: understand --checkout, --merge and --rebase synonyms Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 10/18] submodule--helper: eliminate internal "--update" option Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 11/18] git-submodule.sh: use "$quiet", not "$GIT_QUIET" Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 12/18] git-sh-setup.sh: remove "say" function, change last users Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 13/18] submodule--helper update: use display path helper Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 14/18] submodule--helper: don't recreate recursive prefix Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 15/18] submodule--helper: use correct display path helper Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 16/18] submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags Ævar Arnfjörð Bjarmason via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 17/18] submodule--helper update: use --super-prefix Glen Choo via GitGitGadget
2022-06-30 21:19   ` [PATCH v2 18/18] submodule--helper: remove display path helper Glen Choo via GitGitGadget
2022-06-30 21:57   ` [PATCH v2 00/18] submodule: remove "--recursive-prefix" Glen Choo
2022-06-30 22:11   ` [PATCH v3 0/6] " Glen Choo
2022-06-30 22:11     ` [PATCH v3 1/6] submodule--helper update: use display path helper Glen Choo
2022-06-30 22:11     ` [PATCH v3 2/6] submodule--helper: don't recreate recursive prefix Glen Choo
2022-06-30 22:11     ` [PATCH v3 3/6] submodule--helper: use correct display path helper Glen Choo
2022-06-30 22:11     ` [PATCH v3 4/6] submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags Glen Choo
2022-06-30 22:11     ` [PATCH v3 5/6] submodule--helper update: use --super-prefix Glen Choo
2022-06-30 22:11     ` [PATCH v3 6/6] submodule--helper: remove display path helper Glen Choo
2022-07-01  2:11     ` [PATCH v4 0/7] submodule: remove "--recursive-prefix" Glen Choo
2022-07-01  2:11       ` [PATCH v4 1/7] submodule--helper tests: add missing "display path" coverage Glen Choo
2022-07-01  2:11       ` [PATCH v4 2/7] submodule--helper update: use display path helper Glen Choo
2022-07-01  2:11       ` [PATCH v4 3/7] submodule--helper: don't recreate recursive prefix Glen Choo
2022-07-01  2:11       ` [PATCH v4 4/7] submodule--helper: use correct display path helper Glen Choo
2022-07-01  2:11       ` [PATCH v4 5/7] submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags Glen Choo
2022-07-01  2:11       ` [PATCH v4 6/7] submodule--helper update: use --super-prefix Glen Choo
2022-07-01  2:11       ` [PATCH v4 7/7] submodule--helper: remove display path helper Glen Choo
2022-06-30 22:16   ` [PATCH v2 00/18] submodule: remove "--recursive-prefix" Ævar Arnfjörð Bjarmason
2022-06-30 23:45     ` Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1282.git.git.1656372017.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=raykar.ath@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.