git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: avarab@gmail.com
Cc: jonathantanmy@google.com, git@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] Conditional config includes based on remote URL
Date: Mon, 25 Oct 2021 11:53:56 -0700	[thread overview]
Message-ID: <20211025185356.1232635-1-jonathantanmy@google.com> (raw)
In-Reply-To: <211025.86lf2hgsmj.gmgdl@evledraar.gmail.com>

> I had some concerns about the specifics of the implementation/what
> seemed to be tailoring it a bit too closely to one use-case[1][2], not
> inherently with the idea (although I think e.g. for brian that more
> closely reflects his thoughts).
> 
> Anyway, just saying that aside from this RFC I don't think we were at
> the point of really fleshing out what this would look like, and there
> being some hard "no", so I think that idea could still be pursued.

Which idea specifically do you think could still be pursued?

> On this proposal: this also applies globally to all history, but I don't
> have the same concern with that as the 1=1 mapping of remote-suggested
> hooks, our path includes work that way, after all.
> 
> I think it would be nice if you could think about if/how this and the
> "onbranch" include would work together though to serve the general case
> better.
> 
> Also if you have a repo with N remotes each where "origin" tracks URLs
> at git.example.com, and you add a "dev" tracking dev.example.com, will
> the config apply if you're say on a branch tracking the "live" server,
> if you've said "include this for repos matching dev.example.com?

Right now, the feature is only dependent on remote URLs configured
through remote.?.url. It wouldn't work with "onbranch" because there's
no way to combine conditions (and I have no plans to do that). I think
that if you have something that you want depending on which branch
you're on, you can just use the existing "onbranch" feature.

> Arguably that's what you want, but perhaps something that those more
> used to the centralized workflows wouldn't consider as being unintuitive
> for users who might want to add this config only for their main "origin"
> remote. We don't really have a way of marking that special-ness though,
> except maybe checkout.defaultRemote.

What do you mean by adding a config for a specific remote?

> I'm also still somewhat mystified at how this would better serve your
> userbase than the path-based included, i.e. the selling point of the
> remote-suggested configuration was that it would Just Work.
> 
> But for this the users would either need to setup the config themselves
> for your remote, but that would be easier than pro-actively cloning in
> "work" or whatever? I guess, just wondering if I'm missing something.
> 
> Or if it's a partly-automated system where some automation is dropping
> in a /etc/gitconfig.d/google-remote-config-include 

Yes, the config is meant to be handled e.g. through a package manager
like apt. We don't want to prescribe directory structures like "work",
which is why the include is conditional upon the remote URL.

Even if the user pro-actively clones into "work", the user still needs
to set up the conditional config, so I don't see how that is a net
benefit.

> I wonder if this
> whole thing wouldn't be better for users with such special-needs if we
> just supported an "early config hook".
> 
> i.e. similar to how we read trace2 config from /etc/gitconfig early, we
> could start picking up a hook that just so happens to conform to the
> config schema Emily's config-based hooks use.
> 
> So the /etc/gitconfig would have say:
> 
>     hook.ourConfigThingy.command=/usr/bin/googly-git-config
>     hook.ourConfigThingy.event=include-config
> 
> That hook would just produce a config snippet to be included on STDOUT.
> 
> Since it's an arbitrary external command it would nicely get around any
> chicken and egg problems in git itself, it could run "git remote -v",
> inspect the equivalent of an "onbranch" etc. etc, then just dynamically
> produce config-to-be-included.

I see that later on, you suggest an environment variable to guard
against recursion.

One thing is that if there are multiple such hooks, each one won't be
able to see what the other hooks have produced.

If the feature you described already existed in Git, I think I could use
that, but if we're deciding between implementing the config hook you
describe versus something with more constraints, I think the one I
proposed is better for now. Some design points that have already been
discussed are whether setting a config during processing of an included
file would then invalidate the include and also the order of operations,
both of which would be much more difficult to control with config hooks.

> Please don't take this as some objection to your current proposal, just
> a thought on something that might entirely bypass odd edge cases and
> arbitrary limitations associated with doing this all in the "main"
> process on-the-fly.
> 
> The special-ness with that one would need to be that we'd say it
> wouldn't have the normal "last set wins" semantics, or maybe we could do
> that and just note that we saw it, and execute the "include" when we
> detect the end of the full config parsing (I'm not familiar enough with
> those bits to say where that is).

The "last set" would be those set by the hooks, so yes, a user would
need to know to make their own hook in order to override anything set by
the hooks. The end of the full config parsing is in
config_with_options().

