Re: How to undo previously set configuration? (again)

2019-05-01 Thread Jeff King
On Wed, May 01, 2019 at 02:18:34PM +0200, Ævar Arnfjörð Bjarmason wrote:

> We can make it fancier, but we already deal with this, e.g. if you do
> "git config -l" we'll show "include{,if}" directives at the same "level"
> as other "normal" keys.

We show them, but we _do_ interpret them if the caller asks for it with
--includes (which defaults to on when doing the usual "look in all
files").

I think we'd have something similar here, where the caller can ask to
apply excludes or not.

> We also provide no way in "git config" to properly interpret a
> value. E.g. does a "user.email" showing up twice for me mean I have two
> E-Mails at the same time, or does the last one win? We both know the
> answer, but git-config itself doesn't, and that information lives in
> docs/code outside of it.
> 
> Similarly we'd just print a sequence of:
> 
> user.name=foo
> user.email=bar
> exclude.key=user.*
> user.name=baz
> 
> And it would be up to some "smarter" reader of the config data to
> realize that the end result is one where we have no "user.email" set,
> and "user.name=baz".
> 
> But yeah, optionally having some new --list-normalized or
> --list-after-excludes or whatever would be great, and presumably not
> hard if we had some central "excludes" mechanism...

I think that is all because "--list" really is just about dumping all
values, not about interpreting. If we had an exclude mechanism, then I'd
expect:

  git config user.name

to apply it just like git_config_get_string() would.

Because of its lack of interpretation, I don't think --list is actually
good for much besides debugging. Some scripts do use it to avoid making
a bunch of individual git-config calls, but they'd be much better served
by a --stdin mode which let you feed in a sequence of operations ("I want
x.y.z, as a single last-one-wins value, and interpreted as a bool") and
get a sequence of outputs.

-Peff


Re: How to undo previously set configuration? (again)

2019-05-01 Thread Jeff King
On Wed, May 01, 2019 at 07:32:20PM +0700, Duy Nguyen wrote:

> > We also provide no way in "git config" to properly interpret a
> > value. E.g. does a "user.email" showing up twice for me mean I have two
> > E-Mails at the same time, or does the last one win?
> 
> Actually --get knows this. Single-valued options can be handled
> correctly quite easily. It's --get-all (or rather, the future
> --get-multi because we can't change --get-all's behavior) which can't
> interpret values because there's no standardized way of doing it.

Right. We need a hint from the caller about how they expect us to
interpret the values. And I agree we should probably introduce a new
verb instead of modifying --get-all, which some callers might be
expecting to do their own list processing.

Likewise in the C code. We probably want to leave existing callers of
git_config_get_value_multi() alone, and give them a new
git_config_get_string_list() or something.

-Peff


Re: How to undo previously set configuration? (again)

2019-05-01 Thread Duy Nguyen
On Wed, May 1, 2019 at 7:18 PM Ævar Arnfjörð Bjarmason  wrote:
>
>
> On Wed, May 01 2019, Duy Nguyen wrote:
>
> > On Wed, May 1, 2019 at 4:14 AM Jeff King  wrote:
> >> It's definitely not implemented universally; each consumer of the config
> >> option must decide on it (and it will probably always be that way to
> >> some degree, since we don't know the semantics of each options; recall
> >> that we may be holding config keys for other non-core programs, too).
> >> And we just haven't retro-fitted a lot of those older options because
> >> nobody has been bothered by it.
> >>
> >> That said, I am a proponent of having some kind of clearing mechanism
> >> (and I was the one who added credential.helper's mechanism, which has
> >> been mentioned in this thread).  I think it makes things a lot less
> >> difficult if we don't have to change the syntax of the config files to
> >> do it. With that constraint, that pretty much leaves:
> >>
> >>   1. Some sentinel value like the empty string. That one _probably_
> >>  works in most cases, but there may be lists which want to represent
> >>  the empty value. There could be other sentinel values (e.g.,
> >>  "CLEAR") which are simply unlikely to be used as real values.
> >>
> >>   2. The boolean syntax (i.e., "[foo]bar" with no equals) is almost
> >>  always bogus for a list. So that can work as a sentinel that is
> >>  OK syntactically.
> >
> > Another question about the universal clearing mechanism, how does "git
> > config" fit into this? It would be great if the user can actually see
> > the same thing a git command sees to troubleshoot. Option 1 leaves the
> > interpretation/guessing to the user, "git config" simply gives the raw
> > input list before "clear" is processed. Option 2, "git config"
> > probably can be taught to optionally clear when it sees the boolean
> > syntax.
>
> We can make it fancier, but we already deal with this, e.g. if you do
> "git config -l" we'll show "include{,if}" directives at the same "level"
> as other "normal" keys.
>
> We also provide no way in "git config" to properly interpret a
> value. E.g. does a "user.email" showing up twice for me mean I have two
> E-Mails at the same time, or does the last one win?

Actually --get knows this. Single-valued options can be handled
correctly quite easily. It's --get-all (or rather, the future
--get-multi because we can't change --get-all's behavior) which can't
interpret values because there's no standardized way of doing it.

