All of lore.kernel.org
 help / color / mirror / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Matheus Tavares Bernardino <matheus.bernardino@usp.br>,
	git <git@vger.kernel.org>
Subject: Re: [RFC PATCH 2/2] config: add 'config.superproject' file
Date: Tue, 13 Apr 2021 12:45:19 -0700	[thread overview]
Message-ID: <YHX0zwaES/9Nz97U@google.com> (raw)
In-Reply-To: <xmqqk0pbm6qt.fsf@gitster.g>

On Fri, Apr 09, 2021 at 03:29:46PM -0700, Junio C Hamano wrote:
> 
> Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes:
> 
> > Hi, Emily
> >
> > I'm not familiar enough with this code to give a full review and I
> > imagine you probably want comments more focused on the design level,
> > while this is an RFC, but here are some small nitpicks I found while
> > reading the patch. I Hope it helps :)
> >
> > On Thu, Apr 8, 2021 at 8:39 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> >>
> >> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> >> index 4b4cc5c5e8..a33136fb08 100644
> >> --- a/Documentation/git-config.txt
> >> +++ b/Documentation/git-config.txt
> >> @@ -48,7 +48,7 @@ unset an existing `--type` specifier with `--no-type`.
> >>
> >>  When reading, the values are read from the system, global and
> >>  repository local configuration files by default, and options
> >> -`--system`, `--global`, `--local`, `--worktree` and
> >> +`--system`, `--global`, `--superproject`, `--local`, `--worktree` and
> >>  `--file <filename>` can be used to tell the command to read from only
> >>  that location (see <<FILES>>).
> >>
> >> @@ -127,6 +127,17 @@ rather than from all available files.
> >>  +
> >>  See also <<FILES>>.
> >>
> >> +--superproject::
> >> +       For writing options: write to the superproject's
> >> +       `.git/config.superproject` file, even if run from a submodule of that
> >> +       superproject.
> >
> > Hmm, I wonder what happens if a repo is both a submodule and a
> > superproject (i.e. in case of nested submodules).
> 
> Another thing I am not sure about the design is that a repository
> can be shared as a submodule by more than one superprojects.  The
> superprojects may want their submodule checkouts at different
> submodule commits, but that is something doable by having multiple
> worktrees connected to a single submodule repository.

I think the implementation as-written actually handles this
sharing-via-worktree case you describe gracefully, as it discovers the
gitdir belonging to the worktree above the worktree where it is being
run now:

superproject-a
-> sub-a <gitdir at superproject-a/.git/modules/sub-a/>
superproject-b
-> sub-b <gitdir at superproject-a/.git/modules/sub-a/worktrees/sub-b/>

In this case running `git config --list --superproject` in sub-a will
yield superproject-a/.git/config.superproject and running it in sub-b
will yield superproject-b/.git/config.superproject; that seems logical
to me. If I am adding libc as a submodule via worktree to Git as well
as, say, Wireshark, just to save me on disk space, I wouldn't want my
Wireshark hooks to run in Git project or vice versa.

> I think our design principle has been that it is perfectly OK for a
> superproject to be in total control if its submodules, but
> submodules should not even be aware of being used as a submodule by
> a superproject, and that allows a submodule repository to be shared
> by multiple superprojects.  As "write to the superproject's X file"
> requires a submodule to know who THE superproject of itself is, this
> feature itself (not the implementation) feels somewhat iffy.

As for this, I wonder what the reasoning is. I guess to simplify the
code, and to make the behavior more predictable (for example, 'git
commit' doesn't suddenly make a commit in some project that isn't this
one)?

One could imagine some really nice quality-of-life improvements if the
submodule is allowed to know it's a submodule (even by a config, for
example):

 - We could teach 'git status' to indicate the state when the submodule
   index is clean, but the superproject does not contain a commit
   pointing to the submodule's HEAD - which could still be considered a
   dirty state, since the change isn't associated with the larger project
   yet. I could imagine there might be other handy information related
   to submodule/superproject status we may want to display too.
 - We could teach 'git log' to decorate commits which are referred to by
   superproject commits, perhaps?
 - We could teach 'git push' to, by option or config, push the entire
   superproject-and-submodules package at once, to make it easier to
   coordinate changes across the whole superproject
 - One could envision some other niceties like 'git stash
   --whole-superproject' or similar, where a user could operate on the
   entire overall project (that is, the superproject and all its
   submodules) without needing to switch context away

I don't think lacking these things would stop a user from doing
something they want to do, but it does seem like they could make life
more comfortable for a user developing in a project made up of many
submodules.

 - Emily

  reply	other threads:[~2021-04-13 19:45 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-08 23:39 [RFC PATCH 0/2] share a config between submodule and superproject Emily Shaffer
2021-04-08 23:39 ` [RFC PATCH 1/2] config: rename "submodule" scope to "gitmodules" Emily Shaffer
2021-04-08 23:39 ` [RFC PATCH 2/2] config: add 'config.superproject' file Emily Shaffer
2021-04-09 11:10   ` Philip Oakley
2021-04-13 18:05     ` Emily Shaffer
2021-04-09 14:35   ` Matheus Tavares Bernardino
2021-04-09 22:29     ` Junio C Hamano
2021-04-13 19:45       ` Emily Shaffer [this message]
2021-04-13 18:48     ` Emily Shaffer
2021-04-14 10:32 ` Future structure of submodules and .git/, .git/modules/* organization Ævar Arnfjörð Bjarmason
2021-04-15 21:25   ` Emily Shaffer
2021-04-15 21:41   ` Junio C Hamano
2021-04-23  0:15 ` [RFC PATCH v2 0/4] share a config between submodule and superproject Emily Shaffer
2021-04-23  0:15   ` [RFC PATCH v2 1/4] config: rename "submodule" scope to "gitmodules" Emily Shaffer
2021-04-23  9:46     ` Phillip Wood
2021-04-23  0:15   ` [RFC PATCH v2 2/4] t1510-repo-setup: don't use exact matching on traces Emily Shaffer
2021-04-23  9:59     ` Phillip Wood
2021-04-23  0:15   ` [RFC PATCH v2 3/4] t7006-pager.sh: more lenient trace checking Emily Shaffer
2021-04-23  9:54     ` Phillip Wood
2021-04-23 12:45       ` Phillip Wood
2021-04-23  0:15   ` [RFC PATCH v2 4/4] config: add 'config.superproject' file Emily Shaffer
2021-04-23 12:08     ` Johannes Schindelin
2021-06-19  0:31   ` [PATCH v3 0/2] share a config between submodule and superproject Emily Shaffer
2021-06-19  0:31     ` [PATCH v3 1/2] config: rename "submodule" scope to "gitmodules" Emily Shaffer
2021-06-19  0:31     ` [PATCH v3 2/2] config: add 'config.superproject' file Emily Shaffer

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=YHX0zwaES/9Nz97U@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=matheus.bernardino@usp.br \
    /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.