build: Remove HAVE_CONSENSUS_LIB#29
Closed
Extheoisah wants to merge 1 commit into
Closed
Conversation
The `script/bitcoinconsensus` module defines the public interface for the `bitcoinconsensus` library. Even though this module is only required by the tests and the `bitcoinconsensus` library, it is currently compiled into the static internal `libbitcoin_consensus` library, and therefore used by a bunch of build targets that do not require it. Since it is always part of the internal library, the `HAVE_CONSENSUS_LIB` define used by the tests is essentially meaningless, since the module is always available. This can be verified on master by removing the `HAVE_CONSENSUS_LIB` checks in the tests, configuring with `--with-libs=no`, and running `make check`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
script/bitcoinconsensusmodule defines the public interface for thebitcoinconsensuslibrary. Even though this module is only required by the tests and thebitcoinconsensuslibrary, it is currently compiled into the static internallibbitcoin_consensuslibrary, and therefore used by a bunch of build targets that do not require it.Since it is always part of the internal library, the
HAVE_CONSENSUS_LIBdefine used by the tests is essentiallymeaningless, since the module is always available. This can be verified on master by removing the
HAVE_CONSENSUS_LIBchecks in the tests, configuring with--with-libs=no, and runningmake check.Improve all of this by including the
bitcoinconsensusmodule only where it is required and removing theHAVE_CONSENSUS_LIBdefine.An alternative to this patch was discussed in hebasto#41: Actually linking the test binaries with the external consensus library if it is available. This has the disadvantage though that if the dynamic consensus library is built, it has to be available for the test binaries to load whenever and wherever they are being run.