Bug 217656 - security/bro: Update to 2.5.1, unbreak build with BROKER, add rc.d script
Summary: security/bro: Update to 2.5.1, unbreak build with BROKER, add rc.d script
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: FreeBSD Ports Bugs (Mailing List)
URL:
Keywords: patch
Depends on:
Blocks: 212433
  Show dependency treegraph
 
Reported: 2017-03-09 04:19 UTC by leres
Modified: 2017-07-26 09:51 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback+


Attachments
patch (36.04 KB, text/plain)
2017-03-09 04:19 UTC, leres
leres: maintainer-approval+
Details
patch (36.14 KB, patch)
2017-03-09 18:07 UTC, leres
leres: maintainer-approval+
Details | Diff
poudriere build log (with BROKER enabled) (996.54 KB, text/plain)
2017-03-09 18:09 UTC, leres
no flags Details
patch (41.44 KB, patch)
2017-03-12 18:16 UTC, leres
leres: maintainer-approval+
Details | Diff
patch (39.66 KB, patch)
2017-06-21 17:31 UTC, leres
leres: maintainer-approval+
Details | Diff
poudriere build log (with BROKER enabled) (57.34 KB, application/octet-stream)
2017-06-21 17:33 UTC, leres
no flags Details
patch log (7.04 KB, text/plain)
2017-06-26 15:56 UTC, leres
no flags Details
patch (40.90 KB, patch)
2017-07-06 04:21 UTC, leres
leres: maintainer-approval+
Details | Diff
poudriere build log (with BROKER enabled) (64.89 KB, text/plain)
2017-07-06 04:22 UTC, leres
no flags Details
patch log (7.05 KB, text/plain)
2017-07-06 04:23 UTC, leres
no flags Details
shar file (50.46 KB, text/plain)
2017-07-06 04:26 UTC, leres
no flags Details
patch (41.22 KB, patch)
2017-07-16 16:38 UTC, leres
leres: maintainer-approval+
Details | Diff
poudriere build log (default options) (58.34 KB, text/plain)
2017-07-16 16:39 UTC, leres
no flags Details
poudriere build log (with BROKER enabled) (67.97 KB, text/plain)
2017-07-16 16:41 UTC, leres
no flags Details
patch (39.77 KB, patch)
2017-07-25 22:03 UTC, leres
leres: maintainer-approval+
Details | Diff
Poudriere build log (default options) (60.19 KB, text/plain)
2017-07-25 22:04 UTC, leres
no flags Details
Poudriere build log (with BROKER enabled) (67.06 KB, text/plain)
2017-07-25 22:05 UTC, leres
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description leres 2017-03-09 04:19:25 UTC
Created attachment 180661 [details]
patch

This PR updates security/bro from 2.4.1 to 2.5. It unbreaks building with BROKER enabled and resolves PR 212433:

    security/bro: Add rc script to work with rc.conf

All existing patch files except patch-aux-broker-CMakeLists.txt should be removed.

bro 2.4.1 required devel/caf 0.13.X but devel/caf was upgraded to 0.14.X around the time bro 2.4.1 was release so at the time we used patches to bro that the developers provided.

bro 2.5 requires devel/caf 0.14.X but devel/caf was recently updated to 0.15.3 which is massively incompatible (and currently prevents security/bro 2.4.1 from compiling when the BROKER option is selected). The developers reported that patching bro to work with caf 0.15.X would be difficult so rather than chasing the caf port bro now builds caf 0.14.5 statically. Included are two new patch files from the developers for caf that clean up some sharp edges involved with using caf in this manner.

The new rc.d script was contributed by mshirk@daemon-security.com (PR 212433).

