git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Git Mailing List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Barret Rhoden <brho@google.com>, Olaf Hering <olaf@aepfle.de>,
	Paul Okstad <pokstad@gitlab.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: Re: How to undo previously set configuration? (again)
Date: Fri, 26 Apr 2019 17:18:42 +0200	[thread overview]
Message-ID: <871s1osskt.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <87imv2s3dz.fsf@evledraar.booking.com>


On Thu, Apr 25 2019, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Apr 25 2019, Duy Nguyen wrote:
>
>> On Thu, Apr 25, 2019 at 5:08 PM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> >> Solving (1) without (2) feels like a bit of a missed opportunity to
>>> >> me.  Ideally, what I would like is
>>> >>
>>> >>    i. A central registry of trustworthy Git hooks that can be upgraded
>>> >>       using the system package manager to address (2).  Perhaps just
>>> >>       git-hook-* commands on the $PATH.
>>> >>
>>> >>   ii. Instead of putting hooks in .git/hooks, put a list of hooks to
>>> >>       run for each event in .git/config.
>>> >
>>> > The problem I had with this when discussing it was that our
>>> > configuration system lacks a good way to control inheritance from outer
>>> > files. I recently was working with a system-wide gitconfig file that
>>> > referred to files I didn't have, and my Git installation was subtly
>>> > broken in a variety of ways.
>>> >
>>> > If I have a system-wide hook to run for company code, but I have a
>>> > checkout for my personal dotfiles on my machine where I don't want to
>>> > run that hook, our configuration lacks a way for me to disable that
>>> > system-wide configuration. However, using our current system, I can
>>> > override core.hooksPath in this case and everything works fine.
>>> >
>>> > I mentioned this for completeness, and because I hope that some of those
>>> > people will get some time to chime in here, but I think without that
>>> > feature, we end up with a worse experience than we have now.
>>>
>>> I sent a proposal for this last year "How to undo previously set
>>> configuration?":
>>> https://public-inbox.org/git/874lkq11ug.fsf@evledraar.gmail.com/
>>
>> While reading that mail, it occurs to me that perhaps we can reuse the
>> .gitignore idea.
>>
>> Instead of having a list of untracked files, we have a list of config
>> keys. Instead of having .gitignore files associated to different
>> directories to apply the rules to those dirs only, we have ignore
>> rules that should apply on certain config files (probably based on
>> path).
>>
>> A few differences from your reject/accept/priority example:
>>
>> - we don't redefine priority, inheritance rules apply the same way
>> - reject/accept is handled the same way as positive/negative ignore
>> rules. If we're lucky, we could even reuse the exclude code.
>> - instead of special section names like
>>
>>     [config "section"]
>>
>> we have something more like
>>
>>     [config "/this/path"] # (or pattern)
>>
>> this lets us handle even other config files included by [include] or [includeIf]
>>
>> So, some examples
>>
>> [exclude]            # exclude from all inherited files
>>     key = core.*     # exclude core.*
>>     key = !core.bar  # but keep core.bar
>>
>> [excludeIf "path:/etc/config"] # rules apply for only this file
>>    key = ...
>>
>> [excludeIf "glob:/home/*"]     # rules apply for these config paths
>>    key = ...
>>
>> [excludeIf "system"]           # special names for convenience maybe
>>    key = ...
>>
>>> Obviously the main bottleneck is someone like me working on patching it,
>>
>> Yes, manpower is always the problem.
>>
>>> but in this case it would be very useful if those who are interested in
>>> this could look that proposal over and bikeshed it / point out issues I
>>> may have missed, i.e. "no, this categorically won't work with this
>>> proposed syntax due to XYZ you haven't thought of...".
>
> Thanks, I like this syntax/proposal much better than my initial one,
> especially re-using the syntax we have in .gitignore. Also in that it's
> more similar to the existing include syntax, which in retrospect with an
> example is the obviously better choice both in terms of UI consistency
> and flexibility.
>
> I.e. I didn't want config files by globs, because depending on compile
> options the /etc/gitconfig may be in /opt/etc/gitconfig, but as your
> '[excludeIf "system"]' and '[excludeIf "path:/etc/config"]' examples
> show we can have our cake and eat it too, and as you demonstrate there's
> other reasons to do path globs that excluding the "git config
> --{system,global,local,worktree}" file doesn't cover.
>
> Re priorities: My "I don't really have a use-case for that" in 2018 is
> still 95% true, just a couple of things:
>
>  1. Having it would be a really nice smoke test for this working
>     properly, i.e. now all the config parsing is "streamed" to its
>     consumers via callbacks, having priorities would require the ability
>     to pre-buffer and re-arrange it, the "pre-buffer" you'd need for any
>     exclude mechanism anyway.
>
>     Once we have that "priorities" should just be a quick sort of what
>     order we loop over the files in.
>
>  2. There is the use-case of "I don't want to exclude core.* from config
>     file <A>, I just want file <B> to override it". I can imagine
>     especially if/when we have in-repo .gitconfig that you'd want to
>     trust say core.* from there, but have you ~/.gitconfig override it
>     if you've bothered to set any of them yourself.
>
>     But I think most of that use-case doesn't need priorities. It could
>     just be another "exclude" syntax for common cases, e.g.:
>
>         # ...Having done something else previously to opt-in to scary
>         # in-repo .gitconfig...
>         [excludeIf "repo"]
>         key = core.* # exclude core.*
>         [excludeIf "repo"]
>         existingKey = true # exclude any existing key
>
>     So e.g. you'd keep that .gitconfig's gc.bigPackThreshold or
>     whatever, unless your ~/.gitconfig (parsed before, lower priority)
>     had already set it.

