Replace univalue subtree with proper dependency on external UniValue #7340
Conversation
b4f6034 to
1d13699
Compare
|
Actually, this loses a bugfix, so @jgarzik needs to release UniValue 1.0.2 to make this sane... |
|
Also, I believe the Travis failure crash is unrelated to this PR, and a bug in UniValue (or Core) that we need to fix independently of this - but I can't reproduce it... :/ |
| @@ -0,0 +1,21 @@ | |||
| package=univalue | |||
| $(package)_version=1.0.1 | |||
| $(package)_download_path=https://codeload.github.com/jgarzik/$(package)/tar.gz | |||
There was a problem hiding this comment.
I think we should use the bitcoin/univalue.git repository to allow multiple people to maintain the repository (the subtree is also pointing to bitcoin/univalue.git).
There was a problem hiding this comment.
AFAIK that's just a mirror of @jgarzik 's upstream? Note he could very well add additional maintainers to his upstream repo as well, if he wants.
|
Hmm.. not sure about that. IMO we should use subtrees for dependencies that are not available over common package managers (bdb4.8 might be an exception because a) not really necessary [--with-incompatible-bdb] and b) wallet only). |
|
NACK. Univalue is a nice and small library, and it's useful to have it included.. This makes it much easier for people that want to build. I'd be OK with the option to use an external univalue, but don't remove the internal one. |
|
@laanwj Is it really so much harder to run: |
You'd have to update |
Static linking libstdc++ (as is done by Travis and gitian) with shared libraries breaks empty std::string with GNU
|
Apparently the depends system can't actually be used in the way I expected. While I still think this is the correct way to go, I'll start with making it an option for now... in #7349 |
This is not consensus-critical code, so really had no excuse to be subtree'd in the first place.