msvc: build secp256k1 and leveldb locally #14372

1. 
   ken2812221 commented at 7:12 pm on October 2, 2018: contributor

   In current MSVC build setup, the code depends on leveldb and secp256k1 that are installed from vcpkg which is not controlled by us. If we update our code, we have to wait for vcpkg port being merged.

   This PR move them from vcpkg to local branch to make it as same as autoconf.

   The leveldb changes is based on bitcoin-core/leveldb#14 and bitcoin-core/leveldb#18

2. 
   practicalswift commented at 8:48 pm on October 2, 2018: contributor

   src/leveldb/ should not be edited locally – the leveldb changes should probably be submitted upstream instead :-)
3. 
   DrahtBot commented at 9:40 pm on October 2, 2018: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   No conflicts as of last run.

4. 
   ras0219-msft commented at 9:42 pm on October 2, 2018: none

   If you want to maintain forks of leveldb/secp256k1 locally then it probably makes more sense to do this than install them via vcpkg.

   However, if the intent is to treat them as third party libraries, you might be happier with instead replacing the vcpkg portfiles to point at the modified versions before running install (C:\tools\vcpkg\ports\leveldb\). We always use the local catalog to do the build, so they can just be directly replaced before calling vcpkg remove --outdated; vcpkg install.

   Note: If you want them to be removed by remove --outdated, you need to make sure to change the version in ports\leveldb\CONTROL when making modifications.

5. ken2812221 force-pushed on Oct 2, 2018
6. ken2812221 force-pushed on Oct 2, 2018
7. fanquake added the label Windows on Oct 2, 2018
8. 
   MarcoFalke commented at 4:15 am on October 3, 2018: member

   Concept ACK on using the subtree when compiling with msvc. Though, could the subtree bump be done in a separate pull request?
9. 
   sipsorcery commented at 8:28 am on October 3, 2018: member

   Some of the leveldb changes proposed in this PR are already in PR’s on the leveldb repo, specifically https://github.com/bitcoin-core/leveldb/pull/14 and https://github.com/bitcoin-core/leveldb/pull/7 (as an aside the changes in https://github.com/bitcoin-core/leveldb/pull/14 were the ones I included in the leveldb vcpkg portfiles).

   For what it’s worth I’d agree with @practicalswift that changes to the sub-trees should be done upstream.

   I’m a concept ACK on using subtrees for leveldb and secp256k1 for the msvc/Visual Studio build. It does make debugging a little bit easier.

10. 
    ken2812221 commented at 8:54 am on October 3, 2018: contributor

    @sipsorcery Sorry. I’m not aware that you already submmitted a PR for that. I’ll revert the duplicate thing in bitcoin-core/leveldb#18 @practicalswift @sipsorcery I know that they should be done upstream, but they are not done yet. I could show the result before then and make CI happy. Anyway, this PR shouldn’t be merged before upstream one merged.
11. DrahtBot added the label Needs rebase on Oct 8, 2018
12. ken2812221 force-pushed on Oct 8, 2018
13. ken2812221 force-pushed on Oct 8, 2018
14. DrahtBot removed the label Needs rebase on Oct 8, 2018
15. ken2812221 force-pushed on Oct 8, 2018
16. DrahtBot added the label Needs rebase on Oct 20, 2018
17. ken2812221 force-pushed on Oct 21, 2018
18. DrahtBot removed the label Needs rebase on Oct 21, 2018
19. DrahtBot added the label Needs rebase on Nov 7, 2018
20. ken2812221 force-pushed on Nov 8, 2018
21. DrahtBot removed the label Needs rebase on Nov 8, 2018
22. ken2812221 force-pushed on Nov 8, 2018
23. 
    laanwj commented at 4:28 pm on January 16, 2019: member

    Concept ACK, makes sense if MSVC follows the autoconf-based build system in this regard.
24. ken2812221 force-pushed on Jan 22, 2019
25. ken2812221 force-pushed on Jan 22, 2019
26. ken2812221 force-pushed on Jan 22, 2019
27. ken2812221 force-pushed on Jan 22, 2019
28. MarcoFalke added this to the milestone 0.18.0 on Jan 26, 2019
29. 
    MarcoFalke commented at 3:10 pm on January 31, 2019: member

    Needs rebase
30. MarcoFalke added the label Needs rebase on Jan 31, 2019
31. ken2812221 force-pushed on Jan 31, 2019
32. DrahtBot removed the label Needs rebase on Jan 31, 2019
33. 
    MarcoFalke commented at 4:05 pm on January 31, 2019: member

    @sipsorcery @NicolasDorier Is this good to merge?
34. DrahtBot added the label Needs rebase on Jan 31, 2019
35. msvc: build secp256k1 locally 52091066be
36. msvc: build leveldb locally 82dcacb822
37. ken2812221 force-pushed on Jan 31, 2019
38. DrahtBot removed the label Needs rebase on Jan 31, 2019
39. 
    sipsorcery commented at 7:48 pm on January 31, 2019: member

    tACK 52091066be15a86a38c4db182338808f9316c35a.

    Builds correctly on VS2017, VS2019 and AppVeyor.

    (One note is that Pieter Wuille did comment somewhere that the libsecp256k1 config options set here meant that Windows ecdsa operations were up to 4 times slower than on Linux. I had a quick look but because msvc doesn’t support asm on 64 bit builds didn’t find any way to improve it)

40. MarcoFalke referenced this in commit 3b19d8e341 on Jan 31, 2019
41. MarcoFalke merged this on Jan 31, 2019
42. MarcoFalke closed this on Jan 31, 2019

    ---

43. 
    NicolasDorier commented at 7:28 am on February 1, 2019: contributor

    post merge tACK.
44. ken2812221 referenced this in commit bef8fdd6e2 on Feb 1, 2019
45. MarcoFalke referenced this in commit 6a5feb7d82 on Feb 4, 2019
46. HashUnlimited referenced this in commit 08d5d81f79 on Feb 5, 2019
47. ken2812221 deleted the branch on Feb 9, 2019
48. ken2812221 referenced this in commit 3c6ef0393f on Feb 14, 2019
49. MarcoFalke referenced this in commit e3b1c7a9d6 on Feb 14, 2019
50. HashUnlimited referenced this in commit 3933d6669b on Feb 23, 2019
51. kallewoof referenced this in commit 66c015529e on Oct 4, 2019
52. DrahtBot locked this on Dec 16, 2021

Contributors
ken2812221 practicalswift DrahtBot ras0219-msft MarcoFalke sipsorcery laanwj NicolasDorier

Labels
Windows

Milestone
0.18.0