> We both know the
> answer, but git-config itself doesn't, and that information lives in
> docs/code outside of it.
>
> Similarly we'd just print a sequence of:
>
> user.name=foo
> user.email=bar
> exclude.key=user.*
> user.name=baz
>
> And it would be up to some "smarter" reader of the config data to
> realize that the end result is one where we have no "user.email" set,
> and "user.name=baz".
>
> But yeah, optionally having some new --list-normalized or
> --list-after-excludes or whatever would be great, and presumably not
> hard if we had some central "excludes" mechanism...
-- 
Duy


Re: How to undo previously set configuration? (again)

2019-05-01 Thread Ævar Arnfjörð Bjarmason


On Wed, May 01 2019, Duy Nguyen wrote:

> On Wed, May 1, 2019 at 4:14 AM Jeff King  wrote:
>> It's definitely not implemented universally; each consumer of the config
>> option must decide on it (and it will probably always be that way to
>> some degree, since we don't know the semantics of each options; recall
>> that we may be holding config keys for other non-core programs, too).
>> And we just haven't retro-fitted a lot of those older options because
>> nobody has been bothered by it.
>>
>> That said, I am a proponent of having some kind of clearing mechanism
>> (and I was the one who added credential.helper's mechanism, which has
>> been mentioned in this thread).  I think it makes things a lot less
>> difficult if we don't have to change the syntax of the config files to
>> do it. With that constraint, that pretty much leaves:
>>
>>   1. Some sentinel value like the empty string. That one _probably_
>>  works in most cases, but there may be lists which want to represent
>>  the empty value. There could be other sentinel values (e.g.,
>>  "CLEAR") which are simply unlikely to be used as real values.
>>
>>   2. The boolean syntax (i.e., "[foo]bar" with no equals) is almost
>>  always bogus for a list. So that can work as a sentinel that is
>>  OK syntactically.
>
> Another question about the universal clearing mechanism, how does "git
> config" fit into this? It would be great if the user can actually see
> the same thing a git command sees to troubleshoot. Option 1 leaves the
> interpretation/guessing to the user, "git config" simply gives the raw
> input list before "clear" is processed. Option 2, "git config"
> probably can be taught to optionally clear when it sees the boolean
> syntax.

We can make it fancier, but we already deal with this, e.g. if you do
"git config -l" we'll show "include{,if}" directives at the same "level"
as other "normal" keys.

We also provide no way in "git config" to properly interpret a
value. E.g. does a "user.email" showing up twice for me mean I have two
E-Mails at the same time, or does the last one win? We both know the
answer, but git-config itself doesn't, and that information lives in
docs/code outside of it.

Similarly we'd just print a sequence of:

user.name=foo
user.email=bar
exclude.key=user.*
user.name=baz

And it would be up to some "smarter" reader of the config data to
realize that the end result is one where we have no "user.email" set,
and "user.name=baz".

But yeah, optionally having some new --list-normalized or
--list-after-excludes or whatever would be great, and presumably not
hard if we had some central "excludes" mechanism...


Re: How to undo previously set configuration? (again)

