build: configure.ac and netbsd-build.md for NetBSD

midnightmagic commented at 11:26 am on July 2, 2020: contributor

At the moment these changes are not functional on master as of a few minutes ago, failing with an Abort trap in gmake check, but this can be fixed in another PR. These particular changes should be otherwise correct and back-portable to v0.20.0, and will end up with a working gmake check there.

Hopefully these changes will be of some use to the, apparently, other NetBSD user out there who is by now on an outdated version.

These build-netbsd and configure.ac changes are required to get bitcoin

working on NetBSD.

At the moment they are not functional on master as of a few minutes ago,
failing with an Abort trap in gmake check, but this can be fixed in another
PR. These particular changes should be correct and back-portable to
v0.20.0, and will end up with a working gmake check there.

Hopefully these changes will be of some use to the, apparently, other NetBSD
user out there who is by now on an outdated version.

58702a5c49

fanquake added the label Build system on Jul 2, 2020

midnightmagic commented at 3:30 am on July 3, 2020: contributor

At commit a24806c25d7a81a9c436de58eb5778d93abab16b, gmake check is now working just fine on my NetBSD test machine with the configure.ac changes in this PR..

in doc/build-netbsd.md:59 in 58702a5c49

61-    MAKE=gmake
62+
63+MAKE="gmake" \
64+CPPFLAGS="-I/usr/pkg/include" LDFLAGS="-L/usr/pkg/lib -Wl,-R/usr/pkg/lib" \
65+BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" BDB_CFLAGS="-I${BDB_PREFIX}/include" \
66+./configure --with-gui=no --with-boost=/usr/pkg

laanwj commented at 12:42 pm on July 9, 2020:

Why are you canging this around? In general we prefer putting the environment variables after the ./configure because then they are retained if configure is automatically re-run by the build system (e.g. after a git pul lthat changes one of the build system files).

midnightmagic commented at 7:31 pm on July 9, 2020:

I’m not stuck on why particular way, or order, of anything. It’s entirely a habit born of porting stuff to NetBSD, idea being to keep changes as unobtrusive and light as possible, and to leave the onus on the NetBSD side to maintain/make changes that are specific to NetBSD. I can switch it up if you think that’s better. I don’t think any of these things will (presumably) ever be run on anything but an end-user’s NetBSD machine who is doing a manual one-off build and carefully testing it by hand.

laanwj commented at 12:28 pm on July 15, 2020:

I’d prefer to keep it as-is, it’s the convention that we use everywhere and if there is no need to deviate from it on NetBSD, it’s better to keep it more or less the same. This makes maintenance easier for us.

midnightmagic commented at 3:02 am on July 24, 2020:

Yeah sure thing. I’m completely fine with that. Technically no need whatsoever. Purely habit.

fanquake added the label Waiting for author on Jul 21, 2020

in configure.ac:1020 in 58702a5c49

1014@@ -1015,11 +1015,18 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdint.h>
1015 
1016 dnl LevelDB platform checks
1017 AC_MSG_CHECKING(for fdatasync)
1018+case $host in
1019+  *netbsd*)
1020+AC_MSG_RESULT(no because NetBSD will not fdatasync on directories and this breaks leveldb); HAVE_FDATASYNC=0

luke-jr commented at 11:44 pm on July 23, 2020:

Isn’t there a better way to detect this condition?

luke-jr commented at 11:46 pm on July 23, 2020:

We use fdatasync on files in FileCommit. Currently, it checks __linux__ || __NetBSD__, but it really should be using HAVE_FDATASYNC defined here.

Some other solution seems desirable…

krytarowski commented at 1:32 am on July 24, 2020:

Is this a NetBSD bug?

midnightmagic commented at 2:46 am on July 24, 2020:

Yes; we can detect it by running fdatasync() on a directory that we know exists and observing the result. I’d be glad to rewrite it into an autoconf run-program, but things like includes are disparate across platforms for this and a generic program would likely only be functional on tested platforms. A check for the name of the platform (programmed logic) rather than detected behaviour was simpler and—I figured—perhaps less effort and less prone to failure modes. I’d hate to have corrected this behaviour on NetBSD and then made configure non-functional on a platform I don’t currently have access to. :-)

midnightmagic commented at 2:48 am on July 24, 2020:

This is not a NetBSD bug. The POSIX/SUSvX whatever it is now is being more liberally interpreted by Linux as supporting directory fdatasync, but arguably this is incorrect. NetBSD interprets conditions and/or functionality like this quite a bit more strictly than Linux does, and does so (somewhat) independently and without working to be compatible with Linux. (At least, it has in the past. I have no idea if this is “intended” behaviour but it’s been around for a really long time and they tend to be reluctant to change POSIX-y behaviour without e.g. rulings from on high..)

I can fix whatever you’’d like me to fix, in whatever capacity you’d prefer, including fiddling with the logic inside leveldb but “The NetBSD Way” is to absolutely minimize the patches required to get software going on NetBSD in order to reduce load and etc on upstream and make patches palatable. I’m fine with whatever you guys think is best.

luke-jr commented at 5:35 am on July 24, 2020:

I considered that, but it won’t work if we’re cross-compiling.

BTW, how does it break LevelDB?

midnightmagic commented at 9:27 pm on August 2, 2020:

LevelDB, if HAVE_FDATASYNC is enabled, attempts it on full directories. When bitcoind attempts to commit data to (any) leveldb it issues fdatasync on a directory, which fails and generates an incomprehensible error for the user. bitcoind then exits. Without invasive changes in the leveldb code which special-case NetBSD (or.. platforms where fdatasync on a directory won’t work) then addressing this “properly” won’t happen. Easier to just disable it.

luke-jr changes_requested

fanquake referenced this in commit 1d8338d6b7 on Jul 28, 2020

laanwj referenced this in commit e3272ff290 on Aug 5, 2020

sidhujag referenced this in commit 8e779803bf on Aug 5, 2020

Warchant referenced this in commit ab08baf576 on Aug 6, 2020

DrahtBot commented at 10:04 pm on August 20, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19803 (Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDB by luke-jr)
#14501 (Fix possible data race when committing block files by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Needs rebase on Aug 31, 2020

DrahtBot commented at 6:08 am on August 31, 2020: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

in doc/build-netbsd.md:72 in 58702a5c49

81+BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" BDB_CFLAGS="-I${BDB_PREFIX}/include" \
82+./configure --with-gui=no --with-boost=/usr/pkg --disable-wallet
83 ```
84 
85-Build and run the tests:
86+Build and run the tests (very important!):

fanquake commented at 2:06 pm on April 1, 2021:

I think you can drop this addition as well.

fanquake commented at 2:08 pm on April 1, 2021: member

@midnightmagic are you planning on following up here? Looks like after some revertions, the changes here are essentially adding devel/ to the packages list, and the build system change, assuming it’s still required.

midnightmagic commented at 3:51 pm on April 1, 2021: contributor

It was required at the time, yes. I no longer have machines on which this can be tested. It will have to wait until I have rebuilt that section of my infrastructure.

fanquake commented at 6:31 am on May 31, 2021: member

@midnightmagic if you get back to testing this, let us know, and this can be reopened. I’m going to close this for now.

fanquake closed this on May 31, 2021

PastaPastaPasta referenced this in commit 98c0cbf0ba on Sep 17, 2021

PastaPastaPasta referenced this in commit 682dba0245 on Sep 19, 2021

thelazier referenced this in commit 9a21e410f6 on Sep 25, 2021

DrahtBot locked this on Aug 18, 2022

build: configure.ac and netbsd-build.md for NetBSD #19430

Conflicts