the new controlchan (+gpgverify)

[Russ Allbery]

Marco d'Itri <md at Linux.IT> writes:

> A first draft release is available from
> http://www.linux.it/~md/software/cc.tgz

> Please comment.

Thanks very much!

A couple of functionality comments:

 * Left over from the original controlchan is the code to ignore control
   messages that also contain a Supersedes header (the s/// in the main
   processing loop).  Several hierarchies, such as biz.*, are pretty set
   in their intention to use Supersedes for checkgroups messages; I think
   we should either drop this check or skip it for checkgroups messages.

 * We can dump some of the compatibility cruft that was in controlchan for
   supporting versions of INN <2.3, since this version will be distributed
   with newer versions of INN (unless you were planning on also
   distributing this separately).  Skipping filter_* and the like in the
   control directory can go, for example.

 * Ideally, I'd like a newgroup message with a valid description line that
   we would otherwise act upon to update the newsgroups file, even if we
   already carry that group with the same moderation status as the
   newgroup message.  This is more a TODO sort of item, though.

One style comment:

 * Looks like all of this code is written using literal tabs with vim
   magic to make the tabs look like four spaces.  I'd personally prefer
   literal four spaces (tabs are basically evil in my opinion) or failing
   that code formatted for eight-space tabs.  Right now, when browsing
   some of the code in an xterm with less, there are lines over 80
   characters.  This isn't a big deal; I'm not a style nazi of any sort.
   But four space indents with no tabs would be more consistent with the
   direction that we're trying to do with the rest of INN, and consistent
   with perlstyle.

> Semantic of logger() makes me puke.
> Looks like it really screams to be two distinct functions.
> I also want to remove the use of temp files.
> open_article() should be used *ONCE* per article.

This sounds good to me.

> Why is docheckgroups in @LIBDIR@? What about moving it to $inn:newsbin/?

I don't remember what the rational for that change was, but I don't have
any objections to moving it to the bin directory.

> Check all return values and do the right thing.
> Is aborting controlchan always the right thing?

The places where it aborts look okay to me.  I guess the thing to remember
there is that aborting will cause it generally to be immediately
restarted, which may be what you want.

> Make pgpverify (and docheckgroups?) also work as a module of
> controlchan.

That definitely sounds good.

> I'm also including a much cleaner and faster pgpverify to be used with
> gpg only. gpg now has RSA support out of the box so I can't see any
> reason to not use it.

We need to continue to support PGP for upgrades where the sysadmin already
has PGP installed and doesn't want to install GnuPG just to upgrade INN,
but I definitely agree that GnuPG should be the default where it's
available and I like your approach better than the pgpgpg approach.

I can put this in the tree and we can figure out the best way of
supporting both.  configure can figure out if gpg is available; maybe
setting the path to pgpverify and installing gpgverify as a separate
script would work.  It might be even easier if gpgverify were a module of
controlchan.

I think I want to create a new control directory in the source tree for
controlchan, controlbatch, pgpverify, and the handler programs to clean up
the organization of the source tree some.  I'm willing to put this new
code in the tree now or later, whichever is easier for you if you're still
working on it.  It'll get the most testing when it's in CURRENT.

Thank you very much for working on this!

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>