Re: [new] liblognorm

[Stuart Henderson]

On 2017/12/26 11:57, Remi Locherer wrote:
> On Tue, Dec 26, 2017 at 02:40:30AM +0100, Jeremie Courreges-Anglas wrote:
> > On Sun, Dec 24 2017, Remi Locherer  wrote:
> > > Hi,
> > >
> > > liblognorm is a library to normalize log messages. It requires libfastjson
> > > I sent in the previous mail. It can be used together with rsyslog.
> > >
> > > The patch is needed to run the test suite. 5 out of the 110 tests fail.
> > > I informed the author about the failed tests.
> > >
> > > I tested on amd64.
> > >
> > > Reports from other platforms and OKs are welcome.
> > 
> > No need to keep the TEST_DEPENDS line.  Adding libestr to LIB_DEPENDS is
> > sufficient. 
> 
> ok, good to know!
> 
> > Regarding the patch: there's an easy way to avoid it.
> > Regarding the plist,
> > 
> >   @comment $OpenBSD$
> >   @bin bin/lognormalizer
> >   include/annot.h
> >   include/enc.h
> >   include/liblognorm.h
> >   include/lognorm-features.h
> >   include/lognorm.h
> >   include/parser.h
> >   include/pdag.h
> >   include/samp.h
> > 
> > I don't think that all those include files (with rather broad names)
> > should be installed.  Upstream documentation seems to mention
> > liblognorm.h as entry point.  It looks like this is the result of
> > a misuse of autotools recipes. Here's an updated tarball that stops
> > installing all those headers, with the hope that the patch can be fed
> > upstream (make distcheck passes).
> 
> Upstream provides all these header files in the "-devel" rpm. The
> "regular" rpm that provides the lib does not contain a header file at all.
> But that is how it works in the RHEL eco-system.

-devel rpms are used when you *build* things against a library, whereas
just the "regular" rpm is needed to satisfy run dependencies.

It's a bit like the split between headers in comp*.tgz and libraries in
base*.tgz in OpenBSD.

Re: [new] liblognorm

[Jeremie Courreges-Anglas]

On Tue, Dec 26 2017, Remi Locherer  wrote:
> On Tue, Dec 26, 2017 at 02:40:30AM +0100, Jeremie Courreges-Anglas wrote:
>> On Sun, Dec 24 2017, Remi Locherer  wrote:
>> > Hi,
>> >
>> > liblognorm is a library to normalize log messages. It requires libfastjson
>> > I sent in the previous mail. It can be used together with rsyslog.
>> >
>> > The patch is needed to run the test suite. 5 out of the 110 tests fail.
>> > I informed the author about the failed tests.
>> >
>> > I tested on amd64.
>> >
>> > Reports from other platforms and OKs are welcome.
>> 
>> No need to keep the TEST_DEPENDS line.  Adding libestr to LIB_DEPENDS is
>> sufficient. 
>
> ok, good to know!
>
>> Regarding the patch: there's an easy way to avoid it.
>> Regarding the plist,
>> 
>>   @comment $OpenBSD$
>>   @bin bin/lognormalizer
>>   include/annot.h
>>   include/enc.h
>>   include/liblognorm.h
>>   include/lognorm-features.h
>>   include/lognorm.h
>>   include/parser.h
>>   include/pdag.h
>>   include/samp.h
>> 
>> I don't think that all those include files (with rather broad names)
>> should be installed.  Upstream documentation seems to mention
>> liblognorm.h as entry point.  It looks like this is the result of
>> a misuse of autotools recipes. Here's an updated tarball that stops
>> installing all those headers, with the hope that the patch can be fed
>> upstream (make distcheck passes).
>
> Upstream provides all these header files in the "-devel" rpm.

That's probably just because of the current Makefile.am.

> The
> "regular" rpm that provides the lib does not contain a header file at all.
> But that is how it works in the RHEL eco-system.
>
>> Is the resulting package still working for you?
>
> Yes, works for me. My rsyslog wip package compiles with only the
> liblognorm.h file installed.

Nice. If the removal of those includes proves to be a problem I'll fix
it.  ok jca@ to import

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE


signature.asc
Description: PGP signature

Re: [new] liblognorm

[Remi Locherer]