2019-05-01 Thread Duy Nguyen
On Wed, May 1, 2019 at 4:14 AM Jeff King  wrote:
> It's definitely not implemented universally; each consumer of the config
> option must decide on it (and it will probably always be that way to
> some degree, since we don't know the semantics of each options; recall
> that we may be holding config keys for other non-core programs, too).
> And we just haven't retro-fitted a lot of those older options because
> nobody has been bothered by it.
>
> That said, I am a proponent of having some kind of clearing mechanism
> (and I was the one who added credential.helper's mechanism, which has
> been mentioned in this thread).  I think it makes things a lot less
> difficult if we don't have to change the syntax of the config files to
> do it. With that constraint, that pretty much leaves:
>
>   1. Some sentinel value like the empty string. That one _probably_
>  works in most cases, but there may be lists which want to represent
>  the empty value. There could be other sentinel values (e.g.,
>  "CLEAR") which are simply unlikely to be used as real values.
>
>   2. The boolean syntax (i.e., "[foo]bar" with no equals) is almost
>  always bogus for a list. So that can work as a sentinel that is
>  OK syntactically.

Another question about the universal clearing mechanism, how does "git
config" fit into this? It would be great if the user can actually see
the same thing a git command sees to troubleshoot. Option 1 leaves the
interpretation/guessing to the user, "git config" simply gives the raw
input list before "clear" is processed. Option 2, "git config"
probably can be taught to optionally clear when it sees the boolean
syntax.
-- 
Duy


Re: How to undo previously set configuration? (again)

2019-04-30 Thread Jeff King
On Fri, Apr 26, 2019 at 04:36:40PM +0700, Duy Nguyen wrote:

> > I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
> > That is, I thought the intended behavior is always
> >
> >  1. For single-valued options, last value wins.
> >  2. For multi-valued options, empty clears the list.
> 
> I didn't know this! Should it be documented? At least a quick skimming
> through config.txt does not mention anything about empty value
> clearing multi-valued options.
> 
> I also wanted to see if it's true. However, the first var I checked,
> branch.*.merge does not follow this rule. I got disappointed and
> stopped.

It's definitely not implemented universally; each consumer of the config
option must decide on it (and it will probably always be that way to
some degree, since we don't know the semantics of each options; recall
that we may be holding config keys for other non-core programs, too).
And we just haven't retro-fitted a lot of those older options because
nobody has been bothered by it.

That said, I am a proponent of having some kind of clearing mechanism
(and I was the one who added credential.helper's mechanism, which has
been mentioned in this thread).  I think it makes things a lot less
difficult if we don't have to change the syntax of the config files to
do it. With that constraint, that pretty much leaves:

  1. Some sentinel value like the empty string. That one _probably_
 works in most cases, but there may be lists which want to represent
 the empty value. There could be other sentinel values (e.g.,
 "CLEAR") which are simply unlikely to be used as real values.

  2. The boolean syntax (i.e., "[foo]bar" with no equals) is almost
 always bogus for a list. So that can work as a sentinel that is
 OK syntactically.

> > Which of course leaves room for improvement in documentation and how
> > we organize the implementation (as Peff discussed in [1]), but isn't
> > it nice to already have something in place that doesn't require
> > inventing a new syntax?
> 
> This cannot undefine a variable though, especially those single-valued
> ones. But I think for most cases, the user just needs to find out what
> is the default value and set to that one.

Having "default" is a little more convenient, but it would need to be
implemented on a per-value basis anyway. In many cases the caller of the
config code has implemented last-one-wins by overwriting an old value,
and only it knows how to restore "default".

I guess if we moved completely to a configset world, then asking for
git_config_get_string() could resolve "default" completely. But we are a
ways from that.

-Peff


Re: How to undo previously set configuration? (again)

2019-04-26 Thread Ævar Arnfjörð Bjarmason


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
>>  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@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 , I just want file  to override it". I can imagine
> especially if/when we have in-repo .gitconfig that you'd want to
> trust say core.* from there, b

Re: How to undo previously set configuration? (again)

2019-04-26 Thread Duy Nguyen
On Thu, Apr 25, 2019 at 9:36 PM Jonathan Nieder  wrote:
>
> Hi,
>
> Ævar Arnfjörð Bjarmason wrote:
>
> > Because we don't have some general config facility for this it keeps
> > coming up, and various existing/proposed options have their own little
> > custom hacks for doing it, e.g. this for Barret Rhoden's proposed "blame
> > skip commits" feature
> > https://public-inbox.org/git/878swhfzxb@evledraar.gmail.com/
> > (b.t.w. I *meant* /dev/null in that E-Mail, but due to PEBCAK wrote
> > /dev/zero).
>
> I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
> That is, I thought the intended behavior is always
>
>  1. For single-valued options, last value wins.
>  2. For multi-valued options, empty clears the list.

