From: "J.H." <warthog9@kernel.org>
To: Jakub Narebski <jnareb@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Drew Northup <drew.northup@maine.edu>,
Petr Baudis <pasky@suse.cz>
Subject: Re: [PATCH 0/2] gitweb: Improve handling of configuration files
Date: Wed, 25 May 2011 11:38:57 -0700 [thread overview]
Message-ID: <4DDD4CC1.3070103@kernel.org> (raw)
In-Reply-To: <1306341328-11108-1-git-send-email-jnareb@gmail.com>
On 05/25/2011 09:35 AM, Jakub Narebski wrote:
> This two-in-one series is response to Junio's concerns about being
> backwards-incompatibile and different ways of solving "system-wide
> policy" problem.
I realize I'm jumping on this mid way through, but I did read back
through the history.
Start with Patch 1, it looks fine and I support the change. Makes it a
lot easier to include config files.
> Patches 2/2 version A and 2/2 version B are mutually exclusive.
> Junio, you would have to choose which one to include.
To speak to Junio's concerns seem to boil down to a general concern that
something in /etc/gitweb.conf may interfere with unintended interaction
in the local repository, and has suggested that we do an explicit
include to solve the problem.
I would argue against this, for a couple of reasons:
1) Git itself treats /etc/git.conf in this exact manor, it reads in
stuff from /etc/git.conf and then local repos can change or override
things as needed. In fact this is quite beneficial, because it gives
site admins a simple and easy way to give an automatic hint to a repo
about things the admin would like.
Case in point turning on things like updateserverinfo, or automatic packing.
The fact that an admin doesn't need to do anything for this change to be
effective in most places is actually the expected result of the
sysadmin. The systems administrator is saying "hey we need to have this
on everywhere and instead of getting 100 people to all change their
configs, we make one change and the hint is automatically everywhere".
>From a kernel.org perspective, that's what I'd expect. I'm not keen on
having to go and either add some include line to each config file for
gitweb config (which feels like a kludge) just to centralize config
elements.
2) Trying to get people to do includes in their config files, or to
completely ignore the system wide config isn't a sustainable practice.
It would be akin to asking that every repository on kernel.org add
something to their config, which each individual person has to go and
add on their own.
That's akin to asking 400 people to make a change to 900+ repositories,
and then to remember to make that change for every new repo that gets
created.
Now I don't have 900 instances of gitweb running, but I am up to at
least 3 independent instances, and I can think of a 4th that is in the
works. Having a system-wide config that gives the basics is appealing
to me.
Now that I've said all that, here's what I propose.
We go back to the way Jakub proposed this originally, where it reads in
the system wide config, and then overwrites things from the local
config. This really is more in line with what people, particularly
system wide sysadmins, will expect.
To alleviate the concerns of people who thing the systemwide may cause
problems I propose, instead of an explicit include, we do an explicit
exclude for the system wide config. We have a new config directive that
basically says throw out the systemwide config, take me back to defaults
and let me override from there. This means needing to either store a
copy of the defaults on start-up, or like with gitweb-caching the
defaults are kept in a separate file and can just be re-read, and it
means that the local config would need to be read twice.
This means that if the individual wants to ignore the hints the systems
administrator is giving in system wide config, then they are making an
explicit choice to do so, either because they know better or something
like git-instaweb where it generally doesn't trust it. Either way, it's
the exceptions who are making the choice, not the general case.
I'll admit this might even convince me that a single configuration
structure for the entire script is better than individual variable names.
- John 'Warthog9' Hawley
next prev parent reply other threads:[~2011-05-25 18:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-25 16:35 [PATCH 0/2] gitweb: Improve handling of configuration files Jakub Narebski
2011-05-25 16:35 ` [PATCH 1/2] gitweb: Refactor reading and parsing config file into read_config_file Jakub Narebski
2011-05-25 19:36 ` Junio C Hamano
2011-05-25 16:35 ` [PATCHv1 2/2 (version A)] gitweb: Mention read_config_file in gitweb/README Jakub Narebski
2011-05-25 16:35 ` [PATCHv3 2/2 (version B)] gitweb: Use /etc/gitweb.conf even if gitweb_conf.perl exist Jakub Narebski
2011-05-25 18:54 ` Junio C Hamano
2011-05-26 14:58 ` [PATCH/RFC 2/2 (version C)] gitweb: Introduce common system-wide settings for convenience Jakub Narebski
2011-05-25 18:38 ` J.H. [this message]
2011-05-25 19:34 ` [PATCH 0/2] gitweb: Improve handling of configuration files Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DDD4CC1.3070103@kernel.org \
--to=warthog9@kernel.org \
--cc=drew.northup@maine.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jnareb@gmail.com \
--cc=pasky@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).