git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: Toolforger <toolforger@durchholz.org>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: url.<base>.insteadOf vs. submodules
Date: Tue, 21 Feb 2017 18:40:37 -0500	[thread overview]
Message-ID: <20170221234037.ga44u3birwd5whab@sigill.intra.peff.net> (raw)
In-Reply-To: <CAGZ79kby-UhUqci9Mgdhw+wvS5Y39=Q7AmCrWaTMWbcZPNT6Dw@mail.gmail.com>

On Tue, Feb 21, 2017 at 03:16:27PM -0800, Stefan Beller wrote:

> > I guess one answer is that this is the wrong approach entirely, and the
> > right one is something like: submodules should understand that they are
> > part of a superproject, and respect some whitelisted set of config from
> > the superproject .git/config file.
> 
> This would break one of the core assumptions that submodules
> are "independent" repos.

Yeah, that was the "first half" that I said was hard. :)

You could rationalize it under the fact that they _are_ independent
repos; we're just adding a new config source.  Arguably it could be a
feature for any repository embedded inside the working tree of another,
submodule or not, to consider the outer repository as a (limited) source
of config.

But there are probably a lot of irritating corner cases with the whole
concept unless we apply a strict whitelist of keys (e.g., you probably
don't want remote.* to be propagated). And as the recent
GIT_CONFIG_PARAMETERS whitelist showed, that approach ended up confusing
and annoying.

So maybe the whole thing is insane, and the right answer is that config
values should go into ~/.gitconfig. And we may need better tools there
for limiting that global config to certain parts of the tree (like Duy's
conditional include thing).

> Though I do not know if this is actually a good assumption.
> e.g. "[PATCH v2] git-prompt.sh: add submodule indicator"
> https://public-inbox.org/git/1486075892-20676-2-git-send-email-email@benjaminfuchs.de/
> really had trouble in the first version to nail down how to tell you are in
> a submodule, but people want to know that.

Right, I think it's an interesting thing to know, but I agree there are
probably a lot of corner cases.

> Maybe we need to change that fundamental assumption.
> So a more sophisticated way (thinking long term here) would be
> to include the superprojects config file (with exceptions), and that
> config file has more priority than e.g. the ~/.gitconfig file, but less
> than the submodules own $GIT_DIR/config file.

Yeah, that priority matches what I had been thinking.

> > One other caveat: I'm not sure if we do insteadOf recursively, but it
> > may be surprising to the child "git clone" that we've already applied
> > the insteadOf rewriting (especially if the rules are coming from
> > ~/.gitconfig and may be applied twice).
> 
> When a rule is having effect twice the rule sounds broken. (the outcome
> ought to be sufficiently different from the original?)

If you have:

  url.bar.insteadOf=foo
  url.baz.insteadOf=bar

do we convert "foo" to "baz"? If so, then I think applying the rules
again shouldn't matter. But if we don't, and only do a single level,
then having the caller rewrite the URL before it hands it to "git clone"
means we may end up unexpectedly doing two levels of rewriting.

-Peff

  parent reply	other threads:[~2017-02-21 23:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-19 21:12 url.<base>.insteadOf vs. submodules Toolforger
2017-02-20  9:01 ` Jeff King
2017-02-20 20:31   ` Toolforger
2017-02-20 20:52     ` Jeff King
2017-02-21  5:11       ` Toolforger
2017-02-21  7:06         ` Jeff King
2017-02-21 18:19           ` Stefan Beller
2017-02-21 23:00             ` Jeff King
2017-02-21 23:16               ` Stefan Beller
2017-02-21 23:37                 ` Junio C Hamano
2017-02-21 23:59                   ` Junio C Hamano
2017-02-22  0:07                   ` Stefan Beller
2017-02-22  2:59                     ` Junio C Hamano
2017-02-22 14:00                       ` Jon Loeliger
2017-02-22 17:36                         ` Junio C Hamano
2017-02-22 18:57                           ` Jeff King
2017-02-22 19:11                             ` Stefan Beller
2017-02-21 23:40                 ` Jeff King [this message]
2017-02-22  0:10                   ` Stefan Beller

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=20170221234037.ga44u3birwd5whab@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    --cc=toolforger@durchholz.org \
    /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 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 NNTP newsgroup(s).