Comment 1 Craig 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 Craig 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 Craig Leres 2017-03-09 18:09:20 UTC

Created attachment 180677 [details]
poudriere build log (with BROKER enabled)

Comment 4 Thomas Zander 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 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.

(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 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 2017-06-21 17:33:25 UTC

Created attachment 183682 [details]
poudriere build log (with BROKER enabled)

(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 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 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 2017-07-06 04:22:38 UTC

Created attachment 184108 [details]
poudriere build log (with BROKER enabled)

Comment 13 Craig Leres 2017-07-06 04:23:09 UTC

Created attachment 184109 [details]
patch log

Comment 14 Craig Leres 2017-07-06 04:26:37 UTC

Created attachment 184110 [details]
shar file

Here's the updated port as a shar file

(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

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 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 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 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 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 2017-07-16 16:39:49 UTC

Created attachment 184405 [details]
poudriere build log (default options)

Comment 22 Craig Leres 2017-07-16 16:41:10 UTC

Created attachment 184406 [details]
poudriere build log (with BROKER enabled)

(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 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.

(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.

(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 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 2017-07-25 22:04:39 UTC

Created attachment 184710 [details]
Poudriere build log (default options)

Comment 29 Craig Leres 2017-07-25 22:05:23 UTC

Created attachment 184711 [details]
Poudriere build log (with BROKER enabled)

(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.

(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

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.

Created attachment 184938 [details]
Poudriere build log (default options)

Bro poudriere build log.

This obsoletes the old build log (184710).

Comment 34 Craig Leres 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 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 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 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 2017-08-18 22:47:07 UTC

Created attachment 185560 [details]
Poudriere build log (default options)

Comment 39 Craig Leres 2017-08-18 22:48:11 UTC

Created attachment 185561 [details]
Poudriere build log (with BROKER enabled)

Comment 40 commit-hook 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 2017-08-21 02:14:39 UTC

Committed, thanks!