I didn't know this! Should it be documented? At least a quick skimming
through config.txt does not mention anything about empty value
clearing multi-valued options.

I also wanted to see if it's true. However, the first var I checked,
branch.*.merge does not follow this rule. I got disappointed and
stopped.

>  3. When there is a special behavior triggered by not supplying the
> option at all, offer an explicit value like "default" that triggers
> the same behavior, too.
>
> and that any instance of a command that isn't following that is a bug.

Not sure how many we need to fix. The behaviour does make sense to me though.

> Which of course leaves room for improvement in documentation and how
> we organize the implementation (as Peff discussed in [1]), but isn't
> it nice to already have something in place that doesn't require
> inventing a new syntax?

This cannot undefine a variable though, especially those single-valued
ones. But I think for most cases, the user just needs to find out what
is the default value and set to that one.

I don't know if there is any case where the lack of a variable behaves
differently (or the default value is too dynamic to be set manually)
though.

> Thanks,
> Jonahtan
>
> [1] https://public-inbox.org/git/20180406175044.ga32...@sigill.intra.peff.net/



-- 
Duy


Re: How to undo previously set configuration? (again)

2019-04-25 Thread Junio C Hamano
Jonathan Nieder  writes:

> I'm confused.  Isn't that bog-standard Git usage, not a custom hack?

Depends on the definition of "hack".

The only native provision at the config API level that lets the
calling programs realize the "desired behaviour" you listed below
without doing anything special is "for a single-valued one, the last
one wins", for which the newer style config_get_value() will give
you the "last one wins" semantics.  With the original style parsing
using the git_config() with callback still needs to do the "last one
wins" by the caller of the config API.

The "empty clears the list" for multi-valued option is totally up to
the application.  Even the newer configset__get_value_multi() API
function does not reset a list to empty when it sees an empty
string.  The caller gets a list of value strings that happens to
have an empty string in the middle.

So, I think the "bog-standard" Git convention must be maintained by
each application codepath to implement it with a custom hack.

Perhaps you did not mean with "hack" a "hacky implementation", but a
"hacky design".  If that is the case, yeah, I agree with you that
the items #1 and #2 below are what we try to make sure our programs
follow; I am not sure about #3 myself, as I do not think offhand a
good example of it, though.

> That is, I thought the intended behavior is always
>
>  1. For single-valued options, last value wins.
>  2. For multi-valued options, empty clears the list.
>  3. When there is a special behavior triggered by not supplying the
> option at all, offer an explicit value like "default" that triggers
> the same behavior, too.
>
> and that any instance of a command that isn't following that is a bug.

If we make that declaration, we could enforce the "empty clears the
list" at the API level when the configset API is in use.


Re: How to undo previously set configuration? (again)

2019-04-25 Thread Ævar Arnfjörð Bjarmason


On Thu, Apr 25 2019, Barret Rhoden wrote:

> Hi -
>
> On 4/25/19 10:36 AM, Jonathan Nieder wrote:
>> Hi,
>>
>> Ævar Arnfjörð Bjarmason wrote:
>>
>>> Because we don't have some general config facility for this it keeps
>>> coming up, and various existing/proposed options have their own little
>>> custom hacks for doing it, e.g. this for Barret Rhoden's proposed "blame
>>> skip commits" feature
>>> https://public-inbox.org/git/878swhfzxb@evledraar.gmail.com/
>>> (b.t.w. I*meant*  /dev/null in that E-Mail, but due to PEBCAK wrote
>>> /dev/zero).
>> I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
>> That is, I thought the intended behavior is always
>>
>>   1. For single-valued options, last value wins.
>>   2. For multi-valued options, empty clears the list.
>>   3. When there is a special behavior triggered by not supplying the
>>  option at all, offer an explicit value like "default" that triggers
>>  the same behavior, too.
>>
>> and that any instance of a command that isn't following that is a bug.
>
> Not sure if it's meant to be the standard, but I just went with the
> style used by credential.helper.  This was suggested to me by Johannes
> in [1]:

Just to be clear I'm not picking on your patch, I think doing it that
way makes perfect sense in the current system.

