< bitcoin-git> [bitcoin] gwillen opened pull request #19356: build: Fix & improve the search for BDB (master...feature-bdb-search-fix-osx) https://github.com/bitcoin/bitcoin/pull/19356
< gwillen> if anybody loves (or hates) autotools, I would love a look at the PR I just opened ^
< gwillen> I made one change that I actually need to fix a specific issue on my OS X system, and some general cleanup that I will remove if it's controversial.
< * sipa> summons cfields ^
< gwillen> if anybody remembers my comment about mysterious segfaults when building another PR, this PR is the punchline (without it, I end up getting headers from one version of bdb, and libs from another version.)
< cfields> gwillen: thanks. Looks more correct at a glance, will look in detail tomorrow.
< gwillen> thanks very much for the quick look cfields!
< * cfields> considers picking a CFLAGS fight just because gwillen seems to want it
< cfields> Joking, of course. I agree with you. It's just sometimes hard to balance the fact that everyone wants CFLAGS as a hammer.
< * sipa> idly suggests switching to maven
< cfields> sipa: patches welcome!
< fanquake> 😢
< cfields> (comprehensive patches :p)
< sipa> haha
< gwillen> cfields: yeah I am happy to relinquish the CFLAGS/CPPFLAGS part of the change if that will make life easier, but I figured I'd try for a cleanup while I was poking around
< gwillen> I can understand why people reach for CFLAGS, it is all over documentation everywhere, I would have done the same but I learned the difference in reading the documentation while making this change :-)
< cfields> gwillen: the reason I think I'm going to have to fight you on it is because pkg-config uses the _CFLAGS convention. So if you do "./configure --help", all of the paths are _CFLAGS. Which is really unfortunate, because _CPPFLAGS would make much more sense for those.
< cfields> But I'm not completely sure what your problem is independent of the _CFLAGS change. Is it just that the include-order is wrong in the brew case?
< gwillen> cfields: correct, the change from CFLAGS/LIBS to BDB_CFLAGS/BDB_LIBS fixes the include order in the brew case, it pushes the BDB 4.8 include earlier, to override the inclusion of /usr/local/include elsewhere in the list of paths, which pulls in all kinds of random stuff
< gwillen> whereas the CFLAGS/CPPFLAGS change is just for fun
< gwillen> I confess I don't know much at all about pkg-config, I will read up on that
< cfields> gwillen: many projects (like us) use pkg-config to add dependencies. Doing so automatically hooks up a FOO_CFLAGS and FOO_LIBS that can be used to supply the respective paths for that dependency.
< cfields> gwillen: so when you run "./configure --help" you see those possible overrides. That's why the _CFLAGS convention is so (annoyingly) common for specifying an include path.
< cfields> So while wishing to pass BDB_CPPFLAGS is *far* more correct, it's not what anything expects.
< gwillen> yeah that makes sense, I see that at some point pkg-config apparently grew the ability to deal with CPPFLAGS, but late enough that it's probably locked in at this point
< gwillen> let me fix it up to work the other way, I think there is a small annoyance I will have to deal with in the process and you can review the result
< cfields> gwillen: it's up to package maintainers to supply their own flags...
< cfields> as a data point, we don't use cppflags: https://github.com/bitcoin/bitcoin/blob/master/libbitcoinconsensus.pc.in
< cfields> (maybe we should :)
< luke-jr> libbitcoinconsensus is a C library..
< sipa> cpp = c preprocessor
< cfields> luke-jr: CPP == C PreProcessor
< sipa> cxxflags is for c++ flags
< gwillen> well the thing that makes it annoying is that automake does require it to be CPPFLAGS for BDB in particular, or CXXFLAGS, since we need it to be present for C++ compilation
< luke-jr> the spec seems to say there is no cppflags for pkg-config
< gwillen> I'm curious how other projects that use pkg-config with C++ deal with this
< cfields> gwillen: afaik they all just assume paths are coming in via cflags.
< luke-jr> the autoconf stuff for pkg-config also seems to be CFLAGS/LIBS only
< luke-jr> "The flags encountered in Cflags are grouped into the two following groups: the -I (include path) flags, …"
< cfields> I seem to recall getting really grumpy at some project for including an actual cflag there.
< luke-jr> so yes, pkg-config cflags are *supposed* to include the -Is
< gwillen> but automake does not feed CFLAGS to the C++ compiler
< gwillen> so if you're building C++, you have to bridge that somehow
< luke-jr> gwillen: it doesn't feed *_C*FLAGS to it either - you need to explicitly include those somewhere
< luke-jr> not even for C
< luke-jr> as much as it might makes sense to say CFLAGS = concat(*_CFLAGS), it really doesn't work that way <.<
< cfields> gwillen: as an example of how it works...
< cfields> PKG_CHECK_MODULES([UNIVALUE],[libunivalue >= 1.0.4],[found_univalue=yes],[true])
< cfields> That creates UNIVALUE_CFLAGS and UNIVALUE_LIBS for us
< gwillen> luke-jr: it doesn't automatically include BDB_CPPFLAGS, but it does automatically include libbitcoin_server_a_CPPFLAGS, which is what we put BDB_CPPFLAGS into
< luke-jr> gwillen: point is, DEP_CFLAGS is supposed to be the place for -I and such
< gwillen> anyway, I guess the answer is that someone somewhere stuffs the contents of CFLAGS into something named CPPFLAGS somewhere
< cfields> gwillen: yep, that.
< gwillen> hmmm actually I guess I understand luke-jr's point
< gwillen> although I'm not sure if it quite makes sense for BDB which is itself a C++ library
< luke-jr> gwillen: this is also how C++ libraries do it.. <.<
< luke-jr> there is no Cxxflags in pkg-config either
< gwillen> so basically pkg-config and automake just like, permanently differ on this, I guess?
< luke-jr> seems so
< gwillen> (e.g. ax_boost_base.m4 defines AC_SUBST(BOOST_CPPFLAGS), not BOOST_CFLAGS, for where it finds boost)
< cfields> gwillen: I don't see those exported.
< cfields> also, boost doesn't ship pkg-config files. Which is why we carry that monstrosity of a .m4.
< gwillen> anyway this just confirms my existing hatred for build systems :D
< luke-jr> could be worse - could be maven
< * luke-jr> hides
< cfields> gwillen: right, but that's available as an override (not visible externally via ./configure --help)
< gwillen> ahh, yeah, because it is AC_SUBST and not AC_ARG_VAR
< cfields> right
< luke-jr> what I really don't get is how there are so many efforts to replace autoconf, and they always end up worse
< cfields> *that's not availble. sorry. apparently you understood :)
< sipa> xkcd 927 lalala
< fanquake> It seems my Windows VM has "expired", and now shuts itself down ~ every 45 minutes.
< fanquake> Incredibly convenient
< luke-jr> I bet illicit sources don't have that problem
< fanquake> heh
< fanquake> I will persevere while I wait for the 16GB of "new" VM to download
< bitcoin-git> [bitcoin] jonatack opened pull request #19357: doc: add release note for bitcoin-cli -generate (master...cli-generate-release-note) https://github.com/bitcoin/bitcoin/pull/19357
< fanquake> I think I have figured out the underlying issue with ipv6 & Windows (#18287)
< gribble> https://github.com/bitcoin/bitcoin/issues/18287 | depends: Patch libevent build to fix IPv6 -rpcbind on Windows by luke-jr · Pull Request #18287 · bitcoin/bitcoin · GitHub
< fanquake> The problem is deeper inside libevent, but hopefully can get that fixed up for 0.20.1
< luke-jr> fanquake: ?
< fanquake> luke-jr: the problem seems to be with the “hint” flags that libevent is setting before getaddr* calls. The changes in the PR don’t fix the issue for us entirely, as mentioned in the comments there.
< luke-jr> hrm, been too long since I made that to remember about hint stuff :/
< luke-jr> strange that it fixes it for some of us and not others
< bitcoin-git> [bitcoin] Saibato opened pull request #19358: net: Make sure we do not override proxy settings in hidden service. (master...pr351-bitcoin) https://github.com/bitcoin/bitcoin/pull/19358
< bitcoin-git> [bitcoin] fanquake pushed 9 commits to master: https://github.com/bitcoin/bitcoin/compare/e3fa3c7d671e...80fd474e402b
< bitcoin-git> bitcoin/master b3394ab Carl Dong: contrib: macdeploy: Correctly generate macOS SDK
< bitcoin-git> bitcoin/master 3381e4a Carl Dong: Adapt rest of tooling to new SDK naming scheme
< bitcoin-git> bitcoin/master fbcfcf6 Carl Dong: native_cctools: Don't use libc++ from pinned clang
< bitcoin-git> [bitcoin] fanquake merged pull request #19240: build: macOS toolchain simplification and bump (master...2020-06-macos-sdkgen-simplify) https://github.com/bitcoin/bitcoin/pull/19240
< bitcoin-git> [bitcoin] vasild opened pull request #19360: net: improve encapsulation of CNetAddr (master...improve_encapsulation_of_cnetaddr) https://github.com/bitcoin/bitcoin/pull/19360
< bitcoin-git> [bitcoin] prusnak opened pull request #19362: rpc/blockchain: Reset scantxoutset progress before inferring descriptors (master...rpc-scantxoutset-reset-progress) https://github.com/bitcoin/bitcoin/pull/19362
< sipa> MarcoFalke: i'm finding it a bit annoying that codebase changes often break existing fuzz tests, which require travis or an entirely separate build to find out
< sipa> what do you think about "building" the fuzz tests in normal make all mode (but without the actual fuzzing enabled)
< sipa> one caveat is that the fuzz tests currently already need c++17
< bitcoin-git> [bitcoin] hebasto closed pull request #18710: Add local thread pool to CCheckQueue (master...200419-thread-pool) https://github.com/bitcoin/bitcoin/pull/18710
< bitcoin-git> [bitcoin] hebasto reopened pull request #18710: Add local thread pool to CCheckQueue (master...200419-thread-pool) https://github.com/bitcoin/bitcoin/pull/18710
< cfields> sipa / MarcoFalke: See top 2 commits here for how that could work: https://github.com/theuni/bitcoin/commits/build-fuzz
< wumpus> sipa: I'm not sure it should be the default, but I do agree it should be easier (e.g. a single configure option) to build the fuzz tests as well sa everything else
< wumpus> having to do a second build with either-or exclusive options is kind of a time sink and easy to forget
< sipa> wumpus: it seems that building the actual fuzz tests within one configure is very hard
< wumpus> okay
< sipa> but just something that can test whether the fuzz tests build is great already, i think
< sipa> because it's really easy to introduce compile errors in them (and this will likely worsen as their coverage increases)
< wumpus> right
< sipa> we could even have a minimal main() for the fake-fuzz tests that reads a single input from stdin, and runs the test code on it
< sipa> or all entries in a directory... meaning it'd be actually useful as a test harness; you just wouldn't be able to fuzz with it
< wumpus> that's a neat idea, at least the building feels less like dead weight in that case
< sipa> as the travis fuzz build isn't actually doing any fuzzing, this may even be sufficient there
< bitcoin-git> [bitcoin] jnewbery opened pull request #19364: net processing: Move orphan reprocessing to a global (master...2020-06-global-orphans) https://github.com/bitcoin/bitcoin/pull/19364
< cfields> it seems getting them to link as real progs will have linker issues due to missing libs. I guess the linker doesn't go looking for unresolved symbols with no main?
< sipa> cfields: hmm, what symbols are missing?
< cfields> sipa: zmq/miniupnpc/etc.
< sipa> oh
< sipa> that sounds fixable?
< cfields> We could tack them on, but that kinda sucks.
< cfields> Yeah, there's another approach though to avoid linking altogether though, 1 sec.
< sipa> or maybe it's just not an issue
< sipa> because if it's not a problem for real fuzzing, why would it here?
< sipa> perhaps it's due to not having main() indeed
< cfields> That one's verbose though, requires us to c/p the object names to that list.
< sipa> that's somewhat annoying (it's already annoying to need to add a blob to the makefile for every new fuzz test)
< sipa> but not terrible i guess
< cfields> I'll play with the linker errors a bit, see if there's a quick fix.
< wumpus> even just compiling and not linking would find the largest part of the errors introduced by changes and drift i guess
< wumpus> link-time errors are kind of rare
< luke-jr> in case I overlooked any remaining value, would be good to get extra eyes on https://github.com/bitcoin-core/bitcoincore.org/pull/707
< luke-jr> sipa: instagibbs ^
< sipa> luke-jr: it depends, i think
< sipa> though it can certainly use a note at least that it's subject to users noticing they're being asked to sign multiple times
< luke-jr> sipa: they might think they're only signing twice to 0.0001 BTC txs, and figure "meh, worst case I lose 0.0002 BTC"
< luke-jr> (while the fee could end up being 100 BTC or whatever)
< sipa> a weaker benefit also remains: you can sign without transfering all txn-creating-outputs-being-spent, in case you know they're correct
< sipa> while pre-segwit you just had to have the full transactions at signing time
< bitcoin-git> [bitcoin] practicalswift opened pull request #19366: tests: Provide main(...) function in fuzzer. Allow building uninstrumented harnesses with --enable-fuzz. (master...provide-main-function-in-fuzzer) https://github.com/bitcoin/bitcoin/pull/19366
< luke-jr> sipa: I don't understand what you mean
< sipa> luke-jr: nvm, i'm wrong
< luke-jr> sipa: there is a request to make a revision to taproot, and call it tarproot.
< sipa> ha
< fanquake> sipa / wumpus: can you block kkarimalami if you haven’t already
< sipa> fanquake: i already did