{"thread":{"id":"8091","subject":"[PATCH] t9400: Use the repository config and nothing else.","startedAt":"2007-05-11T23:35:18Z","lastAt":"2007-05-13T19:16:45Z","messageCount":11,"participants":["Junio Hamano","Frank Lichtenheld","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"41871","messageId":"7v8xbvj5mx.fsf@arte.twinsun.com","threadId":"8091","inReplyTo":null,"subject":"[PATCH] t9400: Use the repository config and nothing else.","fromName":"Junio Hamano","fromEmail":"junio@twinsun.com","sentAt":"2007-05-11T23:35:18Z","receivedAt":"2007-05-11T23:35:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"git-cvsserver has a bug in its configuration file output parser\nthat makes it choke if the configuration has these:\n\n [diff]\n color = auto\n [diff.color]\n whitespace = blue reverse\n\nThis needs to be fixed, but thanks to that bug, a separate bug\nin t9400 test script was discovered. The test discarded\nGIT_CONFIG instead of pointing at the proper one to be used in\nthe exoprted repository. This allowed user's .gitconfig and (if\nexists) systemwide /etc/gitconfig to affect the outcome of the\ntest, which is a big no-no.\n\nThe patch fixes the problem in the test. Fixing the\ngit-cvsserver's configuration parser is left as an exercise to\nmotivated volunteers ;-)\n\nSigned-off-by: Junio C Hamano \n---\n\n * I think the fix to git-cvsserver should be straightforward.\n Instead of parsing two or three level names in a hierarchical\n hash around ll.187-192 for *all* config items, it should just\n parse gitcvs related ones only, as that are the only ones\n that the script cares about.\n\n t/t9400-git-cvsserver-server.sh | 15 ++++++++-------\n 1 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex f137b30..b136a97 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -26,6 +26,7 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n unset GIT_DIR GIT_CONFIG\n WORKDIR=$(pwd)\n SERVERDIR=$(pwd)/gitcvs.git\n+git_config=$SERVERDIR/config\n CVSROOT=\":fork:$SERVERDIR\"\n CVSWORK=$(pwd)/cvswork\n CVS_SERVER=git-cvsserver\n@@ -43,7 +44,7 @@ echo >empty &&\n # note that cvs doesn't accept absolute pathnames\n # as argument to co -d\n test_expect_success 'basic checkout' \\\n- 'cvs -Q co -d cvswork master &&\n+ 'GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master &&\n test \"$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))\" = \"empty/1.1/\"'\n \n test_expect_success 'cvs update (create new file)' \\\n@@ -52,7 +53,7 @@ test_expect_success 'cvs update (create new file)' \\\n git commit -q -m \"Add testfile1\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update &&\n test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.1/\" &&\n diff -q testfile1 ../testfile1'\n \n@@ -63,7 +64,7 @@ test_expect_success 'cvs update (update existing file)' \\\n git commit -q -m \"Append to testfile1\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update &&\n test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.2/\" &&\n diff -q testfile1 ../testfile1'\n \n@@ -76,7 +77,7 @@ test_expect_failure \"cvs update w/o -d doesn't create subdir (TODO)\" \\\n git commit -q -m \"Single Subdirectory\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update &&\n test ! -d test'\n \n cd \"$WORKDIR\"\n@@ -89,7 +90,7 @@ test_expect_success 'cvs update (subdirectories)' \\\n git commit -q -m \"deep sub directory structure\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update -d &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update -d &&\n (for dir in A A/B A/B/C A/D E; do\n filename=\"file_in_$(echo $dir|sed -e \"s#/# #g\")\" &&\n if test \"$(echo $(grep -v ^D $dir/CVS/Entries|cut -d/ -f2,3,5))\" = \"$filename/1.1/\" &&\n@@ -107,7 +108,7 @@ test_expect_success 'cvs update (delete file)' \\\n git commit -q -m \"Remove testfile1\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update &&\n test -z \"$(grep testfile1 CVS/Entries)\" &&\n test ! -f testfile1'\n \n@@ -118,7 +119,7 @@ test_expect_success 'cvs update (re-add deleted file)' \\\n git commit -q -m \"Re-Add testfile1\" &&\n git push gitcvs.git >/dev/null &&\n cd cvswork &&\n- cvs -Q update &&\n+ GIT_CONFIG=\"$git_config\" cvs -Q update &&\n test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.4/\" &&\n diff -q testfile1 ../testfile1'\n \n"},{"id":"41936","messageId":"20070512162819.GA7184@planck.djpig.de","threadId":"8091","inReplyTo":"7v8xbvj5mx.fsf@arte.twinsun.com","subject":"Re: [PATCH] t9400: Use the repository config and nothing else.","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-05-12T16:28:20Z","receivedAt":"2007-05-12T16:28:20Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Fri, May 11, 2007 at 04:35:18PM -0700, Junio Hamano wrote:\n> This needs to be fixed, but thanks to that bug, a separate bug\n> in t9400 test script was discovered. The test discarded\n> GIT_CONFIG instead of pointing at the proper one to be used in\n> the exoprted repository. This allowed user's .gitconfig and (if\n> exists) systemwide /etc/gitconfig to affect the outcome of the\n> test, which is a big no-no.\n\nShouldn't you also remove the \"unset GIT_CONFIG\" then?\n\n> @@ -26,6 +26,7 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n> unset GIT_DIR GIT_CONFIG\n> WORKDIR=$(pwd)\n> SERVERDIR=$(pwd)/gitcvs.git\n> +git_config=$SERVERDIR/config\n> CVSROOT=\":fork:$SERVERDIR\"\n> CVSWORK=$(pwd)/cvswork\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"41937","messageId":"7vabwa3swp.fsf@assigned-by-dhcp.cox.net","threadId":"8091","inReplyTo":"20070512162819.GA7184@planck.djpig.de","subject":"Re: [PATCH] t9400: Use the repository config and nothing else.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-12T16:31:34Z","receivedAt":"2007-05-12T16:31:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Lichtenheld writes:\n\n> On Fri, May 11, 2007 at 04:35:18PM -0700, Junio Hamano wrote:\n>> This needs to be fixed, but thanks to that bug, a separate bug\n>> in t9400 test script was discovered. The test discarded\n>> GIT_CONFIG instead of pointing at the proper one to be used in\n>> the exoprted repository. This allowed user's .gitconfig and (if\n>> exists) systemwide /etc/gitconfig to affect the outcome of the\n>> test, which is a big no-no.\n>\n> Shouldn't you also remove the \"unset GIT_CONFIG\" then?\n\nI didn't test the side of the test that works on the git side,\nbut I think you are right.\n"},{"id":"41941","messageId":"7vhcqi2c1g.fsf@assigned-by-dhcp.cox.net","threadId":"8091","inReplyTo":"7vabwa3swp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] t9400: Use the repository config and nothing else.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-12T17:21:15Z","receivedAt":"2007-05-12T17:21:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Frank Lichtenheld writes:\n>\n>> On Fri, May 11, 2007 at 04:35:18PM -0700, Junio Hamano wrote:\n>>> This needs to be fixed, but thanks to that bug, a separate bug\n>>> in t9400 test script was discovered. The test discarded\n>>> GIT_CONFIG instead of pointing at the proper one to be used in\n>>> the exoprted repository. This allowed user's .gitconfig and (if\n>>> exists) systemwide /etc/gitconfig to affect the outcome of the\n>>> test, which is a big no-no.\n>>\n>> Shouldn't you also remove the \"unset GIT_CONFIG\" then?\n>\n> I didn't test the side of the test that works on the git side,\n> but I think you are right.\n\nActually that is not sufficient, as unsetting means using the\nvalue set in test-lib.sh suitable for usual single-repository\ntests.\n\nWhen you prepare gitcvs.enabled config in the cloned gitcvs.git\nrepository, you do not want to have GIT_CONFIG=.git/config in\nthe environment. As you give GIT_DIR to these two commands, not\nhaving GIT_CONFIG would make them do the right thing.\n"},{"id":"41957","messageId":"11789982521112-git-send-email-frank@lichtenheld.de","threadId":"8091","inReplyTo":"7v8xbvj5mx.fsf@arte.twinsun.com","subject":"[PATCH] cvsserver: Complete rewrite of the configuration parser","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-05-12T19:30:52Z","receivedAt":"2007-05-12T19:30:52Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"Move the configuration parsing to a separate GITCVS::config\nmodule. Simplifies using the configuration in the rest of\nthe code.\n\nRestrict parsed configuration variables to\n^gitcvs\\.((ext|pserver)\\.)?\nsince we don't use anything else anyway. This also\nreduces the risk of getting confused with arbitrary\nvariables (especially arbitrary subsection names).\n\nAlso fixes a bug where the config parser got confused\nif a section had a subsection and a variable with the\nsame name.\n\nSigned-off-by: Frank Lichtenheld \n---\n git-cvsserver.perl | 187 ++++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 158 insertions(+), 29 deletions(-)\n\n Maybe a bit overkill if one only wants to solve the problem Junio discovered\n but I believe it's still worthwile.\n\n Has a lot of overlap with perl/Git.pm though...\n\n Not extensively tested but it at least passes the test cases and creates a useful\n log which should take care of the two main code paths (get_gitcvs and\n get_gitcvs_bool).\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 3e7bf5b..e51ffd0 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -174,27 +174,17 @@ sub req_Root\n return 0;\n }\n \n- my @gitvars = `git-config -l`;\n- if ($?) {\n- print \"E problems executing git-config on the server -- this is not a git repository or the PATH is not set correctly.\\n\";\n- print \"E \\n\";\n- print \"error 1 - problem executing git-config\\n\";\n- return 0;\n- }\n- foreach my $line ( @gitvars )\n- {\n- next unless ( $line =~ /^(.*?)\\.(.*?)(?:\\.(.*?))?=(.*)$/ );\n- unless ($3) {\n- $cfg->{$1}{$2} = $4;\n- } else {\n- $cfg->{$1}{$2}{$3} = $4;\n- }\n+ $cfg = GITCVS::config->new();\n+\n+ unless ($cfg) {\n+\tprint \"E problems executing git-config on the server -- \".\n+\t \"this is not a git repository or the PATH is not set correctly.\\n\";\n+\tprint \"E \\n\";\n+\tprint \"error 1 - problem executing git-config\\n\";\n+\treturn 0;\n }\n \n- unless ( ($cfg->{gitcvs}{$state->{method}}{enabled}\n-\t and $cfg->{gitcvs}{$state->{method}}{enabled} =~ /^\\s*(1|true|yes)\\s*$/i)\n-\t or ($cfg->{gitcvs}{enabled}\n-\t and $cfg->{gitcvs}{enabled} =~ /^\\s*(1|true|yes)\\s*$/i) )\n+ unless ( $cfg->get_gitcvs_bool($state->{method},'enabled') )\n {\n print \"E GITCVS emulation needs to be enabled on this repo\\n\";\n print \"E the repo config file needs a [gitcvs] section added, and the parameter 'enabled' set to 1\\n\";\n@@ -203,7 +193,7 @@ sub req_Root\n return 0;\n }\n \n- my $logfile = $cfg->{gitcvs}{$state->{method}}{logfile} || $cfg->{gitcvs}{logfile};\n+ my $logfile = $cfg->get_gitcvs($state->{method},'logfile');\n if ( $logfile )\n {\n $log->setfile($logfile);\n@@ -1967,7 +1957,7 @@ sub kopts_from_path\n \t# what attributes apply to this path.\n \n \t# Until then, take the setting from the config file\n- unless ( defined ( $cfg->{gitcvs}{allbinary} ) and $cfg->{gitcvs}{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i )\n+ unless ( $cfg->get_gitcvs_bool('allbinary') )\n {\n \t\t# Return \"\" to give no special treatment to any path\n \t\treturn \"\";\n@@ -1978,6 +1968,147 @@ sub kopts_from_path\n }\n }\n \n+package GITCVS::config;\n+\n+####\n+#### Copyright 2007 Frank Lichtenheld .\n+####\n+\n+use strict;\n+use warnings;\n+\n+=head1 NAME\n+\n+GITCVS::config -- interface to the git configuration files\n+\n+=head1 DESCRIPTION\n+\n+Parses the output of \"git-config -l\" once and then allows to access that\n+information\n+\n+=head1 METHODS\n+\n+=cut\n+\n+=head2 new\n+\n+Creates a new object and retrieves the config information.\n+If retrieving the configuration fails, returns undef.\n+\n+=cut\n+sub new\n+{\n+ my $class = shift;\n+\n+ my $self = {};\n+\n+ bless $self, $class;\n+\n+ $self->update() or return;\n+\n+ return $self;\n+}\n+\n+=head2 update\n+\n+Update the config information. Is called by new on creation.\n+Currently limits itself to the variables actually used by\n+git-cvsserver since the output of git-config -l is not actually\n+completly maschine-parsable. Multi-valued variables are not\n+supported, the last value found is used.\n+\n+=cut\n+sub update\n+{\n+ my $self = shift;\n+\n+ my @gitvars = `git-config -l`;\n+ return if $?;\n+ foreach my $line ( @gitvars )\n+ {\n+\tnext unless ( $line =~ /^((gitcvs)\\.(?:(ext|pserver)\\.)?([\\w-]+))=(.*)$/ );\n+\t$self->{cfg}{$1} = $5;\n+ }\n+\n+ return $self;\n+}\n+\n+=head2 get\n+\n+Retrieve a configuration value. Give the key as array.\n+\n+=cut\n+sub get {\n+ my $self = shift;\n+ my @key = @_;\n+\n+ unless (($#key == 1)\n+\t || ($#key == 2)) {\n+\treturn;\n+ }\n+\n+ $key[0] = lc $key[0];\n+ $key[-1] = lc $key[-1];\n+\n+ my $key = join('.',@key);\n+ if (exists $self->{cfg}{$key}) {\n+\treturn $self->{cfg}{$key};\n+ }\n+ return;\n+}\n+\n+=head2 get_bool\n+\n+Retrieve a configuration value. Give the key as array.\n+Normalizes the value to either undef, 0, or 1.\n+\n+=cut\n+sub get_bool {\n+ my $self = shift;\n+ my $value = $self->get(@_);\n+\n+ return unless defined($value);\n+ return 1 if $value =~ /^\\s*(1|true|yes)\\s*$/i;\n+ return 0;\n+}\n+\n+=head2 get_gitcvs\n+\n+Like get(), but automatically assumes gitcvs as section.\n+If given two paramters, tries with second one alone\n+if the first query gave no result.\n+\n+=cut\n+sub get_gitcvs {\n+ my $self = shift;\n+ my @key = @_;\n+\n+ my $value = $self->get('gitcvs',@key);\n+ if (!defined($value) && ($#key == 1)) {\n+\t$value = $self->get('gitcvs',$key[1]);\n+ }\n+\n+ return $value;\n+}\n+\n+=head2 get_gitcvs_bool\n+\n+What get_bool is to get that\n+is get_gitcvs_bool to get_gitcvs.\n+\n+=cut\n+sub get_gitcvs_bool {\n+ my $self = shift;\n+ my @key = @_;\n+\n+ my $value = $self->get_bool('gitcvs',@key);\n+ if (!defined($value) && ($#key == 1)) {\n+\t$value = $self->get_bool('gitcvs',$key[1]);\n+ }\n+\n+ return $value;\n+}\n+\n package GITCVS::log;\n \n ####\n@@ -2189,14 +2320,12 @@ sub new\n \n die \"Git repo '$self->{git_path}' doesn't exist\" unless ( -d $self->{git_path} );\n \n- $self->{dbdriver} = $cfg->{gitcvs}{$state->{method}}{dbdriver} ||\n- $cfg->{gitcvs}{dbdriver} || \"SQLite\";\n- $self->{dbname} = $cfg->{gitcvs}{$state->{method}}{dbname} ||\n- $cfg->{gitcvs}{dbname} || \"%Ggitcvs.%m.sqlite\";\n- $self->{dbuser} = $cfg->{gitcvs}{$state->{method}}{dbuser} ||\n- $cfg->{gitcvs}{dbuser} || \"\";\n- $self->{dbpass} = $cfg->{gitcvs}{$state->{method}}{dbpass} ||\n- $cfg->{gitcvs}{dbpass} || \"\";\n+ $self->{dbdriver} = $cfg->get_gitcvs($state->{method},'dbdriver') ||\n+\t\"SQLite\";\n+ $self->{dbname} = $cfg->get_gitcvs($state->{method},'dbname') ||\n+\t\"%Ggitcvs.%m.sqlite\";\n+ $self->{dbuser} = $cfg->get_gitcvs($state->{method},'dbuser') || \"\";\n+ $self->{dbpass} = $cfg->get_gitcvs($state->{method},'dbpass') || \"\";\n my %mapping = ( m => $module,\n a => $state->{method},\n u => getlogin || getpwuid($<) || $<,\n-- \n1.5.1.4\n"},{"id":"41959","messageId":"20070512193608.GB7184@planck.djpig.de","threadId":"8091","inReplyTo":"7vhcqi2c1g.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] t9400: Use the repository config and nothing else.","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-05-12T19:36:08Z","receivedAt":"2007-05-12T19:36:08Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Sat, May 12, 2007 at 10:21:15AM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> When you prepare gitcvs.enabled config in the cloned gitcvs.git\n> repository, you do not want to have GIT_CONFIG=.git/config in\n> the environment. As you give GIT_DIR to these two commands, not\n> having GIT_CONFIG would make them do the right thing.\n\nYeah, which was the reason I unset it in the first place. But\nif your concern is not to use other config files it should still\nset GIT_CONFIG explicetly for these cases and leave it to the\ndefault for all calls inside the non-bare repository, right?\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"41961","messageId":"7v4pmhyfre.fsf@assigned-by-dhcp.cox.net","threadId":"8091","inReplyTo":"11789982521112-git-send-email-frank@lichtenheld.de","subject":"Re: [PATCH] cvsserver: Complete rewrite of the configuration parser","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-12T19:59:49Z","receivedAt":"2007-05-12T19:59:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Lichtenheld writes:\n\n> Maybe a bit overkill if one only wants to solve the problem Junio discovered\n> but I believe it's still worthwile.\n>\n> Has a lot of overlap with perl/Git.pm though...\n>\n> Not extensively tested but it at least passes the test cases and creates a useful\n> log which should take care of the two main code paths (get_gitcvs and\n> get_gitcvs_bool).\n\nI agree that the general direction should be to do something\nlike this in perl/Git.pm (Pasky CC'ed). As there are some\nthings that current Git.pm config interface does not offer an\neasy access to what you would want to do, we need to enumerate\nwhat you need, decide if they are of general interest and design\nwhat to put in Git.pm and what to implement in GITCVS::config as\na special-purpose feature.\n\nperl/Git.pm currently gives us only this: \n\n - grab all values for a named variable, in an array;\n\n - return canonicalized value for a named boolean variable;\n\nGITCVS::config wants to read *everything* from config and\nreturns a 'config' instance you can:\n\n - enumerate keys (you do not have this, but it is easy to add);\n\n - retrieve a value for a key (either one- or two-level);\n\n - retrieve a canonicalized bool value for a key (either one- or\n two-level);\n\n - treat a request for \"gitcvs.method.option\" variable to fall\n back on \"gitcvs.option\" if the former is not given;\n\n - the same for boolean variant.\n\nI think the best abstraction is to have the \"read everything\"\ninterface in perl/Git.pm side, make the current Git::config()\nand Git::config_bool() interface to use it (without issuing\nextra 'git config --get-all'). I am not sure it is common for\nGit.pm users to want the behaviour of \"section.method.option\"\nfalling back to \"section.option\", but if it is common enough, it\nprobably is a good idea to have:\n\n\tsub config_fallback {\n \tmy ($self, $section, $specific, $var) = @_;\n my $cfg = $self->config();\n\n if (exists $cfg{\"$section.$specific.$var\"}) {\n \treturn $cfg{\"$section.$specific.$var\"};\n\t\t}\n if (exists $cfg{\"$section.$var\"}) {\n \treturn $cfg{\"$section.$var\"};\n\t\t}\n return undef;\n\t}\n\non perl/Git.pm side.\n\nBut all of this is post 1.5.2 material; we would want to have a\nminimal fixup on 'master' before 1.5.2, independent of this\nrewrite.\n"},{"id":"41971","messageId":"20070512213153.GC7184@planck.djpig.de","threadId":"8091","inReplyTo":"7v4pmhyfre.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsserver: Complete rewrite of the configuration parser","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-05-12T21:31:53Z","receivedAt":"2007-05-12T21:31:53Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Sat, May 12, 2007 at 12:59:49PM -0700, Junio C Hamano wrote:\n> But all of this is post 1.5.2 material; we would want to have a\n> minimal fixup on 'master' before 1.5.2, independent of this\n> rewrite.\n\nFair enough. So far I see three very minimal solutions, but I can't\ndecide which one is the least ugly:\n\n(For all we can begin by limiting the used variables to\n^gitcvs.((ext|pserver).)? )\n\n1) Drop variables named gitcvs.ext and gitcvs.pserver manually\n2) Use the complete variable name as key to the hash instead of\n using a hash of hashes of hashes\n { \"diff.color => \"auto\",\n \"diff.color.whitespace\" => \"blue reverse\" }\n3) Make the second level always a hash, instead of using a string\n directly, so that Junio's example would look like this\n { diff => { color => { value => \"auto\",\n \t\t\t whitespace => \"blue reverse\" } } }\n\n\nOpinions?\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"41977","messageId":"7v3b21wtlf.fsf@assigned-by-dhcp.cox.net","threadId":"8091","inReplyTo":"20070512213153.GC7184@planck.djpig.de","subject":"Re: [PATCH] cvsserver: Complete rewrite of the configuration parser","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-12T22:43:56Z","receivedAt":"2007-05-12T22:43:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Lichtenheld writes:\n\n> On Sat, May 12, 2007 at 12:59:49PM -0700, Junio C Hamano wrote:\n>> But all of this is post 1.5.2 material; we would want to have a\n>> minimal fixup on 'master' before 1.5.2, independent of this\n>> rewrite.\n>\n> Fair enough. So far I see three very minimal solutions, but I can't\n> decide which one is the least ugly:\n>\n> (For all we can begin by limiting the used variables to\n> ^gitcvs.((ext|pserver).)? )\n\nThat sounds sensible. And ignore anything that do not match.\n\n> 1) Drop variables named gitcvs.ext and gitcvs.pserver manually\n\nI do not see any need for this; gitcvs.ext or gitcvs.pserver as\nvariables do not exist, at least right now. The breakage was\npurely that the old parser tried to parse things it does not\neven know about (e.g. diff.color) without knowing the rules\nthere.\n\n> 2) Use the complete variable name as key to the hash instead of\n> using a hash of hashes of hashes\n> { \"diff.color => \"auto\",\n> \"diff.color.whitespace\" => \"blue reverse\" }\n\nNo need for this nor the next one either. You understand only\ngitcvs.