Re: [htdig-dev] Adjustable logging patch.

[Gilles D. <gr...@sc...>]

According to Brian White:
> >According to J. op den Brouw:
> > > It's a nice patch for those who cannot use syslog facilities, but
> > > the patch removes the syslog logging feature. It would be nice
> > > to select one of them (or have them both) on compile or run time
> > > basis.
> > >
> > > It's also a patch against 3.1.6. It would be nice if there's a
> > > patch for 3.2.0b4-xxxx too.
> > >
> > > Furthermore, I see a flock() call somewhere. AFAIK, different
> > > OS-es use different names and parameter lists. Example
> > >
> > > HP-UX:   int lockf(int fildes, int function, off_t size);
> > > Linux 2.2:   int flock(int fd, int operation);
> >
> >I hadn't noticed when I looked at the patch that it completely removed
> >the ability to log to syslog().  That's one more reason to reject
> >it for 3.1.x.  I rejected it over concerns about portability, as you
> >pointed out.  I don't think it's appropriate for inclusion in 3.1.7
> >either for that reason.
> 
> Ok.
> 
> 1) The patch does not remove the ability to do syslog. In my notes
>     that go with the patch it says:
> 
> >    * logging_file ( Default: none )
> >
> >      If this is set to "none", then it will log using syslog, otherwise
> >      this will be assumed to be the path to the log file
> 
> 
>     The whole way it is set up, it uses the existing default
>     behaviour if it isn't explicitly activated.

Good.  I didn't recall seeing any red flags go up in regards to this last
time I looked at your patch, but that was a while ago.  I didn't review
your patch when Jesse made this statement, so I took his word for it.

> 2) If the issue is the portability of flock, would it be
>     acceptable if I changed it over to using fcntl?
> 
>      (Mr Google threw up the follwoing page which says that "fcntl() is the
>       only POSIX-compliant locking mechanism, and is therefore the only
>       truly portable lock"
> 
>       http://www.erlenstar.demon.co.uk/unix/faq_3.html
>       )

Well, while going with POSIX-compliant locking would help with
portability, I'm not sure all systems currently supported by 3.1.x
are fully POSIX-compliant either, so it may be that some only support
flock(), or even perhaps no locking at all.  Some configure tests for
various locking schemes should be implemented, so the code uses what
the system provides, or no locking at all if nothing appropriate is found.

> 3) It should be simple enough to create a patch that works with 3.2.x,
>     judging by a quick look at the latest Display.cc in the CVS repository.
> 
> I *would* like to get it rolled into 3.1.x if I can. I am
> more than willing to make any changes required to make this
> happen.

I think it would be good to see this in the 3.2 CVS tree, with the
appropriate configure tests.  I'm still a bit lukewarm on the addition
of the "init" input parameter to htsearch.  It seems the absense of a
"page" parameter would mean the same thing, wouldn't it?

As for 3.1.x, though, here are my thoughts.  I'm quite adament about
not wanting to put out a 3.1.8 release.  So, that means I have to be
very adament about getting 3.1.7 right, with no new bugs or portability
problems.  To do that, I think I'm going to need to put my foot down as
far as the feature freeze, and insist that only bug fixes go into 3.1.7,
and no new features.  The only discussed new feature for 3.1.7 that I
haven't completely ruled out yet is location_factor, because it's tied
to some bug fixes in WordList::Word() anyway, and had been planned for
3.1.6 but fell through the cracks.  I may drop this attribute anyway,
and stick to just bug fixes.

-- 
Gilles R. Detillieux              E-mail: <gr...@sc...>
Spinal Cord Research Centre       WWW:    http://www.scrc.umanitoba.ca/
Dept. Physiology, U. of Manitoba  Winnipeg, MB  R3E 3J7  (Canada)