Skip to content

Commit a7f28af

Browse files
fanquakefanquake
committed
Merge bitcoin#22646: build: tighter Univalue integration, remove --with-system-univalue
0f95247 Integrate univalue into our buildsystem (Cory Fields) 9b49ed6 Squashed 'src/univalue/' changes from 98fadc0909..a44caf65fe (fanquake) Pull request description: This PR more tightly integrates building Univalue into our build system. This follows the same approach we use for [LevelDB](https://github.com/bitcoin-core/leveldb/), ([`Makefile.leveldb.include`](https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.leveldb.include)), and [CRC32C](https://github.com/bitcoin-core/crc32c) ([`Makefile.crc32c.include`](https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include)), and will be the same approach we use for [minisketch](https://github.com/sipa/minisketch); see bitcoin#23114. This approach yields a number of benefits, including: * Faster configuration due to one less subconfigure being run during `./configure` i.e 22s with this PR vs 26s * Faster autoconf i.e 13s with this PR vs 17s * Improved caching * No more issues with compiler flags i.e bitcoin#12467 * More direct control means we can build exactly the objects we want There might be one argument against making this change, which is that builders should have the option to use "proper shared/system libraries". However, I think that falls down for a few reasons. The first being that we already don't support building with a number of system libraries (secp256k1, leveldb, crc32c); some for good reason. Univalue is really the odd one out at the moment. Note that the only fork of Core I'm aware of, that actively patches in support for using system libs, also explicitly marks them as ["DANGEROUS"](https://github.com/bitcoinknots/bitcoin/blob/a886811721ce66eb586871706b3f5dd27518ac3e/configure.ac#L1430) and ["NOT SUPPORTED"](https://github.com/bitcoinknots/bitcoin/blob/a886811721ce66eb586871706b3f5dd27518ac3e/configure.ac#L1312). So it would seem they exist more to satisfy a distro requirement, as opposed to something that anyone should, or would actually use in practice. PRs like bitcoin#22412 highlight the "issue" with us operating with our own Univalue fork, where we actively fix bugs, and make improvements, when upstream (https://github.com/jgarzik/univalue) may not be taking those improvements, and by all accounts, is not currently actively maintained. Bitcoin Core should not be hamstrung into not being able to fix bugs in a library, and/or have to litter our source with "workarounds", i.e bitcoin#22412, for bugs we've already fixed, based on the fact that an upstream project is not actively being maintained. Allowing builders to use system libs is really only exacerbating this problem, with little benefit to our project. Bitcoin Core is not quite like your average piece of distro packaged software. There is the potential for us to give the same treatment to libsecp256k1, however it seems doing that is currently less straightforward. ACKs for top commit: dongcarl: ACK 0f95247 less my comment above, always nice to have an include-able `sources.mk` which makes integration easier. theuni: ACK 0f95247. Thanks fanquake for keeping this going. Tree-SHA512: a7f2e41ee7cba06ae72388638e86b264eca1b9a8b81c15d1d7b45df960c88c3b91578b4ade020f8cc61d75cf8d16914575f9a78fa4cef9c12be63504ed804b99
2 parents 16df28c + 0f95247 commit a7f28af

22 files changed

+1401
-433
lines changed

‎ci/test/06_script_b.sh‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@ export LC_ALL=C.UTF-8
99
if [[ $HOST = *-mingw32 ]]; then
1010
# Generate all binaries, so that they can be wrapped
1111
DOCKER_EXEC make $MAKEJOBS -C src/secp256k1 VERBOSE=1
12-
DOCKER_EXEC make $MAKEJOBS -C src/univalue VERBOSE=1
1312
DOCKER_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-wine.sh"
1413
fi
1514

1615
if [ -n "$QEMU_USER_CMD" ]; then
1716
# Generate all binaries, so that they can be wrapped
1817
DOCKER_EXEC make $MAKEJOBS -C src/secp256k1 VERBOSE=1
19-
DOCKER_EXEC make $MAKEJOBS -C src/univalue VERBOSE=1
2018
DOCKER_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-qemu.sh"
2119
fi
2220

‎configure.ac‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -261,12 +261,6 @@ if test "x$use_asm" = xyes; then
261261
AC_DEFINE(USE_ASM, 1, [Define this symbol to build in assembly routines])
262262
fi
263263

264-
AC_ARG_WITH([system-univalue],
265-
[AS_HELP_STRING([--with-system-univalue],
266-
[Build with system UniValue (default is no)])],
267-
[system_univalue=$withval],
268-
[system_univalue=no]
269-
)
270264
AC_ARG_ENABLE([zmq],
271265
[AS_HELP_STRING([--disable-zmq],
272266
[disable ZMQ notifications])],
@@ -1522,34 +1516,6 @@ if test "x$use_zmq" = xyes; then
15221516
esac
15231517
fi
15241518

1525-
dnl univalue check
1526-
1527-
need_bundled_univalue=yes
1528-
if test x$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoin_util$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench = xnononononononono; then
1529-
need_bundled_univalue=no
1530-
else
1531-
if test x$system_univalue != xno; then
1532-
PKG_CHECK_MODULES([UNIVALUE], [libunivalue >= 1.0.4], [found_univalue=yes], [found_univalue=no])
1533-
if test x$found_univalue = xyes; then
1534-
system_univalue=yes
1535-
need_bundled_univalue=no
1536-
elif test x$system_univalue = xyes; then
1537-
AC_MSG_ERROR([univalue not found])
1538-
else
1539-
system_univalue=no
1540-
fi
1541-
fi
1542-
1543-
if test x$need_bundled_univalue = xyes; then
1544-
UNIVALUE_CFLAGS='-I$(srcdir)/univalue/include'
1545-
UNIVALUE_LIBS='univalue/libunivalue.la'
1546-
fi
1547-
fi
1548-
1549-
AM_CONDITIONAL([EMBEDDED_UNIVALUE],[test x$need_bundled_univalue = xyes])
1550-
AC_SUBST(UNIVALUE_CFLAGS)
1551-
AC_SUBST(UNIVALUE_LIBS)
1552-
15531519
dnl libmultiprocess library check
15541520

15551521
libmultiprocess_found=no
@@ -1912,10 +1878,6 @@ PKGCONFIG_LIBDIR_TEMP="$PKG_CONFIG_LIBDIR"
19121878
unset PKG_CONFIG_LIBDIR
19131879
PKG_CONFIG_LIBDIR="$PKGCONFIG_LIBDIR_TEMP"
19141880

1915-
if test x$need_bundled_univalue = xyes; then
1916-
AC_CONFIG_SUBDIRS([src/univalue])
1917-
fi
1918-
19191881
ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental"
19201882
AC_CONFIG_SUBDIRS([src/secp256k1])
19211883

‎contrib/guix/libexec/build.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ mkdir -p "$DISTSRC"
297297
${HOST_CXXFLAGS:+CXXFLAGS="${HOST_CXXFLAGS}"} \
298298
${HOST_LDFLAGS:+LDFLAGS="${HOST_LDFLAGS}"}
299299

300-
sed -i.old 's/-lstdc++ //g' config.status libtool src/univalue/config.status src/univalue/libtool
300+
sed -i.old 's/-lstdc++ //g' config.status libtool
301301

302302
# Build Bitcoin Core
303303
make --jobs="$JOBS" ${V:+V=1}

‎src/Makefile.am‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
print-%: FORCE
77
@echo '$*'='$($*)'
88

9-
DIST_SUBDIRS = secp256k1 univalue
9+
DIST_SUBDIRS = secp256k1
1010

1111
AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS)
1212
AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS)
@@ -15,18 +15,7 @@ AM_LIBTOOLFLAGS = --preserve-dup-deps
1515
PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS)
1616
EXTRA_LIBRARIES =
1717

