On Thu, Oct 04, 2018 at 12:39:57PM +0100, Stuart Henderson wrote:
> On 2018/10/04 12:47, Paul Irofti wrote:
> > > Sorry I don't think it's ready for commit yet, there are a few problems ..
> >
> > Sure, thank you for reviewing this!
> >
> > > - Bad distfile name, it's already using an on-the-fly
> > > tarball from github anyway so the easy fix is to use the GH_* scaffolding
> > >
> > > - Compiler command lines are hidden which makes it hard to track down some
> > > problems in bulk build logs
> > >
> > > - Needs WANTLIB etc.
> > >
> > > (Diff for the above three attached)
> >
> > Thanks!
> >
> > > - In the build of the embedded copy of libz, it's forcing "gcc -O3"
> > > and for lua it does use ${CC} but forces -O2, looks like forced -O2 in
> > > ossec's own files too?
> >
> > I try not to touch hardcoded optimization levels. Which I know is a
> > faux-pas in our ports tree, but I tend to trust the software developers
> > more now that we have modern compilers in the tree.
> >
> > If needed, I will fix this.
>
> Some arches have modern compilers, but we had something that needed -O1
> to avoid a compiler problem just last week (and modern compilers keep
> adding more and more optimisations, and I'm not sure I trust people that
> came up with the build system to track this and know which levels are
> really safe on the various compilers that might be used..)
OK. I'll fix it. Don't want to argue :)
> > > - Patches have hardcoded /usr/local
> >
> > Yes. I thought that we decided against supporting other install
> > directories. I can substitute them for TRUEPREFIX if needed.
>
> espie has been fixing a bunch of things that don't do this recently,
> so I don't think that has been decided :)
>
> For "things relating to the current port" (connected with the install
> location etc) it's TRUEPREFIX, for "things from another port" it's
> LOCALBASE so that's what you need for CFLAGS/LDFLAGS lines etc.
>
> (I'm not convinced the distinction between TRUEPREFIX/LOCALBASE in ports
> makes sense, but that's how it's handled at the moment, so best to follow
> that and save Antoine from fixing it later ;)
I'll fix this too then...
> > > - (also it's not ideal that it's NO_BUILD and everything is built in
> > > "make install", though upstream doesn't make this easy to fix..)
> >
> > It's a horror the way they build it. I would basically have to rewrite
> > the entire build system for them and then maintain it on each release.
> > It has already been a disgusting experience tracking down the file
> > permissions inside pkg/PLIST :(
>
> It is full of wonders. Considering what ossec-hids is used for, I find it
> quite surprising they have written a build system that's A) clearly intended
> for compiling directly on the monitored machine itself and B) with the whole
> build intending to run as root. I guess this is the way of companies who
> make Enterprise Cybersecurity Solutions.
I agree :)
