build: Create test utility library from src/test/util/

brakmic commented at 8:50 pm on November 20, 2019: contributor

This PR creates a static test utility library that replaces repetitive compilations of sources from src/test/util in unit, gui and bench tests.

The original issue is here: #17401

The changes are:

a new Makefile.test_util.include
a new entry in Makefile.am that includes Makefile.test_util.include when testing is enabled
removal of all src/test/util headers & sources from unit, gui and bench Makefiles
addition of libtest_util.a at LDADD’s of every test

fanquake added the label Build system on Nov 20, 2019

fanquake added the label Tests on Nov 20, 2019

in src/Makefile.test.include:209 in b882599b85 outdated

205@@ -219,169 +206,169 @@ test_fuzz_block_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp
206 test_fuzz_block_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DBLOCK_DESERIALIZE=1
207 test_fuzz_block_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
208 test_fuzz_block_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
209-test_fuzz_block_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON)
210+test_fuzz_block_deserialize_LDADD = $(LIBTEST_UTIL) $(FUZZ_SUITE_LD_COMMON)

MarcoFalke commented at 9:24 pm on November 20, 2019:

For a smaller diff you could add this to FUZZ_SUITE_LD_COMMON

brakmic commented at 9:37 pm on November 20, 2019:

Yes, of course. Will update the Makefile.test.include.

MarcoFalke approved

MarcoFalke commented at 9:25 pm on November 20, 2019: member

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

practicalswift commented at 9:33 pm on November 20, 2019: contributor

Concept ACK

Welcome as a contributor @brakmic! Hope to see more great contributions from you in the future!

brakmic commented at 9:48 pm on November 20, 2019: contributor

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

I must admit that I’m not sure what the best way is to link another lib to libtest_util.a.

brakmic commented at 9:48 pm on November 20, 2019: contributor

Concept ACK

Welcome as a contributor @brakmic! Hope to see more great contributions from you in the future!

Many thanks! :)

brakmic commented at 9:59 pm on November 20, 2019: contributor

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

I’ll try it with LIBADD.

–EDIT: the linking seems to be working now.

MarcoFalke commented at 7:13 pm on November 21, 2019: member

Could squash the commits into one with

0git reset --soft b4a1da9ef8e4b673c290d5b882527e627ae1b43a
1git commit -m "build: Create test utility library from src/test/util/"
2git push ... -f

in src/Makefile.am:720 in 1ff8477dd7 outdated

717@@ -718,6 +718,7 @@ include Makefile.leveldb.include
718 endif
719 
720 if ENABLE_TESTS
721+include Makefile.test_util.include

MarcoFalke commented at 7:21 pm on November 21, 2019:

Not sure if it makes sense to guard it by ENABLE_TESTS. If I just want to build the benchmarks with ./configure --disable-tests, then my build fails:

 0$ make -j 9
 1Making all in src
 2make[1]: Entering directory '/home/marco/workspace/btc_bitcoin_core/src'
 3make[2]: Entering directory '/home/marco/workspace/btc_bitcoin_core/src'
 4make[3]: Entering directory '/home/marco/workspace/btc_bitcoin_core'
 5make[3]: Leaving directory '/home/marco/workspace/btc_bitcoin_core'
 6  CXXLD    bench/bench_bitcoin
 7/usr/bin/ld: bench/bench_bitcoin-bench.o: in function `benchmark::BenchRunner::RunAll(benchmark::Printer&, unsigned long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool)':
 8/home/marco/workspace/btc_bitcoin_core/src/./test/util/setup_common.h:92: undefined reference to `TestingSetup::~TestingSetup()'
 9/usr/bin/ld: /home/marco/workspace/btc_bitcoin_core/src/./test/util/setup_common.h:94: undefined reference to `TestingSetup::TestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
10/usr/bin/ld: bench/bench_bitcoin-bench.o: in function `benchmark::BenchRunner::RunAll(benchmark::Printer&, unsigned long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) [clone .cold]':
11/home/marco/workspace/btc_bitcoin_core/src/./test/util/setup_common.h:92: undefined reference to `TestingSetup::~TestingSetup()'
12/usr/bin/ld: bench/bench_bitcoin-verify_script.o: in function `VerifyScriptBench(benchmark::State&)':
13/home/marco/workspace/btc_bitcoin_core/src/bench/verify_script.cpp:40: undefined reference to `BuildCreditingTransaction(CScript const&, int)'
14/usr/bin/ld: /home/marco/workspace/btc_bitcoin_core/src/bench/verify_script.cpp:41: undefined reference to `BuildSpendingTransaction(CScript const&, CScriptWitness const&, CTransaction const&)'
15/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-timedata.o): in function `_(char const*)':
16/home/marco/workspace/btc_bitcoin_core/src/./util/translation.h:38: undefined reference to `G_TRANSLATION_FUN[abi:cxx11]'
17/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-timedata.o): in function `AddTimeData(CNetAddr const&, long)':
18/usr/include/c++/9/bits/std_function.h:690: undefined reference to `G_TRANSLATION_FUN[abi:cxx11]'
19/usr/bin/ld: /usr/include/c++/9/bits/std_function.h:690: undefined reference to `G_TRANSLATION_FUN[abi:cxx11]'
20/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-txdb.o): in function `_(char const*)':
21/home/marco/workspace/btc_bitcoin_core/src/./util/translation.h:38: undefined reference to `G_TRANSLATION_FUN[abi:cxx11]'
22/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-txdb.o): in function `CCoinsViewDB::Upgrade()':
23/usr/include/c++/9/bits/std_function.h:690: undefined reference to `G_TRANSLATION_FUN[abi:cxx11]'
24/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-txdb.o):/usr/include/c++/9/bits/std_function.h:690: more undefined references to `G_TRANSLATION_FUN[abi:cxx11]' follow
25collect2: error: ld returned 1 exit status

