{"thread":{"id":"27457","subject":"[PATCH 1/2] gitweb: Refactor reading and parsing config file into read_config_file","startedAt":"2011-05-25T16:35:25Z","lastAt":"2011-05-26T14:58:17Z","messageCount":9,"participants":["Jakub Narebski","J.H.","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"168678","messageId":"1306341328-11108-1-git-send-email-jnareb@gmail.com","threadId":"27457","inReplyTo":null,"subject":"[PATCH 0/2] gitweb: Improve handling of configuration files","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-25T16:35:25Z","receivedAt":"2011-05-25T16:35:25Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This two-in-one series is response to Junio's concerns about being\nbackwards-incompatibile and different ways of solving \"system-wide\npolicy\" problem.\n\nPatches 2/2 version A and 2/2 version B are mutually exclusive.\nJunio, you would have to choose which one to include.\n\nDrew, is the solution proposed in version A (making it easy to include\nsystem-wide config in per-instance config) acceptable solution?\n\nTable of contents:\n~~~~~~~~~~~~~~~~~~\n [PATCH 1/2] gitweb: Refactor reading and parsing config file into\n [PATCHv1 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README\n [PATCHv3 2/2 (version B)] gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist\n\nShortlog:\n~~~~~~~~~\nJakub Narebski (2):\n  gitweb: Refactor reading and parsing config file into read_config_file\n  gitweb: Mention read_config_file in gitweb/README\n\nor\n\nJakub Narebski (2):\n  gitweb: Refactor reading and parsing config file into read_config_file\n  gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist\n\n\nDiffstat:\n~~~~~~~~~\n gitweb/README      |   10 ++++++++++\n gitweb/gitweb.perl |   28 ++++++++++++++++++++--------\n 2 files changed, 30 insertions(+), 8 deletions(-)\n\nor\n\n gitweb/INSTALL     |    8 +++++---\n gitweb/README      |    2 +-\n gitweb/gitweb.perl |   29 +++++++++++++++++++++--------\n 3 files changed, 27 insertions(+), 12 deletions(-)\n\n-- \n1.7.5\n"},{"id":"168676","messageId":"1306341328-11108-2-git-send-email-jnareb@gmail.com","threadId":"27457","inReplyTo":"1306341328-11108-1-git-send-email-jnareb@gmail.com","subject":"[PATCH 1/2] gitweb: Refactor reading and parsing config file into read_config_file","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-25T16:35:26Z","receivedAt":"2011-05-25T16:35:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Beside being obvious reduction of duplicated code, this is enables us\nto easily call site-wide config file in per-installation config file.\n\nThe actual update to documentation is left for next commit, because of\npossible exclusive alternative (possible other next commit) of always\nreading system-wide config file and relying on per-instalation config\nfile overriding system-wide defaults.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI think it makes code more clear, both version A xor B ;-)\n\n gitweb/gitweb.perl |   28 ++++++++++++++++++++--------\n 1 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e4ede13..ce92d67 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -635,18 +635,30 @@ sub filter_snapshot_fmts {\n # if it is true then gitweb config would be run for each request.\n our $per_request_config = 1;\n \n+# read and parse gitweb config file given by its parameter.\n+# returns true on success, false on recoverable error, allowing\n+# to chain this subroutine, using first file that exists.\n+# dies on errors during parsing config file, as it is unrecoverable.\n+sub read_config_file {\n+\tmy $filename = shift;\n+\treturn unless defined $filename;\n+\t# die if there are errors parsing config file\n+\tif (-e $filename) {\n+\t\tdo $filename;\n+\t\tdie $@ if $@;\n+\t\treturn 1;\n+\t}\n+\treturn;\n+}\n+\n our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM);\n sub evaluate_gitweb_config {\n \tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n \tour $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n-\t# die if there are errors parsing config file\n-\tif (-e $GITWEB_CONFIG) {\n-\t\tdo $GITWEB_CONFIG;\n-\t\tdie $@ if $@;\n-\t} elsif (-e $GITWEB_CONFIG_SYSTEM) {\n-\t\tdo $GITWEB_CONFIG_SYSTEM;\n-\t\tdie $@ if $@;\n-\t}\n+\n+\t# use first config file that exists\n+\tread_config_file($GITWEB_CONFIG) or\n+\tread_config_file($GITWEB_CONFIG_SYSTEM);\n }\n \n # Get loadavg of system, to compare against $maxload.\n-- \n1.7.5\n"},{"id":"168677","messageId":"1306341328-11108-3-git-send-email-jnareb@gmail.com","threadId":"27457","inReplyTo":"1306341328-11108-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv1 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-25T16:35:27Z","receivedAt":"2011-05-25T16:35:27Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Using\n\n  read_config_file($GITWEB_CONFIG_SYSTEM);\n\nat the very beginning of per-instance GITWEB_CONFIG file is quite\nsimilar to e.g. using\n\n  if [ -f /etc/bashrc ]; then\n    . /etc/bashrc\n  fi\n\nin ~/.bashrc to read system-wide defaults.\n\nThis solution is backwards compatibile, but it requires to explicitly\nrequest system-wide defaults... which otherwise are _not used at all_\nwhen per-instance config file exists.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis is alternate solution to \"system-wide policy\" problem, the other\nway to make things convenient.  It was (soft of) suggested by Junio\nin\n\n  Re: [PATCHv2] gitweb: Use GITWEB_CONFIG_SYSTEM even if GITWEB_CONFIG does exist\n  http://thread.gmane.org/gmane.comp.version-control.git/173603/focus=173815\n\nWhile usually user's (here per-instance) configuration file overrides\nsystem-wide configuration file settings (or in other words first\nobtained value from user's and system-wide configuration file, in that\norder, is used), like e.g. the case for ssh_config,... it is not\nuniversal, as described in commit message.\n\n\nThis commit should be thought as exclusive to\n\n  [PATCH 2/2 (version B)] gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist\n\nWe have to choose one or the other.  Note that this other version\nincludes also change to gitweb/INSTALL...\n\n gitweb/README |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex a92bde7..ea10c91 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -127,6 +127,16 @@ Runtime gitweb configuration\n You can adjust gitweb behaviour using the file specified in `GITWEB_CONFIG`\n (defaults to 'gitweb_config.perl' in the same directory as the CGI), and\n as a fallback `GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf).\n+You can read defaults in system-wide GITWEB_CONFIG_SYSTEM from GITWEB_CONFIG\n+by adding\n+\n+  read_config_file($GITWEB_CONFIG_SYSTEM);\n+\n+at very beginning of per-instance GITWEB_CONFIG file.  In this case\n+settings in said per-instance file will override settings from\n+system-wide configuration file.  Note that read_config_file checks\n+itself that the $GITWEB_CONFIG_SYSTEM file exists.\n+\n The most notable thing that is not configurable at compile time are the\n optional features, stored in the '%features' variable.\n \n-- \n1.7.5\n"},{"id":"168679","messageId":"1306341328-11108-4-git-send-email-jnareb@gmail.com","threadId":"27457","inReplyTo":"1306341328-11108-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv3 2/2 (version B)] gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-25T16:35:28Z","receivedAt":"2011-05-25T16:35:28Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"gitweb obtains configuration from the following sources:\n\n  1. per-instance configuration file (default: gitweb_conf.perl)\n  2. system-wide configuration file (default: /etc/gitweb.conf)\n\nIf per-instance configuration file exists, then system-wide\nconfiguration is _not used at all_.  This is quite untypical and\nsuprising behavior.\n\nThis commit changes gitweb behavior so that configuration in\nper-instance configuration file can _override_ settings from\nsystem-wide configuration file.\n\nNote that strictly speaking it is backwards incompatibile change.\nCheck your resulting gitweb configuration, please.\n\nSuggested-by: Drew Northup <drew.northup@maine.edu>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis commit is _*functionally*_ unchanged from previous version\n\n  [PATCHv2] gitweb: Use GITWEB_CONFIG_SYSTEM even if GITWEB_CONFIG does exist\n  http://thread.gmane.org/gmane.comp.version-control.git/173603/focus=173720\n\nbut it uses read_config_file() subroutine introduced in first patch.\nThis IMVHO makes gitweb code more clear.\n\n\nThis commit should be thought as exclusive to\n\n  [PATCH 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README\n\nWe have to choose one or the other.\n\n\n gitweb/INSTALL     |    8 +++++---\n gitweb/README      |    2 +-\n gitweb/gitweb.perl |    5 +++--\n 3 files changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/INSTALL b/gitweb/INSTALL\nindex 4964a67..0584919 100644\n--- a/gitweb/INSTALL\n+++ b/gitweb/INSTALL\n@@ -98,15 +98,17 @@ Gitweb config file\n See also \"Runtime gitweb configuration\" section in README file\n for gitweb (in gitweb/README).\n \n-- You can configure gitweb further using the gitweb configuration file;\n+- You can configure gitweb further using the per-instance gitweb configuration file;\n   by default this is a file named gitweb_config.perl in the same place as\n   gitweb.cgi script. You can control the default place for the config file\n   using the GITWEB_CONFIG build configuration variable, and you can set it\n-  using the GITWEB_CONFIG environment variable. If this file does not\n-  exist, gitweb looks for a system-wide configuration file, normally\n+  using the GITWEB_CONFIG environment variable.\n+  gitweb also looks for a system-wide configuration file, normally\n   /etc/gitweb.conf. You can change the default using the\n   GITWEB_CONFIG_SYSTEM build configuration variable, and override it\n   through the GITWEB_CONFIG_SYSTEM environment variable.\n+  Settings from per-instance configuration file override those from\n+  system-wide configuration file.\n \n - The gitweb config file is a fragment of perl code. You can set variables\n   using \"our $variable = value\"; text from \"#\" character until the end\ndiff --git a/gitweb/README b/gitweb/README\nindex a92bde7..334f13e 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -126,7 +126,7 @@ Runtime gitweb configuration\n \n You can adjust gitweb behaviour using the file specified in `GITWEB_CONFIG`\n (defaults to 'gitweb_config.perl' in the same directory as the CGI), and\n-as a fallback `GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf).\n+`GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf), in that order.\n The most notable thing that is not configurable at compile time are the\n optional features, stored in the '%features' variable.\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex ce92d67..e4b0932 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -656,9 +656,10 @@ sub evaluate_gitweb_config {\n \tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n \tour $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n \n-\t# use first config file that exists\n-\tread_config_file($GITWEB_CONFIG) or\n+\t# let settings in second override ones in first\n \tread_config_file($GITWEB_CONFIG_SYSTEM);\n+\tread_config_file($GITWEB_CONFIG)\n+\t\tif ($GITWEB_CONFIG ne $GITWEB_CONFIG_SYSTEM);\n }\n \n # Get loadavg of system, to compare against $maxload.\n-- \n1.7.5\n"},{"id":"168699","messageId":"4DDD4CC1.3070103@kernel.org","threadId":"27457","inReplyTo":"1306341328-11108-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH 0/2] gitweb: Improve handling of configuration files","fromName":"J.H.","fromEmail":"warthog9@kernel.org","sentAt":"2011-05-25T18:38:57Z","receivedAt":"2011-05-25T18:38:57Z","isPatch":true,"sender":{"key":"warthog9@kernel.org","avatar":"https://avatars.githubusercontent.com/u/2334704?v=4"},"body":"On 05/25/2011 09:35 AM, Jakub Narebski wrote:\n> This two-in-one series is response to Junio's concerns about being\n> backwards-incompatibile and different ways of solving \"system-wide\n> policy\" problem.\n\nI realize I'm jumping on this mid way through, but I did read back\nthrough the history.\n\nStart with Patch 1, it looks fine and I support the change.  Makes it a\nlot easier to include config files.\n\n> Patches 2/2 version A and 2/2 version B are mutually exclusive.\n> Junio, you would have to choose which one to include.\n\nTo speak to Junio's concerns seem to boil down to a general concern that\nsomething in /etc/gitweb.conf may interfere with unintended interaction\nin the local repository, and has suggested that we do an explicit\ninclude to solve the problem.\n\nI would argue against this, for a couple of reasons:\n\n1) Git itself treats /etc/git.conf in this exact manor, it reads in\nstuff from /etc/git.conf and then local repos can change or override\nthings as needed.  In fact this is quite beneficial, because it gives\nsite admins a simple and easy way to give an automatic hint to a repo\nabout things the admin would like.\n\nCase in point turning on things like updateserverinfo, or automatic packing.\n\nThe fact that an admin doesn't need to do anything for this change to be\neffective in most places is actually the expected result of the\nsysadmin.  The systems administrator is saying \"hey we need to have this\non everywhere and instead of getting 100 people to all change their\nconfigs, we make one change and the hint is automatically everywhere\".\n\n>From a kernel.org perspective, that's what I'd expect.  I'm not keen on\nhaving to go and either add some include line to each config file for\ngitweb config (which feels like a kludge) just to centralize config\nelements.\n\n2) Trying to get people to do includes in their config files, or to\ncompletely ignore the system wide config isn't a sustainable practice.\nIt would be akin to asking that every repository on kernel.org add\nsomething to their config, which each individual person has to go and\nadd on their own.\n\nThat's akin to asking 400 people to make a change to 900+ repositories,\nand then to remember to make that change for every new repo that gets\ncreated.\n\nNow I don't have 900 instances of gitweb running, but I am up to at\nleast 3 independent instances, and I can think of a 4th that is in the\nworks.  Having a system-wide config that gives the basics is appealing\nto me.\n\nNow that I've said all that, here's what I propose.\n\nWe go back to the way Jakub proposed this originally, where it reads in\nthe system wide config, and then overwrites things from the local\nconfig.  This really is more in line with what people, particularly\nsystem wide sysadmins, will expect.\n\nTo alleviate the concerns of people who thing the systemwide may cause\nproblems I propose, instead of an explicit include, we do an explicit\nexclude for the system wide config.  We have a new config directive that\nbasically says throw out the systemwide config, take me back to defaults\nand let me override from there.  This means needing to either store a\ncopy of the defaults on start-up, or like with gitweb-caching the\ndefaults are kept in a separate file and can just be re-read, and it\nmeans that the local config would need to be read twice.\n\nThis means that if the individual wants to ignore the hints the systems\nadministrator is giving in system wide config, then they are making an\nexplicit choice to do so, either because they know better or something\nlike git-instaweb where it generally doesn't trust it.  Either way, it's\nthe exceptions who are making the choice, not the general case.\n\nI'll admit this might even convince me that a single configuration\nstructure for the entire script is better than individual variable names.\n\n- John 'Warthog9' Hawley\n"},{"id":"168701","messageId":"7v8vtu4nmy.fsf@alter.siamese.dyndns.org","threadId":"27457","inReplyTo":"1306341328-11108-4-git-send-email-jnareb@gmail.com","subject":"Re: [PATCHv3 2/2 (version B)] gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-25T18:54:13Z","receivedAt":"2011-05-25T18:54:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> This commit should be thought as exclusive to\n>\n>   [PATCH 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README\n>\n> We have to choose one or the other.\n\nWho says?  We do not have to.  Why not do this instead?\n\n\tour $GITWEB_CONFIG_COMMON = $ENV{'GITWEB_CONFIG_COMMON'} ||\n\t\t\"/etc/gitweb-common.conf\";\n\n\t# common system-wide settings for convenience\n        read_config_file($GITWEB_COMMON);\n\n        # as always, use the per-instance GITWEB_CONFIG if exists,\n        # otherwise use GITWEB_SYSTEM_CONFIG\n        read_config_file($GITWEB_CONFIG) ||\n        \tread_config_file($GITWEB_CONFIG_SYSTEM);\n\nThere is no risk of hurting any existing installations, and people who do\nhave things that needs to be shared do not have to go around and update\nall the per-instance configuration files.\n\n diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index ce92d67..e4b0932 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -656,9 +656,10 @@ sub evaluate_gitweb_config {\n>  \tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n>  \tour $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n>  \n> -\t# use first config file that exists\n> -\tread_config_file($GITWEB_CONFIG) or\n> +\t# let settings in second override ones in first\n>  \tread_config_file($GITWEB_CONFIG_SYSTEM);\n> +\tread_config_file($GITWEB_CONFIG)\n> +\t\tif ($GITWEB_CONFIG ne $GITWEB_CONFIG_SYSTEM);\n>  }\n>  \n>  # Get loadavg of system, to compare against $maxload.\n"},{"id":"168705","messageId":"7vr57m377i.fsf@alter.siamese.dyndns.org","threadId":"27457","inReplyTo":"4DDD4CC1.3070103@kernel.org","subject":"Re: [PATCH 0/2] gitweb: Improve handling of configuration files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-25T19:34:25Z","receivedAt":"2011-05-25T19:34:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"J.H.\" <warthog9@kernel.org> writes:\n\n> I would argue against this, for a couple of reasons:\n>\n> 1) Git itself treats /etc/git.conf in this exact manor,...\n> Case in point turning on things like updateserverinfo, or automatic packing.\n\nIf we read both from the beginning of time it would have been more\nconvenient, and I agree with that 100%. So this is not a valid reason to\n\"argue against this\".\n\n> 2) Trying to get people to do includes in their config files, or to\n> completely ignore the system wide config isn't a sustainable practice.\n\nI also agree with this.\n\nIn any case, I do not think we have to choose between Jakub's two patches.\n"},{"id":"168706","messageId":"7vmxia373w.fsf@alter.siamese.dyndns.org","threadId":"27457","inReplyTo":"1306341328-11108-2-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH 1/2] gitweb: Refactor reading and parsing config file into read_config_file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-25T19:36:35Z","receivedAt":"2011-05-25T19:36:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Beside being obvious reduction of duplicated code, this is enables us\n> to easily call site-wide config file in per-installation config file.\n>\n> The actual update to documentation is left for next commit, because of\n> possible exclusive alternative (possible other next commit) of always\n> reading system-wide config file and relying on per-instalation config\n> file overriding system-wide defaults.\n>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n\nAs John acked this one and it looks obviously the right thing to do to me,\nI'll apply this one directly to \"master\".\n\nThanks.\n"},{"id":"168771","messageId":"201105261658.20140.jnareb@gmail.com","threadId":"27457","inReplyTo":"7v8vtu4nmy.fsf@alter.siamese.dyndns.org","subject":"[PATCH/RFC 2/2 (version C)] gitweb: Introduce common system-wide settings for convenience","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-26T14:58:17Z","receivedAt":"2011-05-26T14:58:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Because of backward compatibility we cannot change gitweb to always\nuse /etc/gitweb.conf (i.e. even if gitweb_config.perl exists).  For\ncommon system-wide settings we therefore need new configuration\nfile: /etc/gitweb-common.conf.\n\n\nLong description:\n\ngitweb currently obtains configuration from the following sources:\n\n  1. per-instance configuration file (default: gitweb_conf.perl)\n  2. system-wide configuration file (default: /etc/gitweb.conf)\n\nIf per-instance configuration file exists, then system-wide\nconfiguration is _not used at all_.  This is quite untypical and\nsuprising behavior.\n\nMoreover it is different from way git itself treats /etc/git.conf.  It\nreads in stuff from /etc/git.conf and then local repos can change or\noverride things as needed.  In fact this is quite beneficial, because\nit gives site admins a simple and easy way to give an automatic hint\nto a repo about things the admin would like.\n\nOn the other hand changing current behavior may lead to the situation,\nwhere something in /etc/gitweb.conf may interfere with unintended\ninteraction in the local repository.  One solution would be to\n_require_ to do explicit include; with read_config_file() it is now\neasy, as described in gitweb/README (description introduced in this\ncommit).\n\nBut as J.H. noticed we cannot ask people to modify their per-instance\ngitweb config file to include system-wide settings, nor we can require\nthem to do this.\n\nTherefore, as proposed by Junio, for gitweb to have centralized config\nelements while retaining backwards compatibility, introduce separate\ncommon system-wide configuration file, by default /etc/gitweb-common.conf\n\nNoticed-by: Drew Northup <drew.northup@maine.edu>\nHelped-by: John 'Warthog9' Hawley <warthog9@kernel.org>\nInspired-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nOn Wed, 25 May 2011, Junio C Hamano <gitster@pobox.com> wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > This commit should be thought as exclusive to\n> >\n> >   [PATCH 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README\n> >\n> > We have to choose one or the other.\n> \n> Who says?  We do not have to.  Why not do this instead?\n> \n> \tour $GITWEB_CONFIG_COMMON = $ENV{'GITWEB_CONFIG_COMMON'} ||\n> \t\t\"/etc/gitweb-common.conf\";\n> \n> \t# common system-wide settings for convenience\n>         read_config_file($GITWEB_COMMON);\n> \n>         # as always, use the per-instance GITWEB_CONFIG if exists,\n>         # otherwise use GITWEB_SYSTEM_CONFIG\n>         read_config_file($GITWEB_CONFIG) ||\n>         \tread_config_file($GITWEB_CONFIG_SYSTEM);\n> \n> There is no risk of hurting any existing installations, and people who do\n> have things that needs to be shared do not have to go around and update\n> all the per-instance configuration files.\n\nHere it is, version C.\n\nIt is currently marked as RFC for the following (minor) reasons:\n\n1. In gitweb.perl we now have\n\n  +\t# Protect against duplications of file names, to not read config twice.\n  +\t# Only one of $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM is used, so\n  +\t# there possibility of duplication of filename doesn't matter.\n  +\t$GITWEB_CONFIG = \"\"        if ($GITWEB_CONFIG eq $GITWEB_CONFIG_COMMON);\n  +\t$GITWEB_CONFIG_SYSTEM = \"\" if ($GITWEB_CONFIG_SYSTEM eq $GITWEB_CONFIG_COMMON);\n\nto protect against double inclusion of configuration files.  It is\nnecessary?  Should we protect against such user errors?  Note that in\nmost cases second include wouldn't matter anyway...\n\n\n2. To use first of per-instance and system wide configuration files we\nnow have in gitweb.perl\n\n  \tread_config_file($GITWEB_CONFIG) and return;\n   \tread_config_file($GITWEB_CONFIG_SYSTEM);\n\nIs it more readable than\n\n  \tread_config_file($GITWEB_CONFIG) or\n   \tread_config_file($GITWEB_CONFIG_SYSTEM);\n\nor correctly indented\n\n  \tread_config_file($GITWEB_CONFIG) or\n   \t\tread_config_file($GITWEB_CONFIG_SYSTEM);\n\nor even\n\n  \tread_config_file($GITWEB_CONFIG)\n\t\tor\n   \t\t\tread_config_file($GITWEB_CONFIG_SYSTEM);\n\n\n3. In includes changes to documentation, but does not include new\ntests.  Should it?\n\n gitweb/INSTALL     |   13 ++++++++++++-\n gitweb/Makefile    |    2 ++\n gitweb/README      |   27 ++++++++++++++++++++++++---\n gitweb/gitweb.perl |   18 +++++++++++++++---\n 4 files changed, 53 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/INSTALL b/gitweb/INSTALL\nindex 4964a67..a29ad0c 100644\n--- a/gitweb/INSTALL\n+++ b/gitweb/INSTALL\n@@ -98,7 +98,7 @@ Gitweb config file\n See also \"Runtime gitweb configuration\" section in README file\n for gitweb (in gitweb/README).\n \n-- You can configure gitweb further using the gitweb configuration file;\n+- You can configure gitweb further using the per-instance gitweb configuration file;\n   by default this is a file named gitweb_config.perl in the same place as\n   gitweb.cgi script. You can control the default place for the config file\n   using the GITWEB_CONFIG build configuration variable, and you can set it\n@@ -108,6 +108,17 @@ for gitweb (in gitweb/README).\n   GITWEB_CONFIG_SYSTEM build configuration variable, and override it\n   through the GITWEB_CONFIG_SYSTEM environment variable.\n \n+  Note that if per-instance configuration file exists, then system-wide\n+  configuration is _not used at all_.  This is quite untypical and suprising\n+  behavior.  On the other hand changing current behavior would break backwards\n+  compatibility and can lead to unexpected changes in gitweb behavior.\n+  Therefore gitweb also looks for common system-wide configuration file,\n+  normally /etc/gitweb-common.conf (set during build time using build time\n+  configuration variable GITWEB_CONFIG_COMMON, set it at runtime using\n+  environment variable with the same name).  Settings from per-instance or\n+  system-wide configuration file override those from common system-wide\n+  configuration file.\n+\n - The gitweb config file is a fragment of perl code. You can set variables\n   using \"our $variable = value\"; text from \"#\" character until the end\n   of a line is ignored. See perlsyn(1) for details.\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 5d20515..1c85b5f 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -20,6 +20,7 @@ INSTALL ?= install\n # default configuration for gitweb\n GITWEB_CONFIG = gitweb_config.perl\n GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n+GITWEB_CONFIG_COMMON = /etc/gitweb-common.conf\n GITWEB_HOME_LINK_STR = projects\n GITWEB_SITENAME =\n GITWEB_PROJECTROOT = /pub/git\n@@ -129,6 +130,7 @@ GITWEB_REPLACE = \\\n \t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\n \t-e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n \t-e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n+\t-e 's|++GITWEB_CONFIG_COMMON++|$(GITWEB_CONFIG_COMMON)|g' \\\n \t-e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n \t-e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n \t-e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\ndiff --git a/gitweb/README b/gitweb/README\nindex a92bde7..8743471 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -124,9 +124,30 @@ You can specify the following configuration variables when building GIT:\n Runtime gitweb configuration\n ----------------------------\n \n-You can adjust gitweb behaviour using the file specified in `GITWEB_CONFIG`\n-(defaults to 'gitweb_config.perl' in the same directory as the CGI), and\n-as a fallback `GITWEB_CONFIG_SYSTEM` (defaults to /etc/gitweb.conf).\n+Gitweb obtains configuration data from the following sources in the\n+following order:\n+\n+1. built-in values (some set during build stage),\n+2. common system-wide configuration file (`GITWEB_CONFIG_COMMON`,\n+   defaults to '/etc/gitweb-common.conf'),\n+3. either per-instance configuration file (`GITWEB_CONFIG`, defaults to\n+   'gitweb_config.perl' in the same directory as the installed gitweb),\n+   or if it does not exists then system-wide configuration file\n+   (`GITWEB_CONFIG_SYSTEM`, defaults to '/etc/gitweb.conf').\n+\n+Values obtained in later configuration files override values obtained earlier\n+in above sequence.\n+\n+You can read defaults in system-wide GITWEB_CONFIG_SYSTEM from GITWEB_CONFIG\n+by adding\n+\n+  read_config_file($GITWEB_CONFIG_SYSTEM);\n+\n+at very beginning of per-instance GITWEB_CONFIG file.  In this case\n+settings in said per-instance file will override settings from\n+system-wide configuration file.  Note that read_config_file checks\n+itself that the $GITWEB_CONFIG_SYSTEM file exists.\n+\n The most notable thing that is not configurable at compile time are the\n optional features, stored in the '%features' variable.\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex ce92d67..25a2618 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -651,13 +651,25 @@ sub read_config_file {\n \treturn;\n }\n \n-our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM);\n+our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);\n sub evaluate_gitweb_config {\n \tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n \tour $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n+\tour $GITWEB_CONFIG_COMMON = $ENV{'GITWEB_CONFIG_COMMON'} || \"++GITWEB_CONFIG_COMMON++\";\n \n-\t# use first config file that exists\n-\tread_config_file($GITWEB_CONFIG) or\n+\t# Protect against duplications of file names, to not read config twice.\n+\t# Only one of $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM is used, so\n+\t# there possibility of duplication of filename doesn't matter.\n+\t$GITWEB_CONFIG = \"\"        if ($GITWEB_CONFIG eq $GITWEB_CONFIG_COMMON);\n+\t$GITWEB_CONFIG_SYSTEM = \"\" if ($GITWEB_CONFIG_SYSTEM eq $GITWEB_CONFIG_COMMON);\n+\n+\t# Common system-wide settings for convenience.\n+\t# Those settings can be ovverriden by GITWEB_CONFIG or GITWEB_CONFIG_SYSTEM.\n+\tread_config_file($GITWEB_CONFIG_COMMON);\n+\n+\t# Use first config file that exists.  This means use the per-instance\n+\t# GITWEB_CONFIG if exists, otherwise use GITWEB_SYSTEM_CONFIG.\n+\tread_config_file($GITWEB_CONFIG) and return;\n \tread_config_file($GITWEB_CONFIG_SYSTEM);\n }\n \n-- \n1.7.5\n"}]}