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

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> Here's a few examples:
>>
>> 1. Suppose I track my $HOME directory as a git repository.  Within my
>>    home directory, I have a src/git/ subdirectory with a clone of
>>    git.git, but I never intended to treat this as a submodule.
>>
>>    If I run "git rev-parse --show-superproject-working-tree", then it
>>    will discover my home directory repository, run ls-files in there
>>    to see if it has GITLINK entries, and either see one for src/git if
>>    I had "git add"ed it by mistake or not see one.  In either case,
>>    it would it would view my src/git/ directory as being a submodule
>>    of my home directory even though I hadn't intended it to be so.
>
> I am not sure about this one.  If you added an unrelated one with
> "git add" by mistake, you'd want to know about the mistake sooner
> rather than later, no?

My point with this example is that it's useful to have a definition of
what is a submodule repository, to make it unambiguous whether this
repository is a submodule or whether it's just a repository that
happens to have been cloned inside of a git-managed worktree.

For the specific example of having run "git add", I don't have any
very strong opinions.

[...]
>> 2. Suppose I have a copy of a repository such as
>>    https://gerrit.googlesource.com/gerrit/, with all its submodules.
>>    I am in the plugins/replication/ directory.
[...]
>>                         So for example, if I had run "git rm --cached
>>    plugins/replication" to _prepare to_ remove the plugins/replication
>>    submodule, then "git rev-parse --show-superproject-working-tree"
>>    will produce the wrong result.
>
> Yes, looking only at the index of the superproject will have that
> problem, but don't other things in the superproject point at the
> submodule, too, e.g. submodule.<name>.* configuration variables?

What all of those suggested alternatives have in common is that they
are pointers from another repository to the submodule.

This would be the first time in git history that we are saying a
property of a repository depends on having to examine files outside of
it.  I guess the main question I'd have is, why _wouldn't_ I want a
submodule to be able to point to the superproject containing it?  I
can think of many advantages to having that linkage, and the main
disadvantage I can think of is that it is a change.

I don't think that submodule.<name>.* is an adequate substitute for
having this setting, because it requires
- finding the superproject
- mapping the <name> to a path, using .gitmodules
- comparing the path to the submodule location

which would be complex, slow, and error-prone.

The one thing that I think could approach being an adequate substitute
is examining the path to the current repository and stripping off path
components until we find modules/; then the parent is the containing
superproject.  That would only work for absorbed submodules, though,
and it would be less explicit than having a config item.

> And then, after removing them to truly dissociate the submodule from
> the superproject, "git rev-parse --show-superproject-working-tree"
> may stop saying that it is a submodule, but this series wants to
> make it irrelevant what the command says.  Until you unset the
> configuration variable in the submodule, it will stay to be a
> submodule of the superproject, but the superproject no longer thinks
> it is responsible for the submodule.  You'll have to deal with an
> inconsistent state during the transition either way, so I am not
> sure it is the best solution to introduce an extra setting that can
> easily go out of sync.

This hints at a reason why one wouldn't want the linkage back ---
dealing with the ambiguity of inconsistencies (what if a submodule
declares a superproject but the superproject does not declare the
submodule?).

I would not expect that ambiguity to be much of a problem,
because the typical way to use superproject linkage would be to
print output from commands like "git status": for example,

	This is a submodule of ../../gerrit; you can run

		git -C ../../gerrit status

	to get the status of the superproject.

An inconsistency could occur due to the user using "mv" (instead of
"git mv") to move a submodule to a path a different number of path
components from its superproject.  One way to handle that would be to
make submodules record a boolean setting reflecting whether they are a
submodule, instead of the path to the superproject.  (This would be
similar to settings like core.bare.)  Alternatively, if the path to
the superproject is recorded and if "git fsck" is able to notice such
an inconsistency, then the user should be able to have an okay
experience repairing it.

[...]
>>    If "git status" runs "git rev-parse
>>    --show-superproject-working-tree", then git would walk up the
>>    filesystem above my mawk/ directory, looking for another .git dir.
>>    We can reach an NFS automounter directory and just hang.  Even
>>    without an NFS automounter, we'd expect this to take a while
>>    because, unlike normal repository discovery, we have no reason to
>>    believe that the walk is going to quickly discover a .git directory
>>    and terminate.  So this would violate user expectations.
>
> It would be a problem, but I do not know if "this is a submodule of
> that superproject" link is the only solution, let alone the most
> effective one.  It seems to me that you are looking more for
> something like GIT_CEILING_DIRECTORIES.

Who is the "you" addressed here?  The end user can use
GIT_CEILING_DIRECTORIES if they are expecting to run git commands
within an NFS automounter directory and outside of any git repository,
but they'd be right to be surprised if that suddenly became required
when inside git repositories.  I don't think we should assume that
running an extra .git discovery walk is cost-free to users who are not
using submodules and an acceptable burden to impose on them for the
sake of submodule users.

Thanks and hope that helps,
Jonathan

  reply	other threads:[~2022-02-08  1:23 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
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 [this message]
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=YgHE4iaV8QHRw64U@google.com \
    --to=jrnieder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=albertcui@google.com \
    --cc=avarab@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    --cc=jonathantanmy@google.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).