Finally, bro is now built using ninja which is what the developers use.
Comment 1 leres 2017-03-09 04:22:35 UTC
(I have a poudriere build log but apparently it's too buku to attach to this PR.)
Comment 2 leres 2017-03-09 18:07:03 UTC
Created attachment 180676 [details]
patch

Here is a revised patchset with two improvements. Make CXXFLAGS closer to what the developers use and reduce the number of warning lines by 100X. Also only install rc.d script when BROCTL is selected.
Comment 3 leres 2017-03-09 18:09:20 UTC
Created attachment 180677 [details]
poudriere build log (with BROKER enabled)
Comment 4 Thomas Zander freebsd_committer 2017-03-12 10:04:55 UTC
(In reply to leres from comment #2)

Could you revisit the patch with regard to these two portlint findings:
FATAL: Makefile: bro listed in SUB_FILES/USE_RC_SUBR, but files/bro.in is missing.
WARN: /poudriere/ports/default/security/bro/files/patch-aux-broker-CMakeLists.txt: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' when you need to [re-]generate a patch to ensure proper patch format.
Comment 5 leres 2017-03-12 18:16:28 UTC
Created attachment 180753 [details]
patch

New patchset with bro.in included and portlint makepatch warnings fixed.

Note: The patch file names have changed so please remove all old files/patch-* files before applying this patchset.
Comment 6 Reshad Patuck 2017-06-13 12:07:43 UTC
(In reply to leres from comment #5)
Hey, tried this patchset against bro in the head of the repository, but it fails to apply.

I have gone through the current port and it looks like most of the patches used are already part of v2.5.

A clean port may be a better way to do this as bro builds fine from source without any patches.

Will look into it when I get some time.
Comment 7 leres 2017-06-21 17:31:14 UTC
Created attachment 183681 [details]
patch

(In reply to Reshad Patuck from comment #6)
Here's a new patchset that catches up with unrelated port updates that occurred since I generated the last one. As with the last patchset, please remove all old files/patch-* files before applying this one.
Comment 8 leres 2017-06-21 17:33:25 UTC
Created attachment 183682 [details]
poudriere build log (with BROKER enabled)
Comment 9 Reshad Patuck 2017-06-26 09:27:18 UTC
(In reply to leres from comment #7)
Hey,

I can't seem to apply your patch.

Could you please send me an archive of the directory (shar if possible)

Thanks
Comment 10 leres 2017-06-26 15:56:05 UTC
Created attachment 183814 [details]
patch log

(In reply to Reshad Patuck from comment #9)

> I can't seem to apply your patch.

I'd be curious to see what errors you're getting, maybe you could send me a log offline?

I've attached a log showing the patchset being applied.

> Could you please send me an archive of the directory (shar if possible)

Will do.
Comment 11 leres 2017-07-06 04:21:28 UTC
Created attachment 184107 [details]
patch

Bro 2.5.1 was released on June 26th. Here's a revised patchset for it.
Comment 12 leres 2017-07-06 04:22:38 UTC
Created attachment 184108 [details]
poudriere build log (with BROKER enabled)
Comment 13 leres 2017-07-06 04:23:09 UTC
Created attachment 184109 [details]
patch log
Comment 14 leres 2017-07-06 04:26:37 UTC
Created attachment 184110 [details]
shar file

Here's the updated port as a shar file
Comment 15 Reshad Patuck 2017-07-16 07:06:47 UTC
(In reply to leres from comment #14)

Hi, sorry for the delay.

I just tried compiling this on FreeBSD 10.3-RELEASE, 11.0-RELEASE and 12-CURRENT.

The port compiles with default options set (i.e. `make` runs) however the package build fails (make pkg) with the following error.

---
root@FreeBSD-10:/usr/ports/security/bro # make package
===>  Building package for bro-2.5.1
pkg-static: Unable to access file /usr/ports/security/bro/work/stage/usr/local/lib/broctl/_pybroker.so:No such file or directory
pkg-static: Unable to access file /usr/ports/security/bro/work/stage/usr/local/lib/broctl/pybroker.py:No such file or directory
*** Error code 1

Stop.
make[1]: stopped in /usr/ports/security/bro
*** Error code 1

Stop.
make: stopped in /usr/ports/security/bro
root@FreeBSD-10:/usr/ports/security/bro # 
---

Looks like the pkg-plist has the following files in it which are only generated when you use set the 'BROKER' option.

With the 'BROKER' option set the port builds fine on FreeBSD 10.3 but fails to build on 11.0 and 12.

It stops at "[2/95] Building CXX object libcaf_core/CMakeFiles/libcaf_core_static.dir/src/abstract_actor.cpp.o" for FreeBSD 11.0
and at "[8/95] Building CXX object libcaf_core/CMakeFiles/libcaf_core_static.dir/src/abstract_coordinator.cpp.o" for FreeBSD 12.

In both cases the error message is "ninja: build stopped: subcommand failed."

I will look into this more and also look at removing the pybroker files from the plist and update you here.

Thanks,

Reshad
Comment 16 Reshad Patuck 2017-07-16 07:27:22 UTC
Update:

After removing these two lines from the 'pkg-plist' the bro package builds fine with default ports options (no BROKER) on all three versions of FreeBSD 10.3, 11.0 and 12-CURRENT.

---
%%BROCTL%%lib/broctl/_pybroker.so
%%BROCTL%%lib/broctl/pybroker.py
---
Comment 17 Kubilay Kocak freebsd_committer 2017-07-16 09:28:15 UTC
Comment on attachment 184110 [details]
shar file

Only diffs for updates need approval (since shars are not accepted for updates)
Comment 18 Kubilay Kocak freebsd_committer 2017-07-16 09:29:51 UTC
If the update can be separated from the build/run fix, the latter can be merged to quarterly without the version update (highly preferred)
Comment 19 leres 2017-07-16 15:37:00 UTC
(In reply to Kubilay Kocak from comment #18)
> If the update can be separated from the build/run fix, the latter
> can be merged to quarterly without the version update (highly
> preferred)

The update causes the build issues so it's very likely I don't understand your question.

The BROKER vs. BROCTL pkg-plist error should be easy to fix. I'm looking into the other problems related to building with default options.
Comment 20 leres 2017-07-16 16:38:41 UTC
Created attachment 184404 [details]
patch

Differences from last patch set:
 
 - Fix BROCTL vs. BROKER pkg-plist issues
 
 - BROCTL also needs swig
Comment 21 leres 2017-07-16 16:39:49 UTC
Created attachment 184405 [details]
poudriere build log (default options)
Comment 22 leres 2017-07-16 16:41:10 UTC
Created attachment 184406 [details]
poudriere build log (with BROKER enabled)
Comment 23 Reshad Patuck 2017-07-24 03:47:31 UTC
(In reply to leres from comment #20)

Hey, I cant seem to apply the patch.

To clarify, I can apply the patch but the patch does not clean up files that are no longer needed so make fails to apply patches.

Can you use `svn diff` to make your patch file.

Here is how you go about doing it https://www.freebsd.org/doc/en/books/porters-handbook/port-upgrading.html#svn-diff

Thanks

Reshad
Comment 24 leres 2017-07-24 04:15:32 UTC
(In reply to Reshad Patuck from comment #23)
Actually I do use "svn diff" to generate the patches. 

What I would suggest is to start with:

    svn delete files/patch-*

Then apply the patchset. Finally, add in the new patches with:

    svn add files/patch-*

Then you should be ready to test and/or commit the changes.
Comment 25 Reshad Patuck 2017-07-24 07:16:54 UTC
(In reply to leres from comment #24)

Hey, Thanks for the patch

The patch builds now (stock without BROKER) but with BROKER on 11 and 12 is still broken.

I don't need BROKER so this is fine for me, but we should fix this before we commit it to the ports tree. 

Here is the error I get on 12, I get a similar thing on 11. 10.3 builds fine.

---

root@FreeBSD-ports:/usr/ports/security/bro/work/.build-caf # ninja -j 1 install
[1/88] Building CXX object libcaf_core/CMakeFiles/libcaf_core_static.dir/src/actor_addr.cpp.o
FAILED: libcaf_core/CMakeFiles/libcaf_core_static.dir/src/actor_addr.cpp.o 
/usr/bin/CC   -I/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core -I/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_io -I/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_test -I/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/. -std=c++11 -Wextra -Wall -pedantic -ftemplate-depth=512 -stdlib=libc++ -pthread -fPIC -O2 -g -MD -MT libcaf_core/CMakeFiles/libcaf_core_static.dir/src/actor_addr.cpp.o -MF libcaf_core/CMakeFiles/libcaf_core_static.dir/src/actor_addr.cpp.o.d -o libcaf_core/CMakeFiles/libcaf_core_static.dir/src/actor_addr.cpp.o -c /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/src/actor_addr.cpp
In file included from /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/src/actor_addr.cpp:22:
In file included from /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/local_actor.hpp:33:
/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/message.hpp:403:10: warning: redundant move in return statement [-Wredundant-move]
  return std::move(other);
         ^
/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/message.hpp:403:10: note: remove std::move call here
  return std::move(other);
         ^~~~~~~~~~     ~
In file included from /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/src/actor_addr.cpp:22:
In file included from /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/local_actor.hpp:38:
In file included from /usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/spawn_fwd.hpp:27:
/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/typed_actor.hpp:197:21: error: 'ptr_' is a private member of 'caf::actor'
    ptr_.swap(other.ptr_);
                    ^
/usr/ports/security/bro/work/actor-framework-0.14.5/libcaf_core/caf/actor.hpp:173:22: note: declared private here
  abstract_actor_ptr ptr_;
                     ^
1 warning and 1 error generated.
ninja: build stopped: subcommand failed.
Comment 26 Reshad Patuck 2017-07-24 07:37:26 UTC
(In reply to leres from comment #24)

One more small thing, can we move the bro config files:
- broctl.cfg
- networks.cfg
- node.cfg

and the example files from /usr/local/etc to /usr/local/etc/bro

It just is cleaner to have the config files in its own directory instead of directly in /usr/local/etc
Comment 27 leres 2017-07-25 22:03:44 UTC
Created attachment 184709 [details]
patch

Upstream reports the "private member" error and a few warnings were fixed by caf 0.14.6 (this is literally all that changed) so here's a new patchset that switches from 0.14.5 to 0.14.6. As always with this PR, please remove all existing files/patch-* files before applying the patch.

Regarding movement of the three bro config files, could we please not do this?  Personally this will cause grief for my infrastructure guys (moving configs when upgrading bro on our numerous clusters, updating puppet manifests, etc. I hate to create these types of problems for the bro user base when there really isn't a problem that's being solved.
Comment 28 leres 2017-07-25 22:04:39 UTC
Created attachment 184710 [details]
Poudriere build log (default options)
Comment 29 leres 2017-07-25 22:05:23 UTC
Created attachment 184711 [details]
Poudriere build log (with BROKER enabled)
Comment 30 Reshad Patuck 2017-07-26 04:23:18 UTC
(In reply to leres from comment #27)

Thanks for the patch.

The movement of the bro config files was more a suggestion in the interest of cleanliness, but yes backwords compatibility is probably a more important concern.

Leave the config files where they are.

I'll build your patch and report here in a bit.
Comment 31 Reshad Patuck 2017-07-26 09:51:04 UTC
(In reply to leres from comment #27)

Hey, latest patch is missing the `files/bro.in` file (rc.d script).

I am using the bro.in file from the previous patch and the build works fine, both with and without BROKER.

Thanks