Just as noted upthread using it as an example (that was fresh in my
mind...) of a case where we want this, but it's a bespoke thing for
every use-case, and not consistent (i.e. just supported for your new
thing, but not for the existing fsck code that's mostly the same).

> On 2019-01-18 at 10:47 Johannes Schindelin 
> wrote:
>> A better idea IMHO would be to use an OPT_STRING_LIST() for
>> `--ignore-revs-file`, too, and to allow for multiple
>> `blame.ignoreRevsFile` config entries (with our usual trick of handling an
>> empty setting by resetting the list of paths that were accumulated so
>> far, see e.g. how `credential.helper` is handled).
>
> [1]
> https://public-inbox.org/git/nycvar.qro.7.76.6.1901181038540...@tvgsbejvaqbjf.bet/
>
> Barret


Re: How to undo previously set configuration? (again)

2019-04-25 Thread Ævar Arnfjörð Bjarmason


On Thu, Apr 25 2019, Jonathan Nieder wrote:

> Ævar Arnfjörð Bjarmason wrote:
>
>> Because we don't have some general config facility for this it keeps
>> coming up, and various existing/proposed options have their own little
>> custom hacks for doing it, e.g. this for Barret Rhoden's proposed "blame
>> skip commits" feature
>> https://public-inbox.org/git/878swhfzxb@evledraar.gmail.com/
>> (b.t.w. I *meant* /dev/null in that E-Mail, but due to PEBCAK wrote
>> /dev/zero).
>
> I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
> That is, I thought the intended behavior is always
>
>  1. For single-valued options, last value wins.
>  2. For multi-valued options, empty clears the list.
>  3. When there is a special behavior triggered by not supplying the
> option at all, offer an explicit value like "default" that triggers
> the same behavior, too.
>
> and that any instance of a command that isn't following that is a bug.
>
> Which of course leaves room for improvement in documentation and how
> we organize the implementation (as Peff discussed in [1]), but isn't
> it nice to already have something in place that doesn't require
> inventing a new syntax?

We can quibble over the wording, but I'd say that's a "hack". Yeah for
*some* options setting it to an empty value clears whether it's a
single-value or multi-value.

But it's entirely dependent on the specific callback implemented for
that option, sometimes there's no way to "reset" something, and in the
best case you'd need to exhaustively enumerate everything (e.g. for the
sendemail.* stuff) and hope nobody adds a new option tomorrow, you can't
say "clear this wildard" or "no config from system-wide".

So I think it makes sense to think about how we could promote this to
the config syntax itself so that what somewhat-mostly-but-not-quite
works now by convention would be guaranteed to work, and things that you
can't do at all today (e.g. ignore system config, or selectively ignore
something) would be supported.


Re: How to undo previously set configuration? (again)

2019-04-25 Thread Barret Rhoden

Hi -

On 4/25/19 10:36 AM, Jonathan Nieder wrote:

Hi,

Ævar Arnfjörð Bjarmason wrote:


Because we don't have some general config facility for this it keeps
coming up, and various existing/proposed options have their own little
custom hacks for doing it, e.g. this for Barret Rhoden's proposed "blame
skip commits" feature
https://public-inbox.org/git/878swhfzxb@evledraar.gmail.com/
(b.t.w. I*meant*  /dev/null in that E-Mail, but due to PEBCAK wrote
/dev/zero).

I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
That is, I thought the intended behavior is always

  1. For single-valued options, last value wins.
  2. For multi-valued options, empty clears the list.
  3. When there is a special behavior triggered by not supplying the
 option at all, offer an explicit value like "default" that triggers
 the same behavior, too.

and that any instance of a command that isn't following that is a bug.


Not sure if it's meant to be the standard, but I just went with the 
style used by credential.helper.  This was suggested to me by Johannes 
in [1]:


On 2019-01-18 at 10:47 Johannes Schindelin 
wrote:

A better idea IMHO would be to use an OPT_STRING_LIST() for
`--ignore-revs-file`, too, and to allow for multiple
`blame.ignoreRevsFile` config entries (with our usual trick of handling an
empty setting by resetting the list of paths that were accumulated so
far, see e.g. how `credential.helper` is handled).


[1] 
https://public-inbox.org/git/nycvar.qro.7.76.6.1901181038540...@tvgsbejvaqbjf.bet/


Barret



Re: How to undo previously set configuration? (again)

2019-04-25 Thread Jonathan Nieder
Hi,

Ævar Arnfjörð Bjarmason wrote:

