build: Drop bitcoin-tx and bitcoin-wallet dependencies on libevent

ryanofsky commented at 1:02 pm on April 2, 2020: member

This fixes compile errors trying to build bitcoin-tx and bitcoin-wallet without libevent, which were reported by Luke Dashjr in #18465

The fix avoiding bitcoin-tx dependency on libevent just adds a conditional build rule. This is implemented in the first commit (more details in commit description).

The fix avoiding bitcoin-wallet dependency on libevent requires minor code changes, because bitcoin-wallet (unlike bitcoin-tx) links against code that calls urlDecode / evhttp_uridecode. This fix is implemented in the second commit (again details in the commit description).

Drop unintended bitcoin-tx dependency on libevent

Don't include util/url.cpp to libbitcoin_util.a when libevent isn't available.
This fixes a compile error trying to build bitcoin-tx without libevent reported
by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465

Fixes #18465

0660119ac3

Drop bitcoin-wallet dependency on libevent

Don't require urlDecode function in wallet code since urlDecode implementation
currently uses libevent. Just call urlDecode indirectly though URL_DECODE
function pointer constant if available.

In bitcoind and bitcoin-qt, URL_DECODE is implemented and used to interpret RPC
wallet requests. In bitcoin-wallet, URL_DECODE is null to avoid depending on
libevent.

01a3392b1b

fanquake added the label Build system on Apr 2, 2020

MarcoFalke added the label Needs gitian build on Apr 2, 2020

ryanofsky force-pushed on Apr 3, 2020

ryanofsky commented at 1:49 pm on April 3, 2020: member

Updated ddfe37302dcbf93e031704d5eb8af3c508f71f85 -> 01a3392b1b778fa4fcf568013326d6ea1de4fb3b (pr/dep-libevent.1 -> pr/dep-libevent.2, compare) with msvc fix

DrahtBot commented at 5:58 pm on April 3, 2020: member

Gitian builds

File	commit dce6f3b29b4197b8eea91f5919d56eb222ea8959(master)	commit 8a62b9f52bdedb31ee9603983b813e9d5c4110bd(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`4fba473976a3a85e...`	`0d64e50e2d3f59f1...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`c135739e69c4472d...`	`e30313e60ae497ec...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`06bb06c0f823bf90...`	`1f32dc21b2b701b5...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`7734e4d5cbcb4825...`	`bc88b45b1e981d67...`
bitcoin-0.19.99-osx-unsigned.dmg	`0b344910a70199be...`	`a857203c058c0b26...`
bitcoin-0.19.99-osx64.tar.gz	`46e9c05f70086290...`	`eac6982eb274eebd...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`7831492494057bba...`	`535f6b1abe9bcc58...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`f83ac254cfddb085...`	`9461af2417ad155b...`
bitcoin-0.19.99-win64-debug.zip	`604e4c5e227d50f0...`	`ff4e0bc3d6086b09...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`b7fc22b59cfd6f6d...`	`69d02bb813f8a712...`
bitcoin-0.19.99-win64.zip	`f2836d8a06e7d42c...`	`2456958b486469ab...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`75df0e07d2339760...`	`5999775aa35e1fce...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`f3a6db85e94d7cff...`	`06b0f9039cfaae4d...`
bitcoin-0.19.99.tar.gz	`b478cb46b9bb5433...`	`a1a382d30fbbb817...`
bitcoin-core-linux-0.20-res.yml	`ccc64b0731859e36...`	`0de1fdbeba9f53f7...`
bitcoin-core-osx-0.20-res.yml	`af46833e50fab798...`	`d0a755fa1d01a73a...`
bitcoin-core-win-0.20-res.yml	`aa9152d74d049f56...`	`2e51f6b663bb34e3...`
linux-build.log	`984685556f83f6c0...`	`fef31aab904b165a...`
osx-build.log	`1e5a6165090ecbec...`	`67449ea283c6fb2e...`
win-build.log	`7472b49c6539ee86...`	`65255c959c9db8d6...`
bitcoin-core-linux-0.20-res.yml.diff		`6033300b201259e3...`
bitcoin-core-osx-0.20-res.yml.diff		`5cb3edb9f1c0ff19...`
bitcoin-core-win-0.20-res.yml.diff		`8991f12e402dc537...`
linux-build.log.diff		`1701dfe791eb446e...`
osx-build.log.diff		`f5ecaa77996b17c5...`
win-build.log.diff		`aa1c34d8e895a371...`

DrahtBot removed the label Needs gitian build on Apr 3, 2020

jonasschnelli commented at 11:08 am on April 8, 2020: contributor

utACK 01a3392b1b778fa4fcf568013326d6ea1de4fb3b. Gitian Build: https://bitcoin.jonasschnelli.ch/gitian/build/74

MarcoFalke commented at 12:30 pm on April 8, 2020: member

cc @luke-jr

luke-jr approved

luke-jr commented at 5:08 pm on April 8, 2020: member

Kind of ugly, but utACK

Nit: URL_DECODE should probably be named g_url_decode_fn or something…

ryanofsky commented at 5:34 pm on April 8, 2020: member

Nit: URL_DECODE should probably be named g_url_decode_fn or something…

Constant naming is used because it is a constant, not a global variable

MarcoFalke commented at 5:55 pm on April 8, 2020: member

Constant naming is used because it is a constant, not a global variable

G_URL_DECODE like G_TRANSLATION_FUN :grimacing: ?

luke-jr commented at 6:02 pm on April 8, 2020: member

G_URL_DECODE_FUN sounds better in that case…

But it’s not really a constant - its value varies between builds.

MarcoFalke commented at 6:15 pm on April 8, 2020: member

The docs say “compile time constants”, not “build time constants”

MarcoFalke commented at 6:16 pm on April 8, 2020: member

Anyway, I was mostly joking about that style nit. Let’s not hold back this pull based on naming guidelines.

luke-jr commented at 6:20 pm on April 8, 2020: member

It’s definitely not a compile time constant! ;)

ryanofsky commented at 7:12 pm on April 8, 2020: member

It’s definitely not a compile time constant! ;)

FWIW, following this up in #18568

practicalswift commented at 7:34 pm on April 8, 2020: contributor

Concept ACK, but wouldn’t it be easier to implement std::string urlDecode(const std::string &urlEncoded) without evhttp_uridecode?

It has always seemed weird to me that we depend on third-party code for such a trivial function :)

DrahtBot commented at 6:28 am on April 9, 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:

#18571 (fuzz: Disable debug log file by MarcoFalke)

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.

MarcoFalke merged this on Apr 10, 2020

MarcoFalke closed this on Apr 10, 2020

in src/wallet/rpcwallet.cpp:80 in 01a3392b1b

76@@ -77,9 +77,9 @@ bool HaveKey(const SigningProvider& wallet, const CKey& key)
77 
78 bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name)
79 {
80-    if (request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) {
81+    if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) {

sidhujag commented at 7:11 pm on April 13, 2020:

Shouldn’t this be #ifdef URL_DECODE, ie: on bitcoin-tx the symbol does not exist if libevent is not used.

MarcoFalke commented at 7:26 pm on April 13, 2020:

The symbol URL_DECODE is always defined

sidhujag commented at 3:25 am on April 14, 2020:

my bad, thx

sidhujag referenced this in commit 32d6ead51a on Apr 13, 2020

PhotoshiNakamoto referenced this in commit f7bcb3d341 on Dec 11, 2021

DrahtBot locked this on Feb 15, 2022

build: Drop bitcoin-tx and bitcoin-wallet dependencies on libevent #18504

Gitian builds

Conflicts