FreeBSD and OpenBSD build fixes #1815

jrmithdobbs commented at 12:46 AM on September 10, 2012: contributor

OpenBSD is failing some tests related to wallet coin selection AND SHOULD NOT UNDER ANY CIRCUMSTANCES BE USED AS THE OS ON ANY INSTANCES HANDLING A WALLET YET.

The *.d cleanup in the clean target is needed for instances when you try and link against a bad db version (<4.8) but they shouldn't exist under "normal" circumstances. Should still clean them up.

The swap of the bdb and boost include search paths is intentional. The openbsd port/package for db4 is stuck at 4.6 for right now and exists in /usr/local. Boost also exists in /usr/local so if it's specified first you can't get the linker to find the correct bdb.

Getting the unit tests working on OpenBSD requires patching the boost 1.42 port before building as the provided one will not work with any unit tests whatsoever.

I'll write up build instructions for both platforms for a different pull request.

I've tested that this builds on both OpenBSD 5.1 and FreeBSD 9.0-RELEASE-p4. (AKA, current stable versions as of this writing.)

I've also tested that it does not break builds on debian wheezy.

Edit:

FreeBSD build example:

BOOST_INCLUDE_PATH=/usr/local/include
BOOST_LIB_PATH=/usr/local/lib
BDB_INCLUDE_PATH=/usr/local/include/db48
BDB_LIB_PATH=/usr/local/lib/db48
gmake -f makefile.unix -j8 USE_UPNP= bitcoind test_bitcoin

OpenBSD build example:

BOOST_INCLUDE_PATH=/usr/local/include
BOOST_LIB_PATH=/usr/local/lib
BDB_INCLUDE_PATH=/opt/OpenBSD/5.1/amd64/db4-4.8.30/include
BDB_LIB_PATH=/opt/OpenBSD/5.1/amd64/db4-4.8.30/lib
BOOST_LIB_SUFFIX=-mt
gmake -f makefile.unix -j8 USE_UPNP= bitcoind test_bitcoin

See this pastebin for details on the failing unit tests on OpenBSD: http://pastebin.com/kFjiXui2

BitcoinPullTester commented at 3:52 PM on September 10, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f518b2c39868eeacae871dc79a7f270736b984e8 for binaries and test log.

in src/netbase.cpp:None in f518b2c398 outdated

  83 | @@ -84,7 +84,13 @@ bool static LookupIntern(const char *pszName, std::vector<CNetAddr>& vIP, unsign
  84 |  #  else
  85 |      aiHint.ai_family = AF_INET;
  86 |  #  endif
  87 | +/* If we don't have AI_ADDRCONFIG (*BSD) then don't try and use it as it is
  88 | + * probably the default behaviour anyhow! */
  89 | +#  ifdef AI_ADDRCONFIG
  90 |      aiHint.ai_flags = fAllowLookup ? AI_ADDRCONFIG : AI_NUMERICHOST;
  91 | +#  else
  92 | +    aiHint.ai_flags = fAllowLookup ? 0 : AI_NUMERICHOST;

luke-jr commented at 5:27 AM on September 11, 2012:

Maybe just #define AI_ADDRCONFIG 0 ?

laanwj commented at 5:32 AM on September 11, 2012:

You could add (#ifndef AI_ADDRCONFIG #define AI_ADDRCONFIG 0) to compat.h, it has other such special cases.

in src/makefile.unix:None in f518b2c398 outdated

   7 | @@ -8,9 +8,11 @@ USE_IPV6:=1
   8 |  LINK:=$(CXX)
   9 |  
  10 |  DEFS=-DBOOST_SPIRIT_THREADSAFE
  11 | +# This is needed for broken bdb48 includes when built for openbsd
  12 | +DEFS+=-DHAVE_CXX_STDHEADERS

laanwj commented at 5:33 AM on September 11, 2012:

Shouldn't we add it only for openbsd, then?

jrmithdobbs commented at 5:39 AM on September 11, 2012:

If that def isn't valid, bitcoin wont build correctly anyways. It just implies using #include iostream vs #include iostream.h and similar.

Github wont let me put the angle brackets in the comment for whatever reason, annoying.

This is more related to us not using autotools than anything.

laanwj commented at 5:46 AM on September 11, 2012:

Yes a cmake or autotools build system would be great. Feel free to contribute that instead :)

jrmithdobbs commented at 5:48 AM on September 11, 2012:

No, I mean the db headers seem to assume you're using auto tools. Since that's a (previously un-codified) req for building the rest of bitcoin any how I don't see how it hurts to just define it like we do the boost threading defines.

in src/makefile.unix:None in f518b2c398 outdated

  52 | @@ -51,7 +53,6 @@ endif
  53 |  LIBS+= \
  54 |   -Wl,-B$(LMODE2) \
  55 |     -l z \
  56 | -   -l dl \

laanwj commented at 5:34 AM on September 11, 2012:

Don't remove this, it's required for static builds !

jrmithdobbs commented at 5:42 AM on September 11, 2012:

Suggestions? This breaks all *BSD builds and works on everything for non-static builds.

laanwj commented at 5:47 AM on September 11, 2012:

I suppose you could do

ifdef STATIC
LIBS += -l dl
endif

jrmithdobbs commented at 5:54 AM on September 11, 2012:

I like that better but it actually doesn't work on *BSD at all. -ldl is part of libc. I can successfully build statically on open/freebsd as is without adding it back in.

To clarify, there isn't an -ldl on these systems at all. That's a linuxism. afaik

laanwj commented at 6:14 AM on September 11, 2012:

Yes, that's a linuxism. That's it's best to add a build flag for BSD-ish systems. Make is not smart enough to auto-detect such details about the system.

jrmithdobbs commented at 6:17 AM on September 11, 2012:

Maybe add a new environment variable for determining if this is necessary?

For example (above the -l z -l pthread def):

ifeq($(LMODE2), dynamic) ifdef NEEDS_DL LIBS += -l dl endif endif

...

alternatively that could be ifndef BSD_SOURCE ... but that means you'd have to give the bsd flag when/if anyone ever gets to porting to solaris/aix/ux which seems quite counterintuitive.

luke-jr commented at 6:24 AM on September 11, 2012:

GNU Make at least is smart enough; does BSD Make not have $(shell ...)?

jrmithdobbs commented at 6:30 AM on September 11, 2012:

This requires gmake as is any how, see build examples. :)