> Both of those seem easier than dealing with any chicken & egg problems
> in parsing the config stream itself, since such a hook could just invoke
> "git remote -v" and the like itself, after e.g. setting some environment
> variable of its own to guard against its own recursion (or we'd do it
> for it for such hooks...).
> 
> 1. https://lore.kernel.org/git/87k0mn2dd3.fsf@evledraar.gmail.com/
> 2. https://lore.kernel.org/git/87o8awvglr.fsf@evledraar.gmail.com/

  reply	other threads:[~2021-10-25 18:54 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12 22:57 [RFC PATCH 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 1/2] config: make git_config_include() static Jonathan Tan
2021-10-12 23:07   ` Jeff King
2021-10-12 23:26   ` Junio C Hamano
2021-10-13  8:26   ` Ævar Arnfjörð Bjarmason
2021-10-13 17:00     ` Junio C Hamano
2021-10-13 18:13       ` Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-10-12 23:30   ` Jeff King
2021-10-13 18:33     ` Jonathan Tan
2021-10-27 11:40       ` Jeff King
2021-10-27 17:23         ` Jonathan Tan
2021-10-12 23:48   ` Junio C Hamano
2021-10-13 19:52     ` Jonathan Tan
2021-10-13  0:46 ` [RFC PATCH 0/2] Conditional config includes based on remote URL brian m. carlson
2021-10-13 18:17   ` Jonathan Tan
2021-10-18 20:48 ` Jonathan Tan
2021-10-22  3:12   ` Emily Shaffer
2021-10-27 11:55   ` Jeff King
2021-10-27 17:52     ` Jonathan Tan
2021-10-27 20:32       ` Jeff King
2021-10-25 13:03 ` Ævar Arnfjörð Bjarmason
2021-10-25 18:53   ` Jonathan Tan [this message]
2021-10-26 10:12     ` Ævar Arnfjörð Bjarmason
2021-10-29 17:31 ` [WIP v2 " Jonathan Tan
2021-10-29 17:31   ` [WIP v2 1/2] config: make git_config_include() static Jonathan Tan
2021-11-05 19:45     ` Emily Shaffer
2021-10-29 17:31   ` [WIP v2 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-05 20:24     ` Emily Shaffer
2021-11-06  4:41       ` Ævar Arnfjörð Bjarmason
2021-11-09  0:25         ` Jonathan Tan
2021-11-09  0:22       ` Jonathan Tan
2021-11-16  0:00 ` [PATCH v3 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 1/2] config: make git_config_include() static Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-22 22:59     ` Glen Choo
2021-11-29 17:53       ` Jonathan Tan
2021-11-23  1:22     ` Junio C Hamano
2021-11-29 18:18       ` Jonathan Tan
2021-12-01 18:51         ` Junio C Hamano
2021-12-02 23:14           ` Jonathan Tan
2021-11-23  1:27     ` Ævar Arnfjörð Bjarmason
2021-11-29 18:33       ` Jonathan Tan
2021-11-29 20:50         ` Ævar Arnfjörð Bjarmason
2021-11-29 20:23 ` [PATCH v4 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-29 20:23   ` [PATCH v4 1/2] config: make git_config_include() static Jonathan Tan
2021-11-29 20:23   ` [PATCH v4 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-02  6:57     ` Junio C Hamano
2021-12-02 17:41       ` Jonathan Tan
2021-11-29 20:48   ` [PATCH v4 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-11-30  7:51     ` Junio C Hamano
2021-12-02 23:31 ` [PATCH v5 " Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 1/2] config: make git_config_include() static Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-06 22:32     ` Glen Choo
2021-12-07 17:53       ` Jonathan Tan
2021-12-06 18:57   ` [PATCH v5 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-12-07 17:46     ` Jonathan Tan
2021-12-07 17:56       ` Ævar Arnfjörð Bjarmason
2021-12-07 18:52         ` Jonathan Tan
2021-12-07 23:23 ` [PATCH v6 " Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 1/2] config: make git_config_include() static Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-08 19:19     ` Glen Choo
2021-12-09 22:16       ` Jonathan Tan
2021-12-08 19:55     ` Glen Choo
2021-12-09 22:39       ` Jonathan Tan
2021-12-09 23:33         ` Glen Choo
2021-12-13 23:35           ` Jonathan Tan
2021-12-10 21:45         ` Junio C Hamano
2021-12-13 23:37           ` Jonathan Tan
2021-12-14 21:31 ` [PATCH v7 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 1/2] config: make git_config_include() static Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-16 21:54     ` Glen Choo
2021-12-28  0:55     ` Elijah Newren
2022-01-10 18:58       ` Jonathan Tan
2021-12-16 21:57   ` [PATCH v7 0/2] Conditional config includes based on remote URL Glen Choo
2021-12-28  1:13   ` Elijah Newren
2021-12-28 23:13     ` Glen Choo
2022-01-10 19:22     ` Jonathan Tan
2022-01-10 20:17       ` Elijah Newren
2022-01-25 13:26         ` Scalar vs JGit, was " Johannes Schindelin
2022-01-18 17:47 ` [PATCH v8 " Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 1/2] config: make git_config_include() static Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 2/2] config: include file if remote URL matches a glob Jonathan Tan
2022-01-18 20:54   ` [PATCH v8 0/2] Conditional config includes based on remote URL Elijah Newren

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=20211025185356.1232635-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.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.
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).