A #3 I just encountered[1] where this settable "config priority" might
be handy, but perhaps it's still stupid and exclusions are enough:

 * Vendor's git server wants to run 'git -c gc.writeCommitGraph gc' to
   get commit graphs. I might want to override it.

 * The vendor can't just add that to /etc/gitconfig because they don't
   want to screw with the OS, or might not know which "git" they'll use
   (their own or OS, so system "gitconfig" in different
   places/inconsistent)

So something where they can just do that and I can in what *I* know to
be the system "gitconfig" do:

    [configPriority "cli-at-cwd:/var/lib/vendor/git-storage/*"]
    value = 5

If I know they'll be be running those commands on that path, and I'd
like to s/100/5/ the priority for "-c" there so it goes last (see the
suggested priority numbers in [2]).

Or maybe alternatively, we'd have something like "-c" (unfortunately
"-C" is taken) to add a new config file to the mix, without making it an
all-or-nothing like GIT_CONFIG_NOSYSTEM=1 and GIT_CONFIG=<path>). So
they:

    git --add-this-config-last-please=/var/lib/vendor/etc/gitaly/gitconfig gc

And then I do in /etc/gitconfig:

    [excludeIf "glob:/var/lib/vendor/etc/gitaly/gitconfig"]
    key = gc.*

But priorities might still be sensible. This use-case could be
alternatively done without them with a more sensible version of
"excludeIf.existingKey" discussed in the last mail. I.e. having
"existingKey" be a glob, not "true":

    [excludeIf "glob:/var/lib/vendor/etc/gitaly/gitconfig"]
    existingKey = gc.*

Ditto for "-c" values:

    [excludeIf "cli-at-cwd:/var/lib/vendor/git-storage/*"]
    existingKey = gc.*

So maybe I've managed to talk myself out this whole notion of
priorities.

I.e. maybe we can always process config in a pre-determined order and
just allow people to reach forward/backward with [excludeIf path/glob/-c
at cwd] & [exclude], respectively.

There's still the *theoretical* use-case of a user saying "I know the
sysadmin here knows better, I want their /etc/gitconfig to go after my
~/.gitconfig", but does anyone need/want it in practice? I don't know...

