From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] Conditional config includes based on remote URL
Date: Tue, 26 Oct 2021 12:12:12 +0200 [thread overview]
Message-ID: <211026.86bl3c13pw.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20211025185356.1232635-1-jonathantanmy@google.com>
On Mon, Oct 25 2021, Jonathan Tan wrote:
>> 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?
I meant the whole in-repo .gitconfig. I.e. to the extent that you're
submitting this as an alternative to that because of the negative
feedback on that RFC.
>> 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.
I mean with this and the below...
>> 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?
...what happens if you add a google.com remote for a repository that
"lives" on github.com. I.e. are the semantics "match any remote", or
"match the 'primary' remote (origin?" etc.
>> 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.
Ah, that explains it. I assumed both cases would be ones where the user
would need to manually enable the 'configuration' (or cloning to a given
subdir).
>> 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.
Yes, although aside from this hook that's a general caveat with the
proposed config-based hooks, I think if you need a hook that does that
(whether it's this, or pre-receive etc.) our answer is "put it in your
own wrapper".
> 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.
I suggested it because maybe it would be a lot simpler, i.e. we don't
need such a feature to be aware of remote config at all, or having to
"read forward" to find it, maybe it would be more complex. I haven't
tried to implement it.
>> 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().
On the "user would need to know" that's the same if it's config? I.e. in
either case it would be in /etc/gitconfig or whatever shipped by the
*.deb package.
Anyway, I really just meant this as a suggestion, and one that might
make things simpler. If you don't think it makes sense...
next prev parent reply other threads:[~2021-10-26 10:26 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
2021-10-26 10:12 ` Ævar Arnfjörð Bjarmason [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=211026.86bl3c13pw.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.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.
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.