> Because we don't have some general config facility for this it keeps
> coming up, and various existing/proposed options have their own little
> custom hacks for doing it, e.g. this for Barret Rhoden's proposed "blame
> skip commits" feature
> https://public-inbox.org/git/878swhfzxb@evledraar.gmail.com/
> (b.t.w. I *meant* /dev/null in that E-Mail, but due to PEBCAK wrote
> /dev/zero).

I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
That is, I thought the intended behavior is always

 1. For single-valued options, last value wins.
 2. For multi-valued options, empty clears the list.
 3. When there is a special behavior triggered by not supplying the
option at all, offer an explicit value like "default" that triggers
the same behavior, too.

and that any instance of a command that isn't following that is a bug.

Which of course leaves room for improvement in documentation and how
we organize the implementation (as Peff discussed in [1]), but isn't
it nice to already have something in place that doesn't require
inventing a new syntax?

Thanks,
Jonahtan

[1] https://public-inbox.org/git/20180406175044.ga32...@sigill.intra.peff.net/


Re: How to undo previously set configuration? (again)

2019-04-25 Thread Ævar Arnfjörð Bjarmason


On Thu, Apr 25 2019, Duy Nguyen wrote:

> On Thu, Apr 25, 2019 at 5:08 PM Ævar Arnfjörð Bjarmason
>  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@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 , I just want file  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

Re: How to undo previously set configuration? (again)

2019-04-25 Thread Duy Nguyen
On Thu, Apr 25, 2019 at 5:08 PM Ævar Arnfjörð Bjarmason
 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@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...".
-- 
Duy


Re: How to undo previously set configuration?

2018-04-06 Thread Jeff King
On Fri, Apr 06, 2018 at 05:57:39PM +0100, Rafael Ascensao wrote:

> On Fri, Apr 6, 2018 at 4:55 PM, Olaf Hering  wrote:
> >
> > This does not work. Initially I copied the global config into the
> > repo and set all unwanted values to , like 'smtpuser='.
> > Perhaps the config parser recognized that fact, but the consumer
> > does not?
> >
> 
> Today someone asked on #git for a way to disable diff.external for a
> single command.
> Without thinking much I suggested $git -c diff.external= diff; which fails 
> with:
> `fatal: cannot run : No such file or directory`
> 
> In this particular case there was `--no-ext-diff` to get around this
> and the case was promptly resolved.
> Just another another example where setting configuration values to
> "empty" doesn't translate to "disable this option".

Yeah, I think it would make sense in that case to reset
external_diff_cmd_cfg to NULL. I'd almost say that git_config_string()
should recognize and handle this case, but I suspect there are some
callers who need special behavior (e.g., to set it back to some
particular string).

-Peff


Re: How to undo previously set configuration?

2018-04-06 Thread Rafael Ascensao
On Fri, Apr 6, 2018 at 4:55 PM, Olaf Hering  wrote:
>
> This does not work. Initially I copied the global config into the repo and 
> set all unwanted values to , like 'smtpuser='. Perhaps the config 
> parser recognized that fact, but the consumer does not?
>

Today someone asked on #git for a way to disable diff.external for a
single command.
Without thinking much I suggested $git -c diff.external= diff; which fails with:
`fatal: cannot run : No such file or directory`

In this particular case there was `--no-ext-diff` to get around this
and the case was promptly resolved.
Just another another example where setting configuration values to
"empty" doesn't translate to "disable this option".


Re: How to undo previously set configuration?

2018-04-06 Thread Jeff King
On Fri, Apr 06, 2018 at 05:55:56PM +0200, Olaf Hering wrote:

