Skip to content

OpenBSD support#955

Merged
Neverlord merged 5 commits intomasterfrom
topic/openbsd
Nov 10, 2019
Merged

OpenBSD support#955
Neverlord merged 5 commits intomasterfrom
topic/openbsd

Conversation

@Neverlord
Copy link
Copy Markdown
Member

Since our friends over in Zeek land need the CAF-side fix, I tried #950 on OpenBSD today. I noticed that many unit test fail due to the missing AI_V4MAPPED support. Since fixing this requires non-trivial changes, I'm fleshing out what @douglas-carmichael started

This PR builds on #950, fixes SIGPIPE handling, and makes sure unit tests don't run into AI_V4MAPPED-related failures.

@Neverlord
Copy link
Copy Markdown
Member Author

@douglas-carmichael btw, I've tested this on a fresh OpenBSD 6.6. It'd be great if you could double-check whether this branch works for you as well. 🙂

@douglas-carmichael
Copy link
Copy Markdown
Contributor

I was able to compile and install CAF successfully, but Zeek couldn't find it even when building with the --with-caf option:

CMake Error at /usr/local/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
Could NOT find CAF (missing: CAF_LIBRARIES core io openssl)
Call Stack (most recent call first):
/usr/local/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
cmake/FindCAF.cmake:112 (find_package_handle_standard_args)
CMakeLists.txt:343 (find_package)

@Neverlord
Copy link
Copy Markdown
Member Author

I was able to compile and install CAF successfully

Great, thanks for checking! 👍

Zeek couldn't find it

Just for the record: the Zeek repository is the right place to bring up build issues related to Zeek. But you already did that in the meantime. 🙂

@jsiwek
Copy link
Copy Markdown
Contributor

jsiwek commented Nov 7, 2019

Just for the record: the Zeek repository is the right place to bring up build issues related to Zeek. But you already did that in the meantime.

The inability to use --with-caf in Zeek may need a fix in CAF: the find_library() call in FindCAF.cmake isn't locating CAF libraries only on OpenBSD because the installed libraries on that platform are like:

foo# ls -l /home/sandbox/caf/lib/
total 157632
-rw-r--r--  1 root  wheel  42661128 Nov  6 22:32 libcaf_core.so.0.17.2
-rw-r--r--  1 root  wheel  33096496 Nov  6 22:32 libcaf_io.so.0.17.2
-rw-r--r--  1 root  wheel   4878832 Nov  6 22:32 libcaf_openssl.so.0.17.2

But OpenBSD shared library naming conventions don't permit a full X.Y.Z version, just X.Y. See https://www.openbsd.org/faq/ports/specialtopics.html#SharedLibs

And also CMake's related documentation for behavior of find_library() on OpenBSD also suggests it's only looking for X.Y versions: https://cmake.org/cmake/help/v3.15/prop_gbl/FIND_LIBRARY_USE_OPENBSD_VERSIONING.html

So the minimal thing to do here (or as separate issue/PR) is change the following CMake config (for all CAF shared libs) to instead use VERSION ${CAF_VERSION_MAJOR}.${CAF_VERSION_MINOR}:

set_target_properties(libcaf_core_shared
PROPERTIES
SOVERSION ${CAF_VERSION}
VERSION ${CAF_VERSION}
OUTPUT_NAME caf_core

Could do that conditionally for only OpenBSD. Or consider using just X.Y for all platforms since the last number is optional (e.g. reference for at least Linux to that effect: http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html) and of minor usefulness (IMO, never had a problem understanding which version is installed and don't see much demand for there being two different Patch-level versions installed since the latest one of those is always going to be the better choice).

Though, there's also correctness/strictness of what's being used for the VERSION number: general expectations here are for Major to mean "downward-incompatible ABI (breaking changes)", Minor to mean "upward-incompatible ABI (just additions)", and optional Patch to mean "no ABI change". OpenBSD seems more bent on making this filename version matter more than the soname/soversion, which on other platforms I think is the main way to convey ABI compatibility, so not sure if you'd like to start maintaining the library VERSION differently because of this.

Speaking of SOVERSION, I don't think that has to change necessarily. I understand it to mean, in CAF's case, every new CAF version, even Patch-releases, are considered ABI-incompatible. That might be more conservative than actually needed. Before, I've approached this as an arbitrary number incremented on each ABI breakage and entirely separate from project/library VERSION (which could then use SemVer strictly, loosely, or another system entirely), but not sure anymore that's compliant with OpenBSD's naming scheme since seems VERSION really would have to follow their semantics to play nice.

@Neverlord
Copy link
Copy Markdown
Member Author

@jsiwek thanks a lot for this analysis and the suggested solutions!

So the minimal thing to do here (or as separate issue/PR) is change the following CMake config (for all CAF shared libs) to instead use VERSION ${CAF_VERSION_MAJOR}.${CAF_VERSION_MINOR}

I'll set the version as you suggest. I tend towards doing this only on OpenBSD for now.

I understand it to mean, in CAF's case, every new CAF version, even Patch-releases, are considered ABI-incompatible.

That's correct. Currently, our policy is to not guarantee any ABI compatibility. That will change once we reach the 1.0 milestone.

@Neverlord
Copy link
Copy Markdown
Member Author

Just a quick update: I'm going to rebase this PR after we've merged #965 and then I'll add the changes @jsiwek suggested.

Neverlord and others added 3 commits November 10, 2019 10:27
Listing each OS that hasn't AI_V4MAPPED defined is both verbose and
brittle. Simply checking whether AI_V4MAPPED exists before attempting to
use it always works, regardless of the OS.
@Neverlord Neverlord removed the request for review from josephnoir November 10, 2019 11:05
@Neverlord Neverlord merged commit 8c338f4 into master Nov 10, 2019
@Neverlord Neverlord deleted the topic/openbsd branch November 10, 2019 12:39
@Neverlord
Copy link
Copy Markdown
Member Author

After the changes, CAF install these files on OpenBSD:

lib
|-- libcaf_core.so.0.17
|-- libcaf_core.so.0.17.2 -> libcaf_core.so.0.17
|-- libcaf_io.so.0.17
|-- libcaf_io.so.0.17.2 -> libcaf_io.so.0.17
|-- libcaf_openssl.so.0.17
`-- libcaf_openssl.so.0.17.2 -> libcaf_openssl.so.0.17

CAF builds without issues and the CMake script of Broker picks up the correct CAF library without any changes to Broker:

==================|  Broker Config Summary  |====================
Version:         1.2.0-120
SO version:      2

Build Type:      debug
Install prefix:  /home/neverlord/bundle
Library prefix:  /home/neverlord/bundle/lib
Shared libs:     yes
Static libs:     no

CC:              /usr/bin/cc
CFLAGS:           -Wall -Wno-unused -g -DDEBUG -DBRO_DEBUG -g
CXX:             /usr/bin/c++
CXXFLAGS:         -Wall -Wno-unused -g -DDEBUG -DBRO_DEBUG -Wno-register -std=c++17  -pthread -Wall -Wno-unused -pedantic -ftemplate-depth=512 -ftemplate-backtrace-limit=3 -g

CAF:             /home/neverlord/bundle/include (0.17.2)
RocksDB:         no
Python bindings: yes
Zeek:            no
=================================================================

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants