build: Re-add command to install vcpkg #21733

pull dplusplus1024 wants to merge 1 commits into bitcoin:master from dplusplus1024:patch-1 changing 1 files +5 −1
  1. dplusplus1024 commented at 10:44 PM on April 19, 2021: contributor

    vcpkg integrate install must be executed so that msbuild will automatically install external dependencies.

    It was removed in https://github.com/bitcoin/bitcoin/commit/712f95d3324d02310dd468e7bfd1e1b0df432e77

    It was originally added in https://github.com/bitcoin/bitcoin/commit/76445677586a4c2fa72606b662269a4390c2e71f

    <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

    <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

    <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

  2. Re-add command to install vcpkg
    `vcpkg integrate install` must be executed so that msbuild will automatically install external dependencies.
    
    It was removed in https://github.com/bitcoin/bitcoin/commit/712f95d3324d02310dd468e7bfd1e1b0df432e77
    
    It was originally added in https://github.com/bitcoin/bitcoin/commit/76445677586a4c2fa72606b662269a4390c2e71f
    de17d245b7
  3. fanquake added the label Windows on Apr 19, 2021
  4. dplusplus1024 marked this as ready for review on Apr 19, 2021
  5. ddustin commented at 10:58 PM on April 19, 2021: none

    ACK

  6. sipsorcery commented at 11:19 PM on April 19, 2021: member

    ACK de17d245b7462784f9d36591b0f346aa6b2e9b8c.

    I did shorten the vcpkg instructions when manifests were introduced. The idea being that msbuild would take care of calling vcpkg to install the dependencies automatically and it would be one less step.

    Given that:

    • It seems to be recommended to still call vcpkg integrate install even with manifests enabled.
    • At least one issue, #17864, has been raised with the Bitcoin Core vcpkg instructions.
    • Executing vcpkg integrate install should not cause any problems for manifests (although it's not clear what happens if different versions of a dependency are installed in the main vcpkg directory and the manifest directory).

    It's an ACK from me.

  7. hebasto approved
  8. hebasto commented at 11:00 AM on April 20, 2021: member

    ACK de17d245b7462784f9d36591b0f346aa6b2e9b8c, I use the same in #21551.

  9. MarcoFalke merged this on Apr 20, 2021
  10. MarcoFalke closed this on Apr 20, 2021

  11. MarcoFalke commented at 11:22 AM on April 20, 2021: member

    Sorry, I missed the "Merge message contains an html comment!" warning when merging this. So the merge commit now contains an html comment.

  12. gwillen referenced this in commit 7a6382c3ac on Jun 1, 2022
  13. DrahtBot locked this on Aug 16, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-15 03:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me