> > The general strategy in Git's config is that instead of "unsetting",
> > you
> > should overwrite with whatever value you _do_ want. So a config option
> > like sendemail.smtpauth should accept some kind of empty or "none" value
> > to disable auth.
> > 
> > Most single-value config options should work this way (and if one
> > doesn't, I'd say that's a bug we should fix).
> 
> This does not work. Initially I copied the global config into the repo
> and set all unwanted values to , like 'smtpuser='. Perhaps the
> config parser recognized that fact, but the consumer does not?

Yes. That logic is handled by the consumer (especially in the case of
send-email, which is a perl script, the code runs in a totally separate
process from the config parser). Naively I'd think that:

  [sendemail]
  smtpAuth =

would do what you want, and then we'd ignore smtpUser completely.
It looks like the smtp_auth_maybe function uses perl's "defined" there,
though. Perhaps it should treat the empty string the same there.

(Or maybe it's something else; I don't use send-email myself, and in a
few trivial examples I couldn't get it to complain about similar
config).

-Peff


Re: How to undo previously set configuration?

2018-04-06 Thread Olaf Hering
Am Thu, 5 Apr 2018 12:32:27 -0400
schrieb Jeff King :

> The general strategy in Git's config is that instead of "unsetting", you
> should overwrite with whatever value you _do_ want. So a config option
> like sendemail.smtpauth should accept some kind of empty or "none" value
> to disable auth.
> 
> Most single-value config options should work this way (and if one
> doesn't, I'd say that's a bug we should fix).

This does not work. Initially I copied the global config into the repo and set 
all unwanted values to , like 'smtpuser='. Perhaps the config parser 
recognized that fact, but the consumer does not?

Olaf


pgpbEK2CI1ZX0.pgp
Description: Digitale Signatur von OpenPGP


Re: How to undo previously set configuration?

2018-04-05 Thread Jeff King
On Thu, Apr 05, 2018 at 03:25:25PM +0200, Olaf Hering wrote:

> Am Thu, 05 Apr 2018 13:21:02 +0200
> schrieb Ævar Arnfjörð Bjarmason :
> 
> > I'm assuming you mean something like:
> > [user]
> > # This is an error
> > -email
> 
> Yes. Just some flag to say "whatever value this variable has from
> earlier parsing, forget it in case it really exists". Just like "unset
> PATH" in bash.
> 
> I do not know the git internals, so can not really help with the case.

The general strategy in Git's config is that instead of "unsetting", you
should overwrite with whatever value you _do_ want. So a config option
like sendemail.smtpauth should accept some kind of empty or "none" value
to disable auth.

Most single-value config options should work this way (and if one
doesn't, I'd say that's a bug we should fix).

Multi-valued config (e.g., "remote.*.fetch") is harder, since it's
inherently a list where each new instance adds to the list. We've
discussed there having an empty value reset the list, but it's not
applied consistently.

-Peff


Re: How to undo previously set configuration?

2018-04-05 Thread Olaf Hering
Am Thu, 05 Apr 2018 13:21:02 +0200
schrieb Ævar Arnfjörð Bjarmason :

> I'm assuming you mean something like:
> [user]
> # This is an error
> -email

Yes. Just some flag to say "whatever value this variable has from earlier 
parsing, forget it in case it really exists". Just like "unset PATH" in bash.

I do not know the git internals, so can not really help with the case.

Olaf


pgpnLQis2dHKn.pgp
Description: Digitale Signatur von OpenPGP


RE: How to undo previously set configuration?

2018-04-05 Thread Randall S. Becker
On April 5, 2018 7:21 AM, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Apr 05 2018, Olaf Hering wrote:
> 
> > Am Thu, 05 Apr 2018 10:42:15 +0200
> > schrieb Ævar Arnfjörð Bjarmason :
> >
> >> I've been meaning to work on this but haven't figured out a good syntax
> for it (suggestions welcome!).
> >
> > Just prefix the knob with something like "no." or "-" or whatever to 
> > indicate
> that it never happened.
> 
> Those wouldn't work, respectively, because:
> 
>  a) For 'no.' there would be no way to override three-level keys,
> because prefixing such a key with "no" would introduce a 4th nesting
> level, which would be incompatible with existing config parsers.
> 
>  b) Similarly a prefix of - dies in git now. Unless I misunderstand
> you. I'm assuming you mean something like:
> 
> [user]
> # This is an error
> -email
> 
> Although I see we don't ignore or error out on:
> 
> [user "-email"]
> foo=bar
> 
>But that's back to problem a), and also looks ugly since you need
>something like the extra foo=bar so we'll pay attention to it.
> 
> Also it's important that the syntax leaves room for item #1 that I mentioned,
> 
> I.e. not just ignore stuff like user.email, but being able to specify where 
> from
> you'd like to ignore that. Sometimes your local sysadmin is overzealous with
> his /etc/gitconfig settings and you'd like to quarantine just that, but pay
> attention to everything else in ~/.gitconfig, or similarly in
> /some/repo/.git/config ignore your usual custom sendemail.* from
> ~/.gitconfig but not /etc/gitconfig, so the semantics can't just be "clear
> existing".
> 
> But of course, you might say that it *should* be a syntax error because if you
> rely on this feature and downgrade, you don't want to suddenly pay
> attention to the sendemail.* config again.
> 
> I think that's an acceptable failure mode, and better than the syntax error,
> because that's exactly what we have now, so this is similar to e.g. the
> conditional include directive which was understood but just copmletely
> ignored by older versions.
> 
> So we're OK with getting different config between versions with new
> releases, but at all costs don't want to have new releases introduce
> constructs that older gits error out on, and let's say hypothetically we
> supported something like:
> 
> [user "-email"]
> [user]
> email = ...
> 
> Even `git config -l` on older version won't show that "user.-email", and it's
> better if older tools at least understand the syntax, even though they don't
> pick up on the magic.

I may be missing something but..

Another completely different approach to "undoing" configurations is to 
consider using git for this. Have a repository set up for your ~ directory, 
ignoring content other than .*, so you would ignore any sub-repositories at 
this level. Then manage your configuration as any other repo.

For configurations that are not user-specific, use in-repository configurations 
instead of system and global, so your undo is also handled by git. However, you 
can version control your /etc directory as well. We do that to detect changes 
(as a practical example, we have /etc/.git with some bits ignored but critical 
things involving rc.d, and the system git configurations are managed content in 
that repository. This implies our Ops team has to use git to make changes - a 
good thing - and 'git status' and 'git log' tells me immediately if someone 
changed something.

Undo becomes a git operation in both situations.

This may be complete OT, but I thought it might help

Cheers,
Randall

-- Brief whoami:
 NonStop developer since approximately 2112884442
 UNIX developer since approximately 421664400
-- In my real life, I talk too much.





Re: How to undo previously set configuration?

2018-04-05 Thread Ævar Arnfjörð Bjarmason

On Thu, Apr 05 2018, Olaf Hering wrote:

> Am Thu, 05 Apr 2018 10:42:15 +0200
> schrieb Ævar Arnfjörð Bjarmason :
>
>> I've been meaning to work on this but haven't figured out a good syntax for 
>> it (suggestions welcome!).
>
> Just prefix the knob with something like "no." or "-" or whatever to indicate 
> that it never happened.

Those wouldn't work, respectively, because:

 a) For 'no.' there would be no way to override three-level keys,
because prefixing such a key with "no" would introduce a 4th nesting
level, which would be incompatible with existing config parsers.

 b) Similarly a prefix of - dies in git now. Unless I misunderstand