1. https://gitlab.com/gitlab-org/gitaly/issues/1643
2. https://public-inbox.org/git/874lkq11ug.fsf@evledraar.gmail.com/

  reply	other threads:[~2019-04-26 15:18 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-24  0:49 [PATCH 0/5] Multiple hook support brian m. carlson
2019-04-24  0:49 ` [PATCH 1/5] run-command: add preliminary support for multiple hooks brian m. carlson
2019-04-24  2:27   ` Junio C Hamano
2019-04-24 18:48     ` Johannes Sixt
2019-04-25  0:55       ` Junio C Hamano
2019-04-25  9:39         ` Ævar Arnfjörð Bjarmason
2019-04-25 10:04           ` Junio C Hamano
2019-04-25 19:40         ` Johannes Sixt
2019-04-26 20:58           ` brian m. carlson
2019-04-26 21:53             ` Johannes Sixt
2019-04-24 22:32     ` brian m. carlson
2019-04-24  0:49 ` [PATCH 2/5] builtin/receive-pack: add " brian m. carlson
2019-04-24  0:49 ` [PATCH 3/5] sequencer: " brian m. carlson
2019-04-24  9:51   ` Phillip Wood
2019-04-24 22:46     ` brian m. carlson
2019-04-25 14:59       ` Phillip Wood
2019-04-24  0:49 ` [PATCH 4/5] builtin/worktree: add support for multiple post-checkout hooks brian m. carlson
2019-04-24  0:49 ` [PATCH 5/5] transport: add support for multiple pre-push hooks brian m. carlson
2019-04-24  2:09 ` [PATCH 0/5] Multiple hook support Junio C Hamano
2019-04-24  2:22   ` brian m. carlson
2019-04-24  2:41     ` Junio C Hamano
2019-04-24  8:14     ` Ævar Arnfjörð Bjarmason
2019-04-24  2:34 ` Jonathan Nieder
2019-04-24  7:43   ` Ævar Arnfjörð Bjarmason
2019-04-24  8:22   ` Ævar Arnfjörð Bjarmason
2019-04-24 23:07   ` brian m. carlson
2019-04-24 23:26     ` Jonathan Nieder
2019-04-25 10:08     ` How to undo previously set configuration? (again) Ævar Arnfjörð Bjarmason
2019-04-25 10:43       ` Duy Nguyen
2019-04-25 11:58         ` Ævar Arnfjörð Bjarmason
2019-04-26 15:18           ` Ævar Arnfjörð Bjarmason [this message]
2019-04-25 14:36       ` Jonathan Nieder
2019-04-25 14:43         ` Barret Rhoden
2019-04-25 15:27           ` Ævar Arnfjörð Bjarmason
2019-04-25 15:25         ` Ævar Arnfjörð Bjarmason
2019-04-26  2:13         ` Junio C Hamano
2019-04-26  9:36         ` Duy Nguyen
2019-04-30 21:14           ` Jeff King
2019-05-01 11:41             ` Duy Nguyen
2019-05-01 12:18               ` Ævar Arnfjörð Bjarmason
2019-05-01 12:32                 ` Duy Nguyen
2019-05-01 21:09                   ` Jeff King
2019-05-01 21:15                 ` Jeff King
2019-04-24  8:10 ` [PATCH 0/5] Multiple hook support Ævar Arnfjörð Bjarmason
2019-04-24  9:55   ` Phillip Wood
2019-04-24 18:29   ` Bryan Turner
2019-04-24  9:49 ` Duy Nguyen
2019-04-24 22:49   ` brian m. carlson
2019-04-24 23:40   ` brian m. carlson
2019-04-25  0:08     ` Duy Nguyen
2019-04-30 21:39 ` Jeff King

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=871s1osskt.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=brho@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=olaf@aepfle.de \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=pokstad@gitlab.com \
    --cc=sandals@crustytoothpaste.net \
    --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.
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).