Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented May 10, 2023

The current Autotools-based build system operates with two build artifacts:

The way how the libbitcoinconsensus.vcxproj project is used in the MSVC build system obviously shows that it is the former use case.

This PR makes the related adjustments to the MSVC build system.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 10, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK sipsorcery

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24773 (Enable HW-accelerated implementations of SHA256 for MSVC builds by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto added 3 commits May 10, 2023 11:26
This build artifact is not a user-exposed shared library (DLL) but a
convenience static library.
See: https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md

-BEGIN VERIFY SCRIPT-
sed -i 's/libbitcoinconsensus/libbitcoin_consensus/g' $(git grep -l "libbitcoinconsensus" -- build_msvc)
git mv build_msvc/libbitcoinconsensus build_msvc/libbitcoin_consensus
git mv build_msvc/libbitcoin_consensus/libbitcoinconsensus.vcxproj build_msvc/libbitcoin_consensus/libbitcoin_consensus.vcxproj
-END VERIFY SCRIPT-
See `libbitcoin_consensus_a_SOURCES` in the `src/Makefile.am`.
The `HAVE_CONSENSUS_LIB` symbol is supposed to be defined when a
user-exposed shared library (DLL) is built which is not the case here.
@fanquake
Copy link
Member

Other than renaming things, what does this acheive? Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.

@hebasto
Copy link
Member Author

hebasto commented May 17, 2023

Other than renaming things, what does this acheive?

It is required for #24773.

Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.

Sure. Not a priority at all.

@sipsorcery
Copy link
Contributor

As the rename makes the naming more correct seems reasonable to me.

ACK a94d75f.

@DrahtBot DrahtBot removed the request for review from sipsorcery May 17, 2023 19:21
@fanquake fanquake merged commit 87d6f6c into bitcoin:master May 18, 2023
@hebasto hebasto deleted the 230510-crypto branch May 18, 2023 11:10
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 19, 2023
@bitcoin bitcoin locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants