git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: git@vger.kernel.org
Cc: "Emily Shaffer" <emilyshaffer@google.com>,
	"Albert Cui" <albertcui@google.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Matheus Tavares Bernardino" <matheus.bernardino@usp.br>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Jacob Keller" <jacob.keller@gmail.com>,
	"Atharva Raykar" <raykar.ath@gmail.com>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: [PATCH v6 0/5] teach submodules to know they're submodules
Date: Thu,  3 Feb 2022 13:59:10 -0800	[thread overview]
Message-ID: <20220203215914.683922-1-emilyshaffer@google.com> (raw)
In-Reply-To: <20211117005701.371808-1-emilyshaffer@google.com>

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

CI run: https://github.com/nasamuffin/git/actions/runs/1780282431

Since v6:

I've dropped the fifth commit to use this new config for `git rev-parse
--show-superproject-working-tree`. I think it did more harm than good -
that tool uses an odd way to determine whether the superproject is
actually the superproject, anyways.

I poked a little bit at trying to find some benchmark to demonstrate
that "submodule.superprojectGitDir" is actually faster - but I didn't
end up able to write one without writing a ton of new code to traverse
the filesystem. To be honest, I'm not all that interested in performance
- I want the config added for correctness, instead.

So, the only real changes between v6 and v7 are some documentation
changes suggested by Jonathan Tan
(https://lore.kernel.org/git/20211117234300.2598132-1-jonathantanmy%40google.com).

Since v5:

A couple things. Firstly, a semantics change *back* to the semantics of
v3 - we map from gitdir to gitdir, *not* from common dir to common dir,
so that theoretically a submodule with multiple worktrees in multiple
superproject worktrees will be able to figure out which worktree of the
superproject it's in. (Realistically, that's not really possible right
now, but I'd like to change that soon.)

Secondly, a rewording of comments and commit messages to indicate that
this isn't a cache of some expensive operation, but rather intended to
be the source of truth for all submodules. I also added a fifth commit
rewriting `git rev-parse --show-superproject-working-tree` to
demonstrate what that means in practice - but from a practical
standpoint, I'm a little worried about that fifth patch. More details in
the patch 5 description.

I did discuss Ævar's idea of relying on in-process filesystem digging to
find the superproject's gitdir with the rest of the Google team, but in
the end decided that there are some worries about filesystem digging in
this way (namely, some ugly interactions with network drives that are
actually already an issue for Googler Linux machines). Plus, the allure
of being able to definitively know that we're a submodule is pretty
strong. ;) But overall, this is the direction I'd prefer to keep going
in, rather than trying to guess from the filesystem going forward.

Since v4:

The only real change here is a slight semantics change to map from
<submodule gitdir> to <superproject common git dir>. In every case
*except* for when the superproject has a worktree, this changes nothing.
For the case when the superproject has a worktree, this means that now
submodules will refer to the general superproject common dir (e.g. no
worktree-specific refs or configs or whatnot).

I *think* that because a submodule should exist in the context of the
common dir, not the worktree gitdir, that is ok. However, it does mean
it would be difficult to do something like sharing a config specific to
the worktree (the initial goal of this series).

$ROOT/.git
$ROOT/.git/config.superproject <- shared by $ROOT/.git/modules/sub
$ROOT/.git/modules/sub <- points to $ROOT/.git
$ROOT/.git/worktrees/wt
$ROOT/.git/worktrees/wt/config.superproject <- contains a certain config-based pre-commit hook

If the submodule only knows about the common dir, that is tough, because
the submodule would basically have to guess which worktree it's in from
its own path. There would be no way for '$WT/sub' to inherit
'$ROOT/.git/worktrees/wt/config.superproject'.

That said... right now, we don't support submodules in worktrees very
well at all. A submodule in a worktree will get a brand new gitdir in
$ROOT/.git/worktrees/modules/ (and that brand new gitdir would point to
the super's common dir). So I think we can punt on this entire question
until we teach submodules and worktrees to play more gracefully together
(it's on my long list...), and at that time we can probably introduce a
pointer from $ROOT/.git/modules/sub/worktrees/wt/ to
$ROOT/.git/worktrees/wt/....

Or, to summarize the long ramble above: "this is still kind of weird
with worktrees, but let's fix it later when we fix worktrees more
thoroughly".

(More rambling about worktree weirdness here:
https://lore.kernel.org/git/YYRaII8YWVxlBqsF%40google.com )


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

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

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


Emily Shaffer (4):
  t7400-submodule-basic: modernize inspect() helper
  introduce submodule.superprojectGitDir record
  submodule: record superproject gitdir during absorbgitdirs
  submodule: record superproject gitdir during 'update'

 Documentation/config/submodule.txt |  9 ++++
 builtin/submodule--helper.c        | 11 ++++
 git-submodule.sh                   | 15 ++++++
 submodule.c                        | 23 +++++++++
 t/t7400-submodule-basic.sh         | 54 +++++++++++---------
 t/t7406-submodule-update.sh        | 27 ++++++++++
 t/t7412-submodule-absorbgitdirs.sh | 82 +++++++++++++++++++++++++++++-
 7 files changed, 194 insertions(+), 27 deletions(-)

Range-diff against v6:
1:  f1b08a7057 = 1:  251510c687 t7400-submodule-basic: modernize inspect() helper
2:  d46c8439ab ! 2:  1a85deb1c5 introduce submodule.superprojectGitDir record
    @@ Metadata
      ## Commit message ##
         introduce submodule.superprojectGitDir record
     
    -    Teach submodules a reference to their superproject's gitdir. This allows
    -    us to A) know that we're running from a submodule, and B) have a
    -    shortcut to the superproject's vitals, for example, configs.
    +    Teach submodules a config variable referencing to their superproject's
    +    gitdir. Git commands can rely on this reference to determine whether the
    +    current repo is a submodule to another repo. If this reference is
    +    absent, Git may assume the current repo is not a submodule.
    +
    +    In practice, some commands which specifically reference the submodule
    +    relationship, like 'git rev-parse --show-superproject-working-tree',
    +    will still search the parent directory. Other newly added or implicit
    +    behavior, such as "git status" showing the submodule's status in
    +    relation to the superproject, or a config which is shared between the
    +    superproject and the submodule, should not search the parent directory
    +    and instead rely on this "submodule.superprojectGitDir" config.
     
         By using a relative path instead of an absolute path, we can move the
         superproject directory around on the filesystem without breaking the
    @@ Commit message
         superproject's worktree gitdir (if it exists), we ensure that we can
         tell which worktree contains our submodule.
     
    -    Since this hint value is only introduced during new submodule creation
    -    via `git submodule add`, though, there is more work to do to allow the
    -    record to be created at other times.
    -
    -    Once this new config is reliably in place, we can use it to know
    -    definitively that we are working in a submodule, and to know which
    -    superproject we are a submodule of. This allows us to do some
    -    value-added behavior, like letting "git status" print additional info
    -    about the submodule's status in relation to its superproject, or like
    -    letting the superproject and submodule share an additional config file
    -    separate from either one's local config.
    +    This commit teaches "git submodule add" to add the aformentioned config
    +    variable. Subsequent commits will teach other commands to do so.
     
         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
         Helped-by: Junio C Hamano <gitster@pobox.com>
    @@ Documentation/config/submodule.txt: submodule.alternateErrorStrategy::
      	clone proceeds as if no alternate was specified.
     +
     +submodule.superprojectGitDir::
    -+	The relative path from the submodule's gitdir to its superproject's
    -+	gitdir. When Git is run in a repository, it usually makes no
    -+	difference whether this repository is standalone or a submodule, but if
    -+	this configuration variable is present, additional behavior may be
    -+	possible, such as "git status" printing additional information about
    -+	this submodule's status with respect to its superproject. This config
    -+	should only be present in projects which are submodules, but is not
    -+	guaranteed to be present in every submodule, so only optional
    -+	value-added behavior should be linked to it. It is set automatically
    -+	during submodule creation.
    ++	If this repository is a submodule, the relative path from this repo's
    ++	gitdir to its superproject's gitdir. Git commands may rely on this
    ++	reference to determine whether the current repo is a submodule to
    ++	another repo; if this reference is absent, Git will treat the current
    ++	repo as though it is not a submodule (this does not make a difference to
    ++	most Git commands). It is set automatically during submodule creation.
     
      ## builtin/submodule--helper.c ##
     @@ builtin/submodule--helper.c: static int clone_submodule(struct module_clone_data *clone_data)
3:  63ddaf5608 ! 3:  7a44b0edf9 submodule: record superproject gitdir during absorbgitdirs
    @@ Commit message
     
         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
     
    + ## Documentation/config/submodule.txt ##
    +@@ Documentation/config/submodule.txt: submodule.superprojectGitDir::
    + 	reference to determine whether the current repo is a submodule to
    + 	another repo; if this reference is absent, Git will treat the current
    + 	repo as though it is not a submodule (this does not make a difference to
    +-	most Git commands). It is set automatically during submodule creation.
    ++	most Git commands). It is set automatically during submodule creation
    ++	and 'git submodule absorbgitdir'.
    +
      ## submodule.c ##
     @@ submodule.c: static void relocate_single_git_dir_into_superproject(const char *path)
      	char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
4:  33a582ef13 ! 4:  63e736e69d submodule: record superproject gitdir during 'update'
    @@ Commit message
     
         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
     
    + ## Documentation/config/submodule.txt ##
    +@@ Documentation/config/submodule.txt: submodule.superprojectGitDir::
    + 	reference to determine whether the current repo is a submodule to
    + 	another repo; if this reference is absent, Git will treat the current
    + 	repo as though it is not a submodule (this does not make a difference to
    +-	most Git commands). It is set automatically during submodule creation
    +-	and 'git submodule absorbgitdir'.
    ++	most Git commands). It is set automatically during submodule creation,
    ++	update, and 'git submodule absorbgitdir'.
    +
      ## git-submodule.sh ##
     @@ git-submodule.sh: cmd_update()
      			;;
5:  a8b5d40a77 < -:  ---------- submodule: use config to find superproject worktree
-- 
2.35.0.263.gb82422642f-goog


  parent reply	other threads:[~2022-02-03 21:59 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17  0:56 [PATCH v6 0/5] teach submodules to know they're submodules Emily Shaffer
2021-11-17  0:56 ` [PATCH v6 1/5] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2021-11-17  0:56 ` [PATCH v6 2/5] introduce submodule.superprojectGitDir record Emily Shaffer
2021-11-17 23:43   ` Jonathan Tan
2021-11-17  0:56 ` [PATCH v6 3/5] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2021-11-17  0:57 ` [PATCH v6 4/5] submodule: record superproject gitdir during 'update' Emily Shaffer
2021-11-17  0:57 ` [PATCH v6 5/5] submodule: use config to find superproject worktree Emily Shaffer
2021-11-17 11:43 ` [RFC PATCH 0/2] submodule: test what happens if submodule.superprojectGitDir isn't around Ævar Arnfjörð Bjarmason
2021-11-17 11:43   ` [RFC PATCH 1/2] submodule tests: fix potentially broken "config .. --unset" Ævar Arnfjörð Bjarmason
2021-11-17 11:43   ` [RFC PATCH 2/2] submodule: add test mode for checking absence of "superProjectGitDir" Ævar Arnfjörð Bjarmason
2021-11-23 20:08   ` [RFC PATCH 0/2] submodule: test what happens if submodule.superprojectGitDir isn't around Emily Shaffer
2021-11-24  1:38     ` Ævar Arnfjörð Bjarmason
2021-11-17 23:28 ` [PATCH v6 0/5] teach submodules to know they're submodules Jonathan Tan
2021-11-23 20:28   ` Emily Shaffer
2022-02-03 21:59 ` Emily Shaffer [this message]
2022-02-03 21:59   ` [PATCH v7 1/4] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 2/4] introduce submodule.superprojectGitDir record Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 3/4] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 4/4] submodule: record superproject gitdir during 'update' Emily Shaffer
2022-02-03 22:39   ` [PATCH v6 0/5] teach submodules to know they're submodules Junio C Hamano
2022-02-04  1:15   ` Ævar Arnfjörð Bjarmason
2022-02-04 16:20     ` Junio C Hamano
2022-02-07 19:56     ` Jonathan Nieder
2022-02-07 23:21       ` Junio C Hamano
2022-02-08  1:18         ` Jonathan Nieder
2022-02-08 18:24           ` Junio C Hamano
2022-02-10 22:12             ` Emily Shaffer
2022-02-10 22:53               ` Jonathan Nieder
2022-02-12 20:35       ` Ævar Arnfjörð Bjarmason
2022-02-13  6:25         ` Junio C Hamano
2022-03-01  0:26   ` [PATCH v8 0/3] " Emily Shaffer
2022-03-01  0:26     ` [PATCH v8 1/3] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-03-01  0:26     ` [PATCH v8 2/3] introduce submodule.hasSuperproject record Emily Shaffer
2022-03-01  7:00       ` Junio C Hamano
2022-03-08 20:04         ` Emily Shaffer
2022-03-08 22:13       ` Glen Choo
2022-03-08 22:29         ` Glen Choo
2022-03-01  0:26     ` [PATCH v8 3/3] rev-parse: short-circuit superproject worktree when config unset Emily Shaffer
2022-03-01  7:06       ` Junio C Hamano
2022-03-09  0:38         ` Emily Shaffer
2022-03-01  3:08     ` [PATCH v8 0/3] teach submodules to know they're submodules Junio C Hamano
2022-03-08 18:54       ` Emily Shaffer
2022-03-10  0:44     ` [PATCH v9 " Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 1/3] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 2/3] introduce submodule.hasSuperproject record Emily Shaffer
2022-03-10  2:09         ` Junio C Hamano
2022-03-10 21:29           ` Glen Choo
2022-03-10 21:40           ` Glen Choo
2022-03-10 22:10             ` Junio C Hamano
2022-03-10 23:42               ` Glen Choo
2022-03-10 23:53                 ` Glen Choo
2022-03-15 20:48                   ` Emily Shaffer
2022-03-15 20:56                     ` Emily Shaffer
2022-03-15 21:19                       ` Glen Choo
2022-03-15 18:39               ` Emily Shaffer
2022-03-15 19:19                 ` Junio C Hamano
2022-03-10  2:32         ` Junio C Hamano
2022-03-10 21:54         ` Glen Choo
2022-03-15 18:27           ` Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 3/3] rev-parse: short-circuit superproject worktree when config unset Emily Shaffer
2022-03-10  1:47         ` Junio C Hamano
2022-03-10  4:39           ` Eric Sunshine
2022-03-11  9:09       ` [PATCH v9 0/3] teach submodules to know they're submodules Ævar Arnfjörð Bjarmason
2022-03-13  5:43         ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20220203215914.683922-1-emilyshaffer@google.com \
    --to=emilyshaffer@google.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=albertcui@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=matheus.bernardino@usp.br \
    --cc=phillip.wood123@gmail.com \
    --cc=raykar.ath@gmail.com \
    --cc=stolee@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).