$(shell ...) is just .... ugly

I guess

ifeq($(LMODE2), dynamic) ifeq($(shell uname -s), Linux) .....

Would be another option?

luke-jr commented at 6:34 AM on September 11, 2012:

Is libdl really a Linux-specific thing? I'd think if anything it'd be GNU-specific - so still needed on Debian GNU/kFreeBSD?

jrmithdobbs commented at 6:39 AM on September 11, 2012:

Good question. I'm not sure. Does anyone actually have a debian bastardized kfreebsd system to test on?

jrmithdobbs commented at 7:14 AM on September 11, 2012:

I did want to note that I discussed removing the -ldl with gmaxwell / sipa and a few others before submitting this pull.

Are static builds still being used? They not for the official binaries, to my understanding at least.

laanwj commented at 7:39 AM on September 11, 2012:

Yes, static builds are being used. And libdl is not only linux. I'm on a solaris system right now that has it.

jrmithdobbs commented at 7:36 AM on April 8, 2013:

Does anyone know of a good way to tell whether a platform needs -ldl ? pkg-config maybe?

laanwj commented at 5:37 AM on September 11, 2012: member

I think it's best to a OPENBSD=1 option to the makefile for the OpenBSD-specific changes (unless there's a way to detect that automagically...).

jrmithdobbs commented at 5:41 AM on September 11, 2012: contributor

They're not openbsd specific, they apply to both (and probably netbsd) platforms. The db thing is weird and can occur on other platforms from what I can tell.

I moved the AI_ADDRCONFIG def into compat.h where it should be. Nice catch.

laanwj commented at 5:42 AM on September 11, 2012: member

BSD=1 then...

jrmithdobbs commented at 5:43 AM on September 11, 2012: contributor

BSD is already a reserved define since BSD4.3. (It's a version number, i forget details of parsing it.)

jrmithdobbs commented at 5:47 AM on September 11, 2012: contributor

The pthread_np.h header stands for "non portable," I know that function exists on those two platforms in the expected form, I don't know if there's something better to key off of for it.

Fixes for building on FreeBSD and OpenBSD. OpenBSD should not be used as a wallet yet. e56b181ffc

jrmithdobbs commented at 6:08 AM on September 11, 2012: contributor

Cleaned up and clarified some comments.

laanwj commented at 6:15 AM on September 11, 2012: member

I don't really care what specific flag you use, but you should put the OBSD specific changes (especially the -ldl) behind some flag so that build on Linux is unaffected.

jrmithdobbs commented at 6:22 AM on September 11, 2012: contributor

The stuff in the makefile doesn't have access to the things that define things like _____FreeBSD_____, etc since they're in included headers is the bigger problem. I'm trying to hack it in as cleanly as possible without resorting to autotools because that's a big change. (and last time that got left to rot when jaromil did it)

Trying to make as few changes as possible to attempt to get this fixed, at least for freebsd (don't know that i'll have time to track down the openbsd wallet issues), before 0.7 final.

BitcoinPullTester commented at 3:56 AM on September 12, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e56b181ffc35c6eaf488fd09fc0948c2ea722158 for binaries and test log.

jgarzik commented at 2:40 AM on November 16, 2012: contributor

ACK, seems sane, needs rebase.

Maybe HAVE_CXX_STDHEADERS should be conditionalized for the platforms that need it.

luke-jr commented at 5:32 AM on April 8, 2013: member

Needs rebase.

jgarzik commented at 5:05 PM on May 30, 2013: contributor

Closing due to inattention. If you want to rebase, feel free to re-open.

jgarzik closed this on May 30, 2013

KolbyML referenced this in commit 0fa40d7695 on Dec 5, 2020

KolbyML referenced this in commit 7b2b9d048e on Dec 5, 2020

KolbyML referenced this in commit 8e7fa721af on Dec 5, 2020

DrahtBot locked this on Sep 8, 2021