Replace univalue subtree with proper dependency on external UniValue #7340

1. 
   luke-jr commented at 0:27 am on January 14, 2016: member

   This is not consensus-critical code, so really had no excuse to be subtree’d in the first place.
2. Replace univalue subtree with proper dependency on external UniValue 1d13699162
3. luke-jr force-pushed on Jan 14, 2016
4. luke-jr force-pushed on Jan 14, 2016
5. 
   luke-jr commented at 1:49 am on January 14, 2016: member

   Actually, this loses a bugfix, so @jgarzik needs to release UniValue 1.0.2 to make this sane…
6. 
   luke-jr commented at 1:53 am on January 14, 2016: member

   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… :/
7. depends: Add package for univalue 4752e33afa
8. luke-jr force-pushed on Jan 14, 2016
9. in depends/packages/univalue.mk: in 4752e33afa outdated

   0@@ -0,0 +1,21 @@
   1+package=univalue
   2+$(package)_version=1.0.1
   3+$(package)_download_path=https://codeload.github.com/jgarzik/$(package)/tar.gz
   

   ---

   ---

   

   jonasschnelli commented at 8:19 am on January 14, 2016:

   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).

   ---

   

   luke-jr commented at 3:28 pm on January 14, 2016:

   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.
10. 
    jonasschnelli commented at 8:23 am on January 14, 2016: contributor

    Hmm.. not sure about that. If I’m right, this would mean, in order to compile bitcoin-core, users need to checkout the univalue git repository and compile univalue by themselves unless there is broad support over package managers (apt-get, etc.)?

    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).

11. jonasschnelli added the label Build system on Jan 14, 2016
12. 
    laanwj commented at 12:44 pm on January 14, 2016: member

    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.

13. 
    luke-jr commented at 3:29 pm on January 14, 2016: member

    @laanwj Is it really so much harder to run:

    0make -C depends univalue
    

14. 
    MarcoFalke commented at 5:55 pm on January 14, 2016: member

    make -C depends univalue

    You’d have to update doc/build*

15. Bugfix: depends: Disabled shared libraries for univalue

    Static linking libstdc++ (as is done by Travis and gitian) with shared libraries breaks empty std::string with GNU

    8c059a6a7f
16. Bugfix: depends: Build [static] univalue with PIC 6ae79c66ff
17. depends: Bump univalue to 1.0.2 373ef74b41
18. doc: Add UniValue to build instructions 23e24b093c
19. 
    luke-jr commented at 3:22 am on January 15, 2016: member

    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

20. luke-jr closed this on Jan 15, 2016

    ---

21. MarcoFalke locked this on Sep 8, 2021

Contributors
luke-jr jonasschnelli laanwj MarcoFalke

Labels
Build system