From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:11
Hi.
A week ago or so when I presented my GITCVS::config patch I mentioned
that we should better implement most of it in Git.pm. I would like to
do so but get a bit of input first on how to implement it.
Targets:
1) We should offer to parse the config only once since that is
a huge performance gain if the caller wants to use several
values from it.
2) The parsing should be complete and safe.
3) If at all possible, we should not have to implement a
complete parser in Perl, since that is just needless
code to maintain.
Possible Solutions:
1) Call git-config.
Pro: Easy to implement
Contra: Violates at least target 2. Neither git-config --get nor
git-config --list offer a complete and safe view on the config
file. Just try including = in a subsection name (--list) or newlines in
a value (both) to see what I mean.
2) Extend git-config to give a machine parsable output and then
proceed with solution 1
Pro: Still reasonably easy to implement (?). Would benefit
other scripts, too.
Contra: Neither the fastest nor the most flexible
solution.
3) Try to use the C code from config.c directly.
Pro: Probably the fastest solution due to avoiding the
forks.
Contra: Probably a bit more complex (any XS experts here?),
both to implement and to maintain.
4) Implement an own git config parser in Perl
Pro: Might be actually easier than 3 and faster than 2
Contra: See target 3
I would go for solution 2. Any reason to prefer one of the
others (or one I didn't even think of)?
Gruesse,
--
Frank Lichtenheld [off-list ref]
www: http://www.djpig.de/
From: Petr Baudis <hidden> Date: 2016-06-15 22:43:11
Hi,
On Mon, May 21, 2007 at 12:59:54AM CEST, Frank Lichtenheld wrote:
Possible Solutions:
1) Call git-config.
Pro: Easy to implement
Contra: Violates at least target 2. Neither git-config --get nor
git-config --list offer a complete and safe view on the config
file. Just try including = in a subsection name (--list) or newlines in
a value (both) to see what I mean.
2) Extend git-config to give a machine parsable output and then
proceed with solution 1
Pro: Still reasonably easy to implement (?). Would benefit
other scripts, too.
Contra: Neither the fastest nor the most flexible
solution.
Yes, this might be fine for you. The argument for 4 (implementing our
own in Perl) is that we would like it to be _real_ fast for gitweb
(especially the summary page needs to look at each repository). But it
would be a question of a benchmark to look really how much would calling
git-config slow us down. So as at least a proof-of-concept and initial
implementation I think this is more than fine, and we can proceed to
implement our own parser only when it's clearly needed.
3) Try to use the C code from config.c directly.
Pro: Probably the fastest solution due to avoiding the
forks.
Contra: Probably a bit more complex (any XS experts here?),
both to implement and to maintain.
There was various trouble with XS in the past, and I think the general
feel was that we want to get back to using XS again sometime, but only
when Git will be reasonably libified (to support multiple repositories
at once, etc.).
4) Implement an own git config parser in Perl
Pro: Might be actually easier than 3 and faster than 2
Contra: See target 3
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
-- Samuel Beckett
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:11
This option will enclose key names in quotes (") if they
contain a subsection and then escape " and \. It will also
escape line breaks in values. Together this should produce
an easily parsable output.
Affects --list and --get-*
Signed-off-by: Frank Lichtenheld <redacted>
---
builtin-config.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 83 insertions(+), 9 deletions(-)
Will add asciidoc documentation and test cases if people think that this is
a good idea.
I'm writing C about once a year, so I really don't mind being told if it's
crap ;)
@@ -12,14 +12,73 @@ static int use_key_regexp;staticintdo_all;staticintdo_not_match;staticintseen;+staticintquoted;staticenum{T_RAW,T_INT,T_BOOL}type=T_RAW;+staticchar*quote_key(constchar*key_)+{+char*pos1,*pos2;+char*key_quot;++pos1=strchr(key_,'.');+pos2=strrchr(key_,'.');+if(pos1!=pos2){/* has subsection */+char*key;+key=xmalloc(strlen(key_)*2+1);+key_quot=key;+*key++='"';+while(*key_){+if(*key_=='"'||*key_=='\\')+*key++='\\';+*key++=*key_++;+}+*key++='"';+*key=0;+}else{+key_quot=xstrdup(key_);+}+returnkey_quot;+}++staticchar*quote_value(constchar*value_)+{+char*val_quot,*val;+val=xmalloc(strlen(value_)*2+1);+val_quot=val;++while(*value_){+if(*value_=='\n'){+*val++='\\';+*val++='n';+value_+=2;+}else+*val++=*value_++;+}+*val=0;++returnval_quot;+}+staticintshow_all_config(constchar*key_,constchar*value_){-if(value_)-printf("%s=%s\n",key_,value_);-else-printf("%s\n",key_);+if(!quoted){+if(value_)+printf("%s=%s\n",key_,value_);+else+printf("%s\n",key_);+}else{+char*key_quot=quote_key(key_);++if(value_){+char*val_quot=quote_value(value_);+printf("%s=%s\n",key_quot,val_quot);+free(val_quot);+}else+printf("%s\n",key_quot);++free(key_quot);+}+return0;}
@@ -38,16 +97,26 @@ static int show_config(const char* key_, const char* value_)regexec(regexp,(value_?value_:""),0,NULL,0)))return0;-if(show_keys)-printf("%s ",key_);+if(show_keys){+if(quoted){+char*key=quote_key(key_);+printf("%s ",key);+free(key);+}else+printf("%s ",key_);+}if(seen&&!do_all)dup_error=1;if(type==T_INT)sprintf(value,"%d",git_config_int(key_,value_?value_:""));elseif(type==T_BOOL)vptr=git_config_bool(key_,value_)?"true":"false";-else-vptr=value_?value_:"";+else{+if(quoted)+vptr=value_?quote_value(value_):"";+else+vptr=value_?value_:"";+}seen++;if(dup_error){error("More than one value for the key %s: %s",
From: Jan Hudec <hidden> Date: 2016-06-15 22:43:12
On Mon, May 21, 2007 at 19:46:59 +0200, Frank Lichtenheld wrote:
This option will enclose key names in quotes (") if they
contain a subsection and then escape " and \. It will also
escape line breaks in values. Together this should produce
an easily parsable output.
That will lead to either eval (which runs perl parser and probably won't win
anything) or regexps (which is not big win over parsing the .ini directly
with them) on the perl side. IMHO only thing that would actually be faster is
NUL-separated entries.
Either:
KEY <NUL> VALUE <NUL>
or:
KEY <TAB> VALUE <NUL>
I am not sure whether there can be multi-valued entries. If so, than there
are three options:
1. Simply repeated key/value pairs:
KEY <NUL> VALUE1 <NUL> KEY <NUL> VALUE2 <NUL>.
KEY <TAB> VALUE1 <NUL> KEY <TAB> VALUE2 <NUL> resp.
2. Key/count/values:
KEY <NUL> 1 <NUL> VALUE <NUL>
KEY <NUL> 2 <NUL> VALUE1 <NUL> VALUE2 <NUL>
(there's probably no benefit for the tab-nul format, because the first
value must be terminated with NUL)
3. Empty-entry terminated:
KEY <NUL> VALUE <NUL> <NUL>
KEY <NUL> VALUE1 <NUL> VALUE2 <NUL> <NUL>
(again no point in terminating the KEY with tab)
The advantage of such format is, that it can be parsed with:
local $/ = "\0";
while(<INPUT>) {
$hash{$_} = <INPUT>;
}
and slight variations for the other variants. It should be similarly easy
from python and C. Shell won't like it, though.
Note: In both bash and zsh, read -d '' line reads NUL-terminated "lines".
However, dash does not have -d option to read :-(.
--
Jan 'Bulb' Hudec [off-list ref]
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:12
On Mon, May 21, 2007 at 09:54:23PM +0200, Jan Hudec wrote:
On Mon, May 21, 2007 at 19:46:59 +0200, Frank Lichtenheld wrote:
quoted
This option will enclose key names in quotes (") if they
contain a subsection and then escape " and \. It will also
escape line breaks in values. Together this should produce
an easily parsable output.
That will lead to either eval (which runs perl parser and probably won't win
anything) or regexps (which is not big win over parsing the .ini directly
with them) on the perl side. IMHO only thing that would actually be faster is
NUL-separated entries.
Either:
KEY <NUL> VALUE <NUL>
or:
KEY <TAB> VALUE <NUL>
Both subsection names and values can contain <TAB> characters, so the
latter isn't possible.
Gruesse,
--
Frank Lichtenheld [off-list ref]
www: http://www.djpig.de/
From: Jakub Narebski <hidden> Date: 2016-06-15 22:43:12
Frank Lichtenheld wrote:
On Mon, May 21, 2007 at 09:54:23PM +0200, Jan Hudec wrote:
quoted
On Mon, May 21, 2007 at 19:46:59 +0200, Frank Lichtenheld wrote:
quoted
This option will enclose key names in quotes (") if they
contain a subsection and then escape " and \. It will also
escape line breaks in values. Together this should produce
an easily parsable output.
That will lead to either eval (which runs perl parser and probably won't win
anything) or regexps (which is not big win over parsing the .ini directly
with them) on the perl side. IMHO only thing that would actually be faster is
NUL-separated entries.
quoted
Either:
KEY <NUL> VALUE <NUL>
or:
KEY <TAB> VALUE <NUL>
Both subsection names and values can contain <TAB> characters, so the
latter isn't possible.
But neither subsection names (even [section "subsection"] style) not key
names cannot contain newline <LF>. I.e.
KEY <LF> VALUE <NUL>
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:17
Use \n as delimiter between key and value and \0 as
delimiter after each key/value pair. This should be
easily parsable output.
Signed-off-by: Frank Lichtenheld <redacted>
---
builtin-config.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
On Tue, May 22, 2007 at 12:37:57AM +0200, Jakub Narebski wrote:
> Frank Lichtenheld wrote:
> > On Mon, May 21, 2007 at 09:54:23PM +0200, Jan Hudec wrote:
> >> KEY <TAB> VALUE <NUL>
> > Both subsection names and values can contain <TAB> characters, so the
> > latter isn't possible.
> But neither subsection names (even [section "subsection"] style) not key
> names cannot contain newline <LF>. I.e.
> KEY <LF> VALUE <NUL>
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:17
Hi,
On Mon, 18 Jun 2007, Frank Lichtenheld wrote:
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
AFAIK --list was meant as a replacement to git-var --list. Thus, it had to
behave exactly the same.
As for the FIXME: If you have a config like this:
[core]
Some = where
over
the = core.rainbow
git-config -z would output something like this:
core.some\0where\0core.over\0core.the\0core.rainbow\0
Right?
As you can see, it is quite hard for a parser to find out what is key, and
what is value. That FIXME is _exactly_ about this dilemma.
IIRC I stated once that -z should output a value of "true" for these
cases, since they only make sense as booleans. But AFAIR nothing
conclusive came out of that thread.
Ciao,
Dscho
From: Jakub Narebski <hidden> Date: 2016-06-15 22:43:17
Johannes Schindelin wrote:
As for the FIXME: [...]
Please read original message carefully:
quoted
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
I think the FIXME is for --get-regexp. And was added by the patch.
If you have a config like this:
[core]
Some = where
over
the = core.rainbow
git-config -z would output something like this:
core.some\0where\0core.over\0core.the\0core.rainbow\0
Right?
False. Delim is different from term. You would get
core.some\nwhere\0core.over\0core.the\ncore.rainbow\0
As you can see, it is quite hard for a parser to find out what is
key, and what is value.
local $/ = "\0";
while (my $line = <$fd>) {
chomp $line;
my ($key, $value) = split(/\n/, $line, 2);
push @{$config{$key}}, $value if defined $key;
}
--
Jakub Narebski
Poland
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:17
On Tue, Jun 19, 2007 at 01:55:24AM +0100, Johannes Schindelin wrote:
Hi,
On Mon, 18 Jun 2007, Frank Lichtenheld wrote:
quoted
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
AFAIK --list was meant as a replacement to git-var --list. Thus, it had to
behave exactly the same.
As for the FIXME: If you have a config like this:
[core]
Some = where
over
the = core.rainbow
git-config -z would output something like this:
core.some\0where\0core.over\0core.the\0core.rainbow\0
Right?
No. At least not with my patch. As you noted that would be
incredibly stupid and worthless. Instead we output something like
core.some\nwhere\0core.over\0core.the\ncore.rainbow\0
So you just can split on \0 and then split on \n. If there is
no \n between two \0, you have a key without value.
Gruesse,
--
Frank Lichtenheld [off-list ref]
www: http://www.djpig.de/
From: Jakub Narebski <hidden> Date: 2016-06-15 22:43:17
On Mon, 18 Jun 2007, Frank Lichtenheld wrote:
Use \n as delimiter between key and value and \0 as
delimiter after each key/value pair. This should be
easily parsable output.
Signed-off-by: Frank Lichtenheld <redacted>
---
builtin-config.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
No documentation. But this is an RFC.
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
I don't know, but at least two scripts use --get-regexp, namely
git-remote and git-submodule. So we would have to be careful about
changing that.
I would be enough to add the following to your patch:
quoted hunk
@@ -12,14 +12,16 @@ static int use_key_regexp; static int do_all; static int do_not_match; static int seen;+static char delim = '=';+static char term = '\n';
+static char key_delim = ' ';
static enum { T_RAW, T_INT, T_BOOL } type = T_RAW;
[...]
quoted hunk
@@ -39,6 +41,7 @@ static int show_config(const char* key_, const char* value_) return 0; if (show_keys)+ /* FIXME: not useful with --null */
@@ -155,6 +158,10 @@ int cmd_config(int argc, const char **argv, const char *prefix) } else if (!strcmp(argv[1], "--system")) setenv("GIT_CONFIG", ETC_GITCONFIG, 1);+ else if (!strcmp(argv[1], "--null") || !strcmp(argv[1], "-z")) {+ term = '\0';+ delim = '\n';
+ key_delim = '\n';
+ }
else if (!strcmp(argv[1], "--rename-section")) {
int ret;
if (argc != 4)
By the way, I have tried to use git-config --null to redo config
file parsing in gitweb, so one git-config call would be needed for
all the config. I have noticed that --bool option description does
not describe the observed behavior fully. For example it returns
'true' not only for '1', but for any integer != 0, including 0xdeadbeef.
By the way, the error message when key value _cannot_ be converted to
the boolean is somewhat misleading:
$ GIT_CONFIG=conftest git config --bool bool.key7
fatal: bad config value for 'bool.key7' in conftest
--
Jakub Narebski
Poland
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:17
On Fri, Jun 22, 2007 at 01:56:00AM +0200, Jakub Narebski wrote:
On Mon, 18 Jun 2007, Frank Lichtenheld wrote:
quoted
Note the FIXME. Does anyone remember the reason why --get-regexp
and --list use different output format?
I don't know, but at least two scripts use --get-regexp, namely
git-remote and git-submodule. So we would have to be careful about
changing that.
I would be enough to add the following to your patch:
Yeah, I found this a but ugly though so I left it out of the first
patch in case someone had a better idea. Will include that in the
second version (which will also include documentation).
By the way, I have tried to use git-config --null to redo config
file parsing in gitweb, so one git-config call would be needed for
all the config. I have noticed that --bool option description does
not describe the observed behavior fully. For example it returns
'true' not only for '1', but for any integer != 0, including 0xdeadbeef.
Yeah, the description of --bool is very incomplete. Note that empty
values are false, and keys without values are true. I think this all
are valid choices, but they are indeed not documented.
Gruesse.
--
Frank Lichtenheld [off-list ref]
www: http://www.djpig.de/
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:18
The asciidoc documentation of the --get-regexp option was
incomplete. Add some missing pieces:
- List the option in SYNOPSIS
- Mention that key names are printed
Signed-off-by: Frank Lichtenheld <redacted>
---
Documentation/git-config.txt | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
@@ -73,6 +74,7 @@ OPTIONS --get-regexp:: Like --get-all, but interprets the name as a regular expression.+ Also outputs the key names. --global:: For writing options: write to global ~/.gitconfig file rather than
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:18
Print no space after the name of a key without value.
Otherwise keys without values are printed exactly the
same as keys with empty values.
Signed-off-by: Frank Lichtenheld <redacted>
---
builtin-config.c | 8 ++++++--
t/t1300-repo-config.sh | 6 ++++++
2 files changed, 12 insertions(+), 2 deletions(-)
I hope that nobody depends on this specific behaviour.
Backwards compatibilty would be a pain here, since the --null
patch would get really complicated
@@ -283,6 +283,12 @@ EOF test_expect_success'get variable with no value'\'git-config --get novalue.variable ^$'+echonovalue.variable>expect++test_expect_success'get-regexp variable with no value'\+'git-config--get-regexpnovalue>output&&+cmpoutputexpect'+ git-config>output2>&1 test_expect_success'no arguments, but no crash'\
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:18
Use \n as delimiter between key and value and \0 as
delimiter after each key/value pair. This should be
easily parsable output.
Signed-off-by: Frank Lichtenheld <redacted>
---
Documentation/git-config.txt | 18 +++++++++++++-----
builtin-config.c | 20 ++++++++++++++------
t/t1300-repo-config.sh | 32 ++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 11 deletions(-)
This time with documentation and test cases.
@@ -9,17 +9,17 @@ git-config - Get and set repository or global options SYNOPSIS -------- [verse]-'git-config' [--system | --global] name [value [value_regex]]+'git-config' [--system | --global] [-z|--null] name [value [value_regex]] 'git-config' [--system | --global] --add name value 'git-config' [--system | --global] --replace-all name [value [value_regex]]-'git-config' [--system | --global] [type] --get name [value_regex]-'git-config' [--system | --global] [type] --get-all name [value_regex]-'git-config' [--system | --global] [type] --get-regexp name_regex [value_regex]+'git-config' [--system | --global] [type] [-z|--null] --get name [value_regex]+'git-config' [--system | --global] [type] [-z|--null] --get-all name [value_regex]+'git-config' [--system | --global] [type] [-z|--null] --get-regexp name_regex [value_regex] 'git-config' [--system | --global] --unset name [value_regex] 'git-config' [--system | --global] --unset-all name [value_regex] 'git-config' [--system | --global] --rename-section old_name new_name 'git-config' [--system | --global] --remove-section name-'git-config' [--system | --global] -l | --list+'git-config' [--system | --global] [-z|--null] -l | --list DESCRIPTION -----------
@@ -118,6 +118,14 @@ See also <<FILES>>. in the config file will cause the value to be multiplied by 1024, 1048576, or 1073741824 prior to output.+-z, --null::+ For all options that output values and/or keys, always+ end values with with the null character (instead of a+ newline). Use newline instead as a delimiter between+ key and value. This allows for secure parsing of the+ output without getting confused e.g. by values that+ contain line breaks. + [[FILES]] FILES
@@ -519,4 +519,36 @@ git config --list > result test_expect_success'value continued on next line''cmp result expect'+cat>.git/config<<\EOF+[section"sub=section"]+val1=foo=bar+val2=foo\nbar+val3=\n\n+val4=+val5+EOF++cat>expect<<\EOF+Key:section.sub=section.val1+Value:foo=bar+Key:section.sub=section.val2+Value:foo+bar+Key:section.sub=section.val3+Value:+++Key:section.sub=section.val4+Value:+Key:section.sub=section.val5+EOF++gitconfig--null--list|perl-0ne'chop;($key,$value)=split(/\n/,$_,2);print "Key: $key\n";print "Value: $value\n" if defined($value)'>result++test_expect_success'--null --list''cmp result expect'++gitconfig--null--get-regexp'val[0-9]'|perl-0ne'chop;($key,$value)=split(/\n/,$_,2);print "Key: $key\n";print "Value: $value\n" if defined($value)'>result++test_expect_success'--null --get-regexp''cmp result expect'+ test_done
From: Frank Lichtenheld <hidden> Date: 2016-06-15 22:43:18
On Tue, Jun 26, 2007 at 01:29:46AM +0200, Jakub Narebski wrote:
Frank Lichtenheld wrote:
quoted
else
- printf("%s", key_);
+ printf("%s",key_);
}
That is a mistake, I think?
Indeed. I remember I saw and fixed that before sending. Must've
been a halluzination ;)
Gruesse,
--
Frank Lichtenheld [off-list ref]
www: http://www.djpig.de/