Re: [RFC/PATCHv3 4/5] gitweb: Starting work on a man page for /etc/gitweb.conf (WIP)

Hi,

Drew Northup wrote:

On Mon, 2011-06-06 at 17:12 -0500, Jonathan Nieder wrote:

quoted

Micronit: a single line like

	SYNOPSIS
	--------
	$GITWEBDIR/gitweb_config.perl, /etc/gitweb.conf

might fit better with the pattern established by gitattributes(5) and
its kin.

I thought that's how I had originally put it.

I take that as a "yes, it would work better".

To save others the trouble of digging previous rounds up for
comparison:

 - the original: http://thread.gmane.org/gmane.comp.version-control.git/173422
 - v2: http://thread.gmane.org/gmane.comp.version-control.git/174700/focus=174701
 - v3: http://thread.gmane.org/gmane.comp.version-control.git/175140/focus=175145

The original had one line but it didn't mention
$GITWEBDIR/gitweb_config.perl.  So we are slowly converging.

[...]

quoted

quoted

+The configuration file is used to override the default settings that
+were built into gitweb at the time 'gitweb.cgi' script was generated.
+
+While one could just alter the configuration settings in the gitweb
+CGI itself, those changes would be lost upon upgrade.

What if I upgrade using RCS "merge"? :)  So maybe:

	s/one could just alter/one can alter/
	s/would be lost/could be lost/

to clarify that (1) editing the CGI script is allowed and fine but (2)
it might be a pain to keep those changes.

The point of this was originally if you were a mere mortal then upgrades
of the gitweb CGI would flush all of the settings you just spent a week
getting right down the drain. That's the point of the stronger language.
It's meant to be a solid "If you do it that way you can expect it to
hurt" message.

Sorry, I suppose I was unclear.  All I meant is that as a naïve
reader, I found myself losing trust with "those changes would be lost
upon upgrade".  Oddly enough, by contrast something like:

	While it would be possible to change the gitweb CGI directly
	instead, DON'T DO THAT!  Your changes would be lost on upgrade!

would have worked fine for me, since there are enough cues to know I
am reading hyperbole rather than an explanation of mechanism.

Anyway, it was just how I read it, and it's likely my proposed change
would have made it worse in some other way.

quoted

quoted

Configuration
+settings might also be placed into a file in the same directory as the
+CGI script with the default name 'gitweb_config.perl' -- allowing
+one to have multiple gitweb instances with different configurations by
+the use of symlinks.

Might also in addition to what?  

This comment is nonsensical, please clarify it.

I found the sentence starting with "Configuration settings" hard to
understand because the "also" didn't have an obvious antecedent.
Therefore I proposed some alternative phrasing:

quoted

Continuing the thought from before:

	So gitweb allows settings to be placed in a separate file named
	'gitweb_config.perl' in the same directory as the CGI script.
	This also allows one to set up multiple gitweb instances with
	different configurations by using symlinks to a common gitweb.cgi
	script.

How is this any different from what I wrote?

I'm starting to sense annoyance.  Please feel free to ignore my
comments if you think they're unhelpful; they're meant as a gift, not
demands.

Anyway, the difference is in phrasing.  Saying "So" is meant to make
the relationship to the previous sentence clearer: we are explaining
the purpose of the configuration file, by contrasting it with editing
the CGI script directly.  The next sentence is a separate thought, so
I thought it might be clearer to put it in a separate sentence.

quoted

quoted

+DISCUSSION
+----------

First, as a general thought, by the time I get this far I start
thinking, "so where's the list of possible settings?".  But there are
certainly some more basic pieces of information to cover before then
(like the global configuration file), so I think this part is in the
right place.  It just could benefit from being a little shorter.

I used resolv.conf(5) as the model here. I realized that without a
little introduction most of the settings and the way they are applied
didn't make a heck of a lot of sense if you've never seen them before.

Yes, I think the general structure is good.

quoted

quoted

+The location of system-wide gitweb configuration file is defined at compile
+time using the configuration value `GITWEB_CONFIG_SYSTEM` and defaults to
+'/etc/gitweb.conf'.  The name of the per-instance configuration file is
+defined in gitweb by `GITWEB_CONFIG`, and defaults to 'gitweb_config.perl'
+(relative path means located in the same directory gitweb is installed).

Maybe:

	In addition to the per-instance configuration file, there can
	be a system-wide configuration file to act as a fallback when
	the per-instance configuration file does not exist.

	The system-wide configuration file is named /etc/gitweb.conf
	by default.  Filenames for the system-wide and per-instance
	configuration variables can be overridden at compile time and
	run time; see the FILES section for details.

This is the manpage for the system wide configuration file. If you'd
like to scrap this effort in favor of something else please speak up.

Huh?

quoted

quoted

+*NOTE:* If per-instance configuration file ('gitweb_config.perl') exists,
+then system-wide configuration file ('/etc/gitweb.conf') is _not used at
+all_!!!

Over the top. :)  I think the best way to document this is to contrast
it with /etc/gitweb-common.conf once the latter exists.

If we were to change gitweb to handle configuration files like the rest
of git (and in fact like most things we deal with daily, where settings
are overridden one by one) then this section becomes moot. Until or
unless that becomes the case it is important to loudly make note of it. 

Does using three exclamation marks and italics make it clearer?

Previously there was no manpage documenting this at all, so I think
explaining it is already a big and good step.  If this needs to be
so prominent that people just skimming through will notice it, then
I'd suggest putting it in the DESCRIPTION section higher up.  But I'm
not the one writing this; others are free to disagree.

quoted

quoted

+The syntax of the configuration files is that of Perl, as these files are

[...]

quoted

quoted

+page for more information.

Duplicates DESCRIPTION.

Perhaps it does, but not everybody is a Perl coder. It was that way in
the text I started from and I saw little point in changing it.

No, the paragraph is actually repeated from the DESCRIPTION section.

[...]

quoted

quoted

+The default configuration with no configuration file at all may work
+perfectly well for some installations.  Still, a configuration file is
+useful for customizing or tweaking the behavior of gitweb in many ways, and
+some optional features will not be present unless explicitly enabled using
+the configurable `%features` variable (see also "Configuring gitweb
+features" section below).

I suppose this is based on the text

	The most notable thing that is not configurable at compile time are the
	optional features, stored in the '%features' variable.

I'd suggest removing the paragraph.

The motivation for this paragraph was actually the innumerable tutorials
out there which tell people to mangle the CGI directly because:
(1) There didn't seem to be a standard documented configuration file
(2) There wasn't an appropriate place to shove %features settings that
the author could find.
(3) It really needed to be said that you might not actually have to mess
with anything.

Apparently you think that clearing these misconceptions up is a useless
exercise, or at least it sounds a lot like it.

What?  I wrote exactly what I was thinking, which was that I thought this
paragraph was based on the text I quoted from gitweb/INSTALL and that
contrasting the run-time and compile-time configuration as that
paragraph did didn't seem very important for the installed manpage
(since the reader might not have been involved in the installation at
all).

If the point was actually to say "Contrary to popular belief, you can
set %features in the configuration file instead of hard-coding it in
the CGI script itself", why not say that directly?

[...]

I cannot keep up with this pace of patching right now.

No problem.  For what it's worth, I skipped a couple of rounds of
reading, too; I suppose they came quickly.  Does that mean you don't
want to be cc-ed on later incarnations of the patch, or would you like
to stay notified?

It seems my review was not very useful; sorry about that.  Thanks for
some clarifications.

Ciao,
Jonathan