18-
if EMBEDDED_UNIVALUE
19-
LIBUNIVALUE = univalue/libunivalue.la
20-
21-
$(LIBUNIVALUE): $(wildcard univalue/lib/*) $(wildcard univalue/include/*)
22-
$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C $(@D) $(@F)
23-
else
24-
LIBUNIVALUE = $(UNIVALUE_LIBS)
25-
endif
26-
27-
BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/secp256k1/include $(BDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS)
28-
29-
BITCOIN_INCLUDES += $(UNIVALUE_CFLAGS)
18+
BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/secp256k1/include -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) $(BDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS)
3019

3120
LIBBITCOIN_SERVER=libbitcoin_server.a
3221
LIBBITCOIN_COMMON=libbitcoin_common.a
@@ -80,6 +69,7 @@ EXTRA_LIBRARIES += \
8069
$(LIBBITCOIN_ZMQ)
8170

8271
lib_LTLIBRARIES = $(LIBBITCOINCONSENSUS)
72+
noinst_LTLIBRARIES =
8373

8474
bin_PROGRAMS =
8575
noinst_PROGRAMS =
@@ -797,7 +787,6 @@ $(top_srcdir)/$(subdir)/config/bitcoin-config.h.in: $(am__configure_deps)
797787

798788
clean-local:
799789
-$(MAKE) -C secp256k1 clean
800-
-$(MAKE) -C univalue clean
801790
-rm -f leveldb/*/*.gcda leveldb/*/*.gcno leveldb/helpers/memenv/*.gcda leveldb/helpers/memenv/*.gcno
802791
-rm -f config.h
803792
-rm -rf test/__pycache__
@@ -875,3 +864,5 @@ endif
875864
if ENABLE_QT_TESTS
876865
include Makefile.qttest.include
877866
endif
867+
868+
include Makefile.univalue.include

‎src/Makefile.test.include‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,26 @@ if ENABLE_BENCH
350350
endif
351351
endif
352352
$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check
353-
if EMBEDDED_UNIVALUE
354-
$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C univalue check
353+
354+
if !ENABLE_FUZZ
355+
UNIVALUE_TESTS = univalue/test/object univalue/test/unitester univalue/test/no_nul
356+
noinst_PROGRAMS += $(UNIVALUE_TESTS)
357+
TESTS += $(UNIVALUE_TESTS)
358+
359+
univalue_test_unitester_SOURCES = $(UNIVALUE_TEST_UNITESTER_INT)
360+
univalue_test_unitester_LDADD = $(LIBUNIVALUE)
361+
univalue_test_unitester_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) -DJSON_TEST_SRC=\"$(srcdir)/$(UNIVALUE_TEST_DATA_DIR_INT)\"
362+
univalue_test_unitester_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
363+
364+
univalue_test_no_nul_SOURCES = $(UNIVALUE_TEST_NO_NUL_INT)
365+
univalue_test_no_nul_LDADD = $(LIBUNIVALUE)
366+
univalue_test_no_nul_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
367+
univalue_test_no_nul_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
368+
369+
univalue_test_object_SOURCES = $(UNIVALUE_TEST_OBJECT_INT)
370+
univalue_test_object_LDADD = $(LIBUNIVALUE)
371+
univalue_test_object_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
372+
univalue_test_object_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
355373
endif
356374

357375
%.cpp.test: %.cpp

‎src/Makefile.univalue.include‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
include univalue/sources.mk
2+
3+
LIBUNIVALUE = libunivalue.la
4+
noinst_LTLIBRARIES += $(LIBUNIVALUE)
5+
libunivalue_la_SOURCES = $(UNIVALUE_LIB_SOURCES_INT) $(UNIVALUE_DIST_HEADERS_INT) $(UNIVALUE_LIB_HEADERS_INT) $(UNIVALUE_TEST_FILES_INT)
6+
libunivalue_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)

‎src/univalue/.cirrus.yml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
env:
2+
MAKEJOBS: "-j4"
3+
RUN_TESTS: "true"
4+
BASE_OUTDIR: "$CIRRUS_WORKING_DIR/out_dir_base"
5+
DEBIAN_FRONTEND: "noninteractive"
6+
7+
task:
8+
container:
9+
image: ubuntu:focal
10+
cpu: 1
11+
memory: 1G
12+
greedy: true # https://medium.com/cirruslabs/introducing-greedy-container-instances-29aad06dc2b4
13+
14+
matrix:
15+
- name: "gcc"
16+
env:
17+
CC: "gcc"
18+
CXX: "g++"
19+
APT_PKGS: "gcc"
20+
- name: "clang"
21+
env:
22+
CC: "clang"
23+
CXX: "clang++"
24+
APT_PKGS: "clang"
25+
- name: "mingw"
26+
env:
27+
CC: ""
28+
CXX: ""
29+
UNIVALUE_CONFIG: "--host=x86_64-w64-mingw32"
30+
APT_PKGS: "g++-mingw-w64-x86-64 gcc-mingw-w64-x86-64 binutils-mingw-w64-x86-64"
31+
RUN_TESTS: "false"
32+
33+
install_script:
34+
- apt update
35+
- apt install -y pkg-config build-essential libtool autotools-dev automake bsdmainutils
36+
- apt install -y $APT_PKGS
37+
autogen_script:
38+
- ./autogen.sh
39+
configure_script:
40+
- ./configure --cache-file=config.cache --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib $UNIVALUE_CONFIG
41+
make_script:
42+
- make $MAKEJOBS V=1
43+
test_script:
44+
- if [ "$RUN_TESTS" = "true" ]; then make $MAKEJOBS distcheck; fi

‎src/univalue/.travis.yml‎

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎src/univalue/Makefile.am‎

Lines changed: 15 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,17 @@
1+
include sources.mk
12
ACLOCAL_AMFLAGS = -I build-aux/m4
2-
.PHONY: gen
3+
.PHONY: gen FORCE
34
.INTERMEDIATE: $(GENBIN)
45

5-
include_HEADERS = include/univalue.h
6-
noinst_HEADERS = lib/univalue_escapes.h lib/univalue_utffilter.h
6+
include_HEADERS = $(UNIVALUE_DIST_HEADERS_INT)
7+
noinst_HEADERS = $(UNIVALUE_LIB_HEADERS_INT)
78

89
lib_LTLIBRARIES = libunivalue.la
910

1011
pkgconfigdir = $(libdir)/pkgconfig
1112
pkgconfig_DATA = pc/libunivalue.pc
1213

13-
libunivalue_la_SOURCES = \
14-
lib/univalue.cpp \
15-
lib/univalue_get.cpp \
16-
lib/univalue_read.cpp \
17-
lib/univalue_write.cpp
14+
libunivalue_la_SOURCES = $(UNIVALUE_LIB_SOURCES_INT)
1815

1916
libunivalue_la_LDFLAGS = \
2017
-version-info $(LIBUNIVALUE_CURRENT):$(LIBUNIVALUE_REVISION):$(LIBUNIVALUE_AGE) \
@@ -30,89 +27,32 @@ $(GENBIN): $(GEN_SRCS)
3027
@echo Building $@
3128
$(AM_V_at)c++ -I$(top_srcdir)/include -o $@ $<
3229

33-
gen: lib/univalue_escapes.h $(GENBIN)
34-
@echo Updating $<
30+
gen: $(GENBIN) FORCE
31+
@echo Updating lib/univalue_escapes.h
3532
$(AM_V_at)$(GENBIN) > lib/univalue_escapes.h
3633

3734
noinst_PROGRAMS = $(TESTS) test/test_json
3835

39-
TEST_DATA_DIR=test
40-
41-
test_unitester_SOURCES = test/unitester.cpp
36+
test_unitester_SOURCES = $(UNIVALUE_TEST_UNITESTER_INT)
4237
test_unitester_LDADD = libunivalue.la
43-
test_unitester_CXXFLAGS = -I$(top_srcdir)/include -DJSON_TEST_SRC=\"$(srcdir)/$(TEST_DATA_DIR)\"
38+
test_unitester_CXXFLAGS = -I$(top_srcdir)/include -DJSON_TEST_SRC=\"$(srcdir)/$(UNIVALUE_TEST_DATA_DIR_INT)\"
4439
test_unitester_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
4540

46-
test_test_json_SOURCES = test/test_json.cpp
41+
test_test_json_SOURCES = $(UNIVALUE_TEST_JSON_INT)
4742
test_test_json_LDADD = libunivalue.la
4843
test_test_json_CXXFLAGS = -I$(top_srcdir)/include
4944
test_test_json_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
5045

51-
test_no_nul_SOURCES = test/no_nul.cpp
46+
test_no_nul_SOURCES = $(UNIVALUE_TEST_NO_NUL_INT)
5247
test_no_nul_LDADD = libunivalue.la
5348
test_no_nul_CXXFLAGS = -I$(top_srcdir)/include
5449
test_no_nul_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
5550

56-
test_object_SOURCES = test/object.cpp
51+
test_object_SOURCES = $(UNIVALUE_TEST_OBJECT_INT)
5752
test_object_LDADD = libunivalue.la
5853
test_object_CXXFLAGS = -I$(top_srcdir)/include
5954
test_object_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS)
6055

61-
TEST_FILES = \
62-
$(TEST_DATA_DIR)/fail10.json \
63-
$(TEST_DATA_DIR)/fail11.json \
64-
$(TEST_DATA_DIR)/fail12.json \
65-
$(TEST_DATA_DIR)/fail13.json \
66-
$(TEST_DATA_DIR)/fail14.json \
67-
$(TEST_DATA_DIR)/fail15.json \
68-
$(TEST_DATA_DIR)/fail16.json \
69-
$(TEST_DATA_DIR)/fail17.json \
70-
$(TEST_DATA_DIR)/fail18.json \
71-
$(TEST_DATA_DIR)/fail19.json \
72-
$(TEST_DATA_DIR)/fail1.json \
73-
$(TEST_DATA_DIR)/fail20.json \
74-
$(TEST_DATA_DIR)/fail21.json \
75-
$(TEST_DATA_DIR)/fail22.json \
76-
$(TEST_DATA_DIR)/fail23.json \
77-
$(TEST_DATA_DIR)/fail24.json \
78-
$(TEST_DATA_DIR)/fail25.json \
79-
$(TEST_DATA_DIR)/fail26.json \
80-
$(TEST_DATA_DIR)/fail27.json \
81-
$(TEST_DATA_DIR)/fail28.json \
82-
$(TEST_DATA_DIR)/fail29.json \
83-
$(TEST_DATA_DIR)/fail2.json \
84-
$(TEST_DATA_DIR)/fail30.json \
85-
$(TEST_DATA_DIR)/fail31.json \
86-
$(TEST_DATA_DIR)/fail32.json \
87-
$(TEST_DATA_DIR)/fail33.json \
88-
$(TEST_DATA_DIR)/fail34.json \
89-
$(TEST_DATA_DIR)/fail35.json \
90-
$(TEST_DATA_DIR)/fail36.json \
91-
$(TEST_DATA_DIR)/fail37.json \
92-
$(TEST_DATA_DIR)/fail38.json \
93-
$(TEST_DATA_DIR)/fail39.json \
94-
$(TEST_DATA_DIR)/fail40.json \
95-
$(TEST_DATA_DIR)/fail41.json \
96-
$(TEST_DATA_DIR)/fail42.json \
97-
$(TEST_DATA_DIR)/fail44.json \
98-
$(TEST_DATA_DIR)/fail45.json \
99-
$(TEST_DATA_DIR)/fail3.json \
100-
$(TEST_DATA_DIR)/fail4.json \
101-
$(TEST_DATA_DIR)/fail5.json \
102-
$(TEST_DATA_DIR)/fail6.json \
103-
$(TEST_DATA_DIR)/fail7.json \
104-
$(TEST_DATA_DIR)/fail8.json \
105-
$(TEST_DATA_DIR)/fail9.json \
106-
$(TEST_DATA_DIR)/pass1.json \
107-
$(TEST_DATA_DIR)/pass2.json \
108-
$(TEST_DATA_DIR)/pass3.json \
109-
$(TEST_DATA_DIR)/pass4.json \
110-
$(TEST_DATA_DIR)/round1.json \
111-
$(TEST_DATA_DIR)/round2.json \
112-
$(TEST_DATA_DIR)/round3.json \
113-
$(TEST_DATA_DIR)/round4.json \
114-
$(TEST_DATA_DIR)/round5.json \
115-
$(TEST_DATA_DIR)/round6.json \
116-
$(TEST_DATA_DIR)/round7.json
117-
118-
EXTRA_DIST=$(TEST_FILES) $(GEN_SRCS)
56+
TEST_FILES = $(UNIVALUE_TEST_FILES_INT)
57+
58+
EXTRA_DIST=$(UNIVALUE_TEST_FILES_INT) $(GEN_SRCS)

0 commit comments

Comments
 (0)