On Sun, Dec 24, 2017 at 08:30:11PM +, Stuart Henderson wrote:
> On 2017/12/24 11:35, Remi Locherer wrote:
> > Hi,
> > 
> > liblognorm is a library to normalize log messages. It requires libfastjson
> > I sent in the previous mail. It can be used together with rsyslog.
> > 
> > The patch is needed to run the test suite. 5 out of the 110 tests fail.
> > I informed the author about the failed tests.
> > 
> > I tested on amd64.
> > 
> > Reports from other platforms and OKs are welcome.
> > 
> > Remi
> 
> Don't hardcode /usr/local (in the patch), use $(LOCALBASE) instead and
> pass it in as a variable (or SUBST_CMD it if you prefer).

Thank you for spotting this. With "pass it in as a variable" do you mean
something like this:

TEST_ENV =  INCDIR=${LOCALBASE}/include

And then use "-I$(INCDIR)" in the patch?

(But for this package I think jca@ came up with a solution that does not
need this patch.)

Re: [new] liblognorm

[Remi Locherer]

On Tue, Dec 26, 2017 at 02:40:30AM +0100, Jeremie Courreges-Anglas wrote:
> On Sun, Dec 24 2017, Remi Locherer  wrote:
> > Hi,
> >
> > liblognorm is a library to normalize log messages. It requires libfastjson
> > I sent in the previous mail. It can be used together with rsyslog.
> >
> > The patch is needed to run the test suite. 5 out of the 110 tests fail.
> > I informed the author about the failed tests.
> >
> > I tested on amd64.
> >
> > Reports from other platforms and OKs are welcome.
> 
> No need to keep the TEST_DEPENDS line.  Adding libestr to LIB_DEPENDS is
> sufficient. 

ok, good to know!

> Regarding the patch: there's an easy way to avoid it.
> Regarding the plist,
> 
>   @comment $OpenBSD$
>   @bin bin/lognormalizer
>   include/annot.h
>   include/enc.h
>   include/liblognorm.h
>   include/lognorm-features.h
>   include/lognorm.h
>   include/parser.h
>   include/pdag.h
>   include/samp.h
> 
> I don't think that all those include files (with rather broad names)
> should be installed.  Upstream documentation seems to mention
> liblognorm.h as entry point.  It looks like this is the result of
> a misuse of autotools recipes. Here's an updated tarball that stops
> installing all those headers, with the hope that the patch can be fed
> upstream (make distcheck passes).

Upstream provides all these header files in the "-devel" rpm. The
"regular" rpm that provides the lib does not contain a header file at all.
But that is how it works in the RHEL eco-system.

> Is the resulting package still working for you?

Yes, works for me. My rsyslog wip package compiles with only the
liblognorm.h file installed.

Re: [new] liblognorm

[Jeremie Courreges-Anglas]

On Sun, Dec 24 2017, Remi Locherer  wrote:
> Hi,
>
> liblognorm is a library to normalize log messages. It requires libfastjson
> I sent in the previous mail. It can be used together with rsyslog.
>
> The patch is needed to run the test suite. 5 out of the 110 tests fail.
> I informed the author about the failed tests.
>
> I tested on amd64.
>
> Reports from other platforms and OKs are welcome.

No need to keep the TEST_DEPENDS line.  Adding libestr to LIB_DEPENDS is
sufficient.  Regarding the patch: there's an easy way to avoid it.
Regarding the plist,

  @comment $OpenBSD$
  @bin bin/lognormalizer
  include/annot.h
  include/enc.h
  include/liblognorm.h
  include/lognorm-features.h
  include/lognorm.h
  include/parser.h
  include/pdag.h
  include/samp.h

I don't think that all those include files (with rather broad names)
should be installed.  Upstream documentation seems to mention
liblognorm.h as entry point.  It looks like this is the result of
a misuse of autotools recipes. Here's an updated tarball that stops
installing all those headers, with the hope that the patch can be fed
upstream (make distcheck passes).

Is the resulting package still working for you?



liblognorm.tgz
Description: Binary data

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: [new] liblognorm

[Stuart Henderson]

On 2017/12/24 11:35, Remi Locherer wrote:
> Hi,
> 
> liblognorm is a library to normalize log messages. It requires libfastjson
> I sent in the previous mail. It can be used together with rsyslog.
> 
> The patch is needed to run the test suite. 5 out of the 110 tests fail.
> I informed the author about the failed tests.
> 
> I tested on amd64.
> 
> Reports from other platforms and OKs are welcome.
> 
> Remi

Don't hardcode /usr/local (in the patch), use $(LOCALBASE) instead and
pass it in as a variable (or SUBST_CMD it if you prefer).