Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upConsensus build package #7091
Consensus build package #7091
Conversation
|
After making travis happy (https://travis-ci.org/bitcoin/bitcoin/builds/93038691 thanks to @theuni ) I force push with some squashes, some suggestions (not all yet) collected from @theuni on IRC and one of the commits nacked and deleted (unifying the crypto and consensus packages). Building locally on xubuntu-0.14, it is clear to me that I'm not using the following commands efficiently when touching Makefile.am:
Advice welcomed. |
ed0dbfe
|
Sorry for abusing travis but I got really paranoid while trying to find the needles (jtimon/bitcoin@consensus-build...jtimon:consensus-build-full). |
|
To clarify: it's my understanding that the problem is that autotools generate both the command-line arguments to the linker, and the emitted rules in the makefile in the same order, based on the order of things in LDADD. The order of arguments to the linker does matter (needs to be in order from dependers to dependencies), but for fast-failure behaviour, @jtimon wants them to be build in order from dependency to depender, which seems reasonable. @theuni Any idea about this? |
|
Yes, @sipa, that's what I would like: that packages are built in the opposite way they have to be listed for linking (ie lowest level things first). I was going to ask something like that in http://github.com/jtimon/bitcoin/commit/4f5cf2655f150c65748989aef97c3b7f6a78f9d7#commitcomment-14660501 but your explanation is more complete. |
|
By the way, the building order discussion is related to #5112 |
|
Rebased(1). |
|
Rebased(2). |
|
@theuni and I plan to peer program this to also avoid building the objects in the consensus package twice like it's being done now (once for libconsensus and once for the rest). So this will hopefully be replaced with something better. |
…package
…ackage
…nd dependend on both crypto and consensus packages Some extra bytes in libconsensus to get all the crypto (except for signing, which is in the common module) below the libconsensus future independent repo (that has libsecp256k1 as a subtree). hmac_sha256.o seems to be the only thing libbitcoinconsensus doesn't depend on from crypto, some more bytes for the final libconsensus: I'm not personally worried.
|
Reopened and updated (I had missed the new prevector [which by the way could have been created in the consensus folder directly instead of having to move it later]). |
devrandom
commented
Dec 10, 2015
|
ACK Looks good, and I tested linking with a trivial main function. Should also separate out the unit tests, but that could be postponed to a follow up PR. Let me know if you want me to take that on. |
|
At some point it would be ideal to separate the tests for the things in the consensus module/package to the consensus directory (just like wallet and qt). |
devrandom
commented
Dec 10, 2015
|
OK, cool. I'll wait for this to be merged first. |
|
Concept ACK |
|
ut ACK. Objections withdrawn. Lumping everything together is non-ideal, but it's still an improvement. Makes sense as a step forward. |
|
@theuni I'm also eager to see it building the modules on the consensus package being built only once instead of once, but I'll leave that in your hands. |
|
anything holding this? |
|
Not really, going to test... |
|
ACK cf82d05 |
cf82d05 Build: Consensus: Make libbitcoinconsensus_la_SOURCES fully dynamic and dependend on both crypto and consensus packages (Jorge Timón) 4feadec Build: Libconsensus: Move libconsensus-ready files to the consensus package (Jorge Timón) a3d5eec Build: Consensus: Move consensus files from common to its own module/package (Jorge Timón)
cf82d05 Build: Consensus: Make libbitcoinconsensus_la_SOURCES fully dynamic and dependend on both crypto and consensus packages (Jorge Timón) 4feadec Build: Libconsensus: Move libconsensus-ready files to the consensus package (Jorge Timón) a3d5eec Build: Consensus: Move consensus files from common to its own module/package (Jorge Timón)
cf82d05 Build: Consensus: Make libbitcoinconsensus_la_SOURCES fully dynamic and dependend on both crypto and consensus packages (Jorge Timón) 4feadec Build: Libconsensus: Move libconsensus-ready files to the consensus package (Jorge Timón) a3d5eec Build: Consensus: Move consensus files from common to its own module/package (Jorge Timón)
jtimon commentedNov 24, 2015
Although the encapsulations necessary to expose VerifyScript in libbitcoinconsensus were done, we're still building it in different packages. Creating an independent consensus package will make much more clear which files are currently part of libbitcoinconsensus. Furthermore, other libconsensus-ready files like arith_uint256.o, consensus/params.h, consensus/validation.h and primitives/block.o can be moved to that package already. This will make it harder to "undo" consensus encapsulation work while having travis happy.
Every executable that depends on the util and common packages now depends on the consensus package too.
Also, every executable depending on the consensus package seems to depend on the crypto package too, and libbitcoin_consensus only dependencies are crypto and libsecp256k1, so it doesn't seem to be of harm if we unify the crypto and consensus packages.
If we are going to eventually move all the libconsensus code to a subtree, this will make clearer which files need to be moved.
I'm specially interested in @theuni 's and travis' opinion before I make some squashes and/or reduce the scope of the PR.