Build against system UniValue when available #7349
Conversation
luke-jr
force-pushed
the
sys_univalue_opt
branch
2 times, most recently
from
92a1e65
to
5c09393
Compare
luke-jr
force-pushed
the
sys_univalue_opt
branch
from
5c09393
to
5d3b29b
Compare
|
This is nice. |
|
ut ACK |
|
utACK |
|
Ping @theuni |
|
Concept ack, but let's not default to using the system lib unless --with-system-univalue is specified, please. Otherwise someone (like myself) who has at some point built/installed univalue to system will suddenly find themselves frustrated when their in-tree univalue source changes aren't reflected in the build. |
|
|
||
| AM_LDFLAGS = $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) | ||
| AM_CXXFLAGS = $(HARDENED_CXXFLAGS) | ||
| AM_CPPFLAGS = $(HARDENED_CPPFLAGS) | ||
|
|
||
| if EMBEDDED_UNIVALUE | ||
| DIST_SUBDIRS += univalue |
There was a problem hiding this comment.
always add to dist, regardless of the config.
Just use --without-system-univalue. Defaults should not be set just for niche scenarios... |
|
Agreed, and the new (system) scenario is the niche scenario. It should be opt-in as long as we build it in-tree. |
|
Using system libraries when they are available is the standard use case, not the niche. I'm happy to go back to #7340 if that's preferable though. |
|
I'm not yet convinced that univalue will be a library found by default on many systems, will be maintained actively, and if so will do proper versioning in lock-step with our requirements. So to err on the side of caution I agree with @theuni, defaulting to the in-tree univalue makes sense. It can always be reconsidered later when things have matured for a few versions. |
|
I agree with @theuni. |
|
I think defaulting to "no" is still very unreasonable considering @jgarzik is the maintainer, but changed for now. |
|
Ok, thanks, utACK after other two @theuni's nits solved |
|
@theuni 's nits taken care of |
| @@ -46,6 +46,7 @@ Optional dependencies: | |||
| qt | GUI | GUI toolkit (only needed when GUI enabled) | |||
| protobuf | Payments in GUI | Data interchange format used for payment protocol (only needed when GUI enabled) | |||
| libqrencode | QR codes in GUI | Optional for generating QR codes (only needed when GUI enabled) | |||
| univalue | Utility | JSON parsing and encoding (if missing, bundled version will be used) | |||
There was a problem hiding this comment.
Nit: should specify that bundled version is used except if --with-system-univalue
|
Thanks, ut ack. |
42407ed build-unix: Update UniValue build conditions (Luke Dashjr) cdcad9f LDADD dependency order shuffling (Luke Dashjr) 62f7f2e Bugfix: Always include univalue in DIST_SUBDIRS (Luke Dashjr) 2356515 Change default configure option --with-system-univalue to "no" (Luke Dashjr) 5d3b29b doc: Add UniValue to build instructions (Luke Dashjr) ab22705 Build against system UniValue when available (Luke Dashjr) 2adf7e2 Bugfix: The var is LIBUNIVALUE,not LIBBITCOIN_UNIVALUE (Luke Dashjr)
If UniValue is present on the system, it will be used instead of bundled copy.
If not, bundled copy will automatically be built and used statically.
User can override either way using --with[out]-system-univalue configure option.