you. I'm assuming you mean something like:

[user]
# This is an error
-email

Although I see we don't ignore or error out on:

[user "-email"]
foo=bar

   But that's back to problem a), and also looks ugly since you need
   something like the extra foo=bar so we'll pay attention to it.

Also it's important that the syntax leaves room for item #1 that I
mentioned,

I.e. not just ignore stuff like user.email, but being able to specify
where from you'd like to ignore that. Sometimes your local sysadmin is
overzealous with his /etc/gitconfig settings and you'd like to
quarantine just that, but pay attention to everything else in
~/.gitconfig, or similarly in /some/repo/.git/config ignore your usual
custom sendemail.* from ~/.gitconfig but not /etc/gitconfig, so the
semantics can't just be "clear existing".

But of course, you might say that it *should* be a syntax error because
if you rely on this feature and downgrade, you don't want to suddenly
pay attention to the sendemail.* config again.

I think that's an acceptable failure mode, and better than the syntax
error, because that's exactly what we have now, so this is similar to
e.g. the conditional include directive which was understood but just
copmletely ignored by older versions.

So we're OK with getting different config between versions with new
releases, but at all costs don't want to have new releases introduce
constructs that older gits error out on, and let's say hypothetically we
supported something like:

[user "-email"]
[user]
email = ...

Even `git config -l` on older version won't show that "user.-email", and
it's better if older tools at least understand the syntax, even though
they don't pick up on the magic.


Re: How to undo previously set configuration?

2018-04-05 Thread Olaf Hering
Am Thu, 05 Apr 2018 10:42:15 +0200
schrieb Ævar Arnfjörð Bjarmason :

> I've been meaning to work on this but haven't figured out a good syntax for 
> it (suggestions welcome!).

Just prefix the knob with something like "no." or "-" or whatever to indicate 
that it never happened.

Olaf


pgpfQgIWe0Wl7.pgp
Description: Digitale Signatur von OpenPGP