Consensus build package #7091
Consensus build package #7091
Conversation
jtimon
force-pushed
the
consensus-build
branch
from
164870c to
73d8f2a
Compare
|
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. |
jtimon
force-pushed
the
consensus-build
branch
3 times, most recently
from
cc96087 to
ed0dbfe
Compare
|
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 |
jtimon
force-pushed
the
consensus-build
branch
from
675078d to
6830388
Compare
|
Rebased(1). |
jtimon
force-pushed
the
consensus-build
branch
from
6830388 to
7f89bcb
Compare
|
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. |
…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.
jtimon
force-pushed
the
consensus-build
branch
from
7f89bcb to
cf82d05
Compare
|
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]). |
|
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). |
|
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)
6 participants
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.