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: Closed FIXED
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 (Nobody)
URL:
Keywords:
Depends on:
Blocks: 212433
  Show dependency treegraph
 
Reported: 2017-03-09 04:19 UTC by Craig Leres
Modified: 2017-08-21 02:14 UTC (History)
4 users (show)

See Also:
koobs: maintainer-feedback+


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

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 2017-03-09 18:09:20 UTC
Created attachment 180677 [details]
poudriere build log (with BROKER enabled)
Comment 4 Thomas Zander freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 2017-07-06 04:22:38 UTC
Created attachment 184108 [details]
poudriere build log (with BROKER enabled)
Comment 13 Craig Leres freebsd_committer freebsd_triage 2017-07-06 04:23:09 UTC
Created attachment 184109 [details]
patch log
Comment 14 Craig Leres freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 2017-07-16 16:39:49 UTC
Created attachment 184405 [details]
poudriere build log (default options)
Comment 22 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 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 Craig Leres freebsd_committer freebsd_triage 2017-07-25 22:04:39 UTC
Created attachment 184710 [details]
Poudriere build log (default options)
Comment 29 Craig Leres freebsd_committer freebsd_triage 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
Comment 32 Reshad Patuck 2017-08-01 06:04:06 UTC
Created attachment 184883 [details]
patch

New patch that re-adds the 'files/bro.in' and also removes old patch files from 'files/'.

Please apply from '/usr/ports/' or the root of your ports tree.

I cant obsolete 'attachment 184709 [details]' but this patch should obsolete it.
Comment 33 Reshad Patuck 2017-08-02 05:23:47 UTC
Created attachment 184938 [details]
Poudriere build log (default options)

Bro poudriere build log.

This obsoletes the old build log (184710).
Comment 34 Craig Leres freebsd_committer freebsd_triage 2017-08-02 19:44:30 UTC
Comment on attachment 184883 [details]
patch

Sorry guys, I had no idea you were waiting for me to generate a new patchset that included the missing rc.d script; last I thing I read was, "I am using the bro.in file from the previous patch and the build works fine".

I tested the new patchset and it looks ok (other than leaving the old patches files behind, all zero length).
Comment 35 Craig Leres freebsd_committer freebsd_triage 2017-08-17 16:38:38 UTC
This PR appears blocked again. My best understanding is that as of August 2nd we had a tested patchset with maintainer approval. Is there something more I need to do?
Comment 36 Steve Wills freebsd_committer freebsd_triage 2017-08-18 19:16:20 UTC
The patch builds fine, but has a few leftover files:

====>> Error: Files or directories left over:
spool/debug.log
spool/installed-scripts-do-not-touch/site/local-logger.bro

Also, it should have USES=bison instead of BUILD_DEPENDS on bison (not a huge issue of course).

Not familiar enough with the software to find where those files are getting created. Can you find that and patch it? Thanks for the work on this port, sorry it's taken so long to get this committed. Hopefully we can clear up those issues and commit it.
Comment 37 Craig Leres freebsd_committer freebsd_triage 2017-08-18 22:46:07 UTC
Created attachment 185559 [details]
patch

(In reply to Steve Wills from comment #36)
> The patch builds fine, but has a few leftover files:
>
> ====>> Error: Files or directories left over:
> spool/debug.log
> spool/installed-scripts-do-not-touch/site/local-logger.bro
>
> Also, it should have USES=bison instead of BUILD_DEPENDS on bison (not a huge issue of course).

patch8.txt addresses these issues. As with previous patchsets, all old patches should be removed.

Sorry about the leftover files, I didn't notice because I had been using poudriere "bulk" instead of "testport" for testing.
Comment 38 Craig Leres freebsd_committer freebsd_triage 2017-08-18 22:47:07 UTC
Created attachment 185560 [details]
Poudriere build log (default options)
Comment 39 Craig Leres freebsd_committer freebsd_triage 2017-08-18 22:48:11 UTC
Created attachment 185561 [details]
Poudriere build log (with BROKER enabled)
Comment 40 commit-hook freebsd_committer freebsd_triage 2017-08-21 02:13:32 UTC
A commit references this bug:

Author: swills
Date: Mon Aug 21 02:12:49 UTC 2017
New revision: 448446
URL: https://svnweb.freebsd.org/changeset/ports/448446

Log:
  security/bro: Update to 2.5.1

  Also, unbreak build with BROKER, add rc.d script

  PR:		217656
  Submitted by:	leres@ee.lbl.gov (maintainer)

Changes:
  head/security/bro/Makefile
  head/security/bro/distinfo
  head/security/bro/files/bro.in
  head/security/bro/files/patch-aux-broker-CMakeLists.txt
  head/security/bro/files/patch-aux-broker-README
  head/security/bro/files/patch-aux-broker-src-address_type_info.hh
  head/security/bro/files/patch-aux-broker-src-data_type_info.hh
  head/security/bro/files/patch-aux-broker-src-endpoint_impl.hh
  head/security/bro/files/patch-aux-broker-src-peering_type_info.hh
  head/security/bro/files/patch-aux-broker-src-port_type_info.hh
  head/security/bro/files/patch-aux-broker-src-queue_impl.hh
  head/security/bro/files/patch-aux-broker-src-store-clone_impl.hh
  head/security/bro/files/patch-aux-broker-src-store-frontend.cc
  head/security/bro/files/patch-aux-broker-src-store-frontend_impl.hh
  head/security/bro/files/patch-aux-broker-src-store-master_impl.hh
  head/security/bro/files/patch-aux-broker-src-store-result_type_info.hh
  head/security/bro/files/patch-aux-broker-src-store-value_type_info.hh
  head/security/bro/files/patch-aux-broker-src-subnet_type_info.hh
  head/security/bro/files/patch-aux-broker-src-subscription.hh
  head/security/bro/files/patch-aux-broker-src-util-queue_actor.hh
  head/security/bro/files/patch-aux_broccoli_src_bro__openssl.c
  head/security/bro/files/patch-aux_broker_CMakeLists.txt
  head/security/bro/files/patch-aux_broker_cmake_FindCAF.cmake
  head/security/bro/files/patch-src_ChunkedIO.cc
  head/security/bro/pkg-plist
Comment 41 Steve Wills freebsd_committer freebsd_triage 2017-08-21 02:14:39 UTC
Committed, thanks!