brakmic commented at 7:24 pm on November 21, 2019:

Then I could either repetitively add it into every ENABLE_ guard (bench, qttest, unit) or simply include it unconditionally.

MarcoFalke commented at 7:26 pm on November 21, 2019:

Is the lib going to be compiled if it is included unconditionally?

Edit: If so, that would be unwanted, I think.

brakmic commented at 7:35 pm on November 21, 2019:

Or we can include the Makefile.test_util.include in qttest, unit and bench Makefiles?

MarcoFalke commented at 7:41 pm on November 21, 2019:

I tested locally and it seemed that it wouldn’t be compiled if it was included unconditionally. If you can confirm my findings, I think this is a fine approach.

brakmic commented at 7:43 pm on November 21, 2019:

I am recompiling everything again, just to be sure.

brakmic commented at 8:07 pm on November 21, 2019:

When compiled with –disable-tests –disable-bench no libtest_util.a is generated. With –disable-tests –enable-bench libtest_util.a gets generated.

MarcoFalke commented at 8:09 pm on November 21, 2019:

Yeah, that is what we want, right?

brakmic commented at 8:10 pm on November 21, 2019:

Yes, of course. Libs only when needed. :) Should I commit & squash again?

MarcoFalke commented at 8:11 pm on November 21, 2019:

Ja, bitte.

brakmic commented at 8:13 pm on November 21, 2019:

Erledigt.

build: Create test utility library from src/test/util/ a2e581de94

MarcoFalke commented at 8:15 pm on November 21, 2019: member

ACK a2e581de942ece85b84c4f5a15287654b02867cb 🍞

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK a2e581de942ece85b84c4f5a15287654b02867cb 🍞
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYEACgkQzit1aX5p
 7pUjDOQv+OyAcqDgHcIOr0IFlXmYp9mHraRe2d32wREQdaWZWxV/6MEZCxf98Pcl3
 8DZDnrkuXw7EuONBXKZj6zTTEAnL1Q87FMyZYRTWlcn5py0WlT7CbOxCP66nteCDq
 9wq1HkMscLAIm6ymxGRBNCe45FbZVLT1GXcvb5Ylb1W2zJMOgmkwZfkESqdaN9MgZ
10g8Y7BljZdeEWJU8eXWbiJJiwipUMadWN4WU0azzfgvhVpxc86HZgMWblR3BICIYw
11gJ5y7oW/tddXubmgGlApY7SvZ1oAnn77PSLM0Prz0NJoPXJGFfJYsEOoEw4yBgcj
127bgOZnQv5qsGNk8wDQJHZDAISdKLCrMZn68hJ68didlMAyqtMXQTWFiQBn4br707
13VrksXVKLmovxTkcj9W5OYj9/yfYltb1wcEu72O3by22UbfKf707TpQleXGFWwHYB
14xfHimAbyv0AsGqktVp2XOguPVt8Uoni6sS0MjZXLsxFLYSa6WRNMB+ZXrnJVAWMt
15/C8rLkOp
16=a+eq
17-----END PGP SIGNATURE-----

Timestamp of file with hash 4cbf916b05be4c1f535990b2e54da8495aba2db3e8a27b1b66c3a76beb122412 -

MarcoFalke referenced this in commit b983e7e172 on Nov 22, 2019

MarcoFalke merged this on Nov 22, 2019

MarcoFalke closed this on Nov 22, 2019

brakmic deleted the branch on Nov 22, 2019

fanquake referenced this in commit 2c98e2024d on Nov 23, 2019

sidhujag referenced this in commit c7cea2356d on Nov 23, 2019

MarcoFalke referenced this in commit 94c6f2bba4 on Dec 16, 2019

sidhujag referenced this in commit d566bad3b3 on Dec 16, 2019

MarcoFalke referenced this in commit 96a30b98c9 on Apr 5, 2020

deadalnix referenced this in commit c4788eec78 on Jun 28, 2020

sidhujag referenced this in commit 54bf552cae on Nov 10, 2020

sidhujag referenced this in commit d346a17938 on Nov 10, 2020

DrahtBot locked this on Dec 16, 2021

build: Create test utility library from src/test/util/ #17542