cmake: Add FindQRencode module and enable libqrencode package for MSVC #31173

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:241028-qrencode changing 6 files +78 −11
  1. hebasto commented at 3:01 pm on October 28, 2024: member

    This PR introduces the FindQRencode CMake module, following the official CMake guidelines for managing upstream libraries that lack a config file package. This module enhances flexibility in locating the libqrencode library by making the use of pkg-config optional.

    With this update, libqrencode can be detected on systems where either pkg-config or the libqrencode.pc file is unavailable, such as Windows environments using the vcpkg package manager. However, if libqrencode.pc is available, it remains beneficial as the only direct source of the library’s version information.

    Additionally, the libqrencode vcpkg package is enabled for MSVC builds.

    Here is a diff for configuration output on Ubuntu 24.10:

    0 -- Detecting CXX compile features - done
    1 -- Found SQLite3: /usr/include (found suitable version "3.46.1", minimum required is "3.7.17")
    2 -- Found PkgConfig: /usr/bin/pkg-config (found version "1.8.1")
    3--- Checking for module 'libqrencode'
    4---   Found libqrencode, version 4.1.1
    5+-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so (found version "4.1.1")
    6 -- Found Qt: /usr/lib/x86_64-linux-gnu/cmake/Qt5 (found suitable version "5.15.15", minimum required is "5.11.3")
    7 -- Performing Test CXX_SUPPORTS__WERROR
    8 -- Performing Test CXX_SUPPORTS__WERROR - Success
    
  2. DrahtBot commented at 3:01 pm on October 28, 2024: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31173.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK fanquake
    Concept ACK jarolrod
    Stale ACK hodlinator

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

    Conflicts

    No conflicts as of last run.

  3. DrahtBot added the label Build system on Oct 28, 2024
  4. in cmake/module/FindQRencode.cmake:12 in 2b93e7d62d outdated
     7+------------
     8+
     9+Finds the QRencode header and library.
    10+
    11+This is a wrapper around find_package()/pkg_check_modules() commands that:
    12+ - facilitates searching in various build environments
    


    fanquake commented at 3:14 pm on October 28, 2024:

    various build environments

    Given that all the new code is just to facilitate MSVC/vcpkg, you could probably be a bit more concrete with the documentation. Otherwise someone might wonder why all of this is being done, when

    0  find_package(PkgConfig REQUIRED)
    1  pkg_check_modules(PC_QRencode QUIET libqrencode)
    

    would otherwise work.


    hebasto commented at 12:20 pm on November 1, 2024:
    The branch and the PR description have been updated.
  5. fanquake commented at 3:21 pm on October 28, 2024: member

    Concept ACK

    — Checking for module ’libqrencode’ — Found libqrencode, version 4.1.1 +– Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so

    This output seems less useful, given it no-longer has the version.

  6. jarolrod commented at 8:47 am on October 29, 2024: member
    Concept ACK
  7. hebasto force-pushed on Nov 1, 2024
  8. hebasto commented at 12:20 pm on November 1, 2024: member

    — Checking for module ’libqrencode’ — Found libqrencode, version 4.1.1 +– Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so

    This output seems less useful, given it no-longer has the version.

    Updated.

  9. hebasto commented at 12:22 pm on November 1, 2024: member

    @davidgumberg @hodlinator

    Want to test this PR on Windows?

  10. hodlinator commented at 10:31 pm on November 1, 2024: contributor

    Concept ACK c9baf41372b06e08976a3daf459468d3f6ad28ea

    Screenshot 2024-11-01 232102

    Tested with and without PR changes:

    0> cmake -B build --preset vs2022-static
    

    Confirmed that QR image is not shown when displaying receive address without the PR. 👍

    Only found a download path for the QREncode lib in depends/packages/qrencode.mk, could not spot how the new cmake/module/FindQRencode.cmake script hooks into Depends after the PkgConfig check fails? 🤔

  11. hebasto commented at 9:01 am on November 2, 2024: member

    Only found a download path for the QREncode lib in depends/packages/qrencode.mk, could not spot how the new cmake/module/FindQRencode.cmake script hooks into Depends after the PkgConfig check fails? 🤔

    Depends are not involved when building on Windows.

    The toolchain file provided by vcpkg sets the CMAKE_FIND_ROOT_PATH variable, directing the find_path() and find_library() commands to the appropriate paths.

  12. fanquake added this to the milestone 29.0 on Nov 2, 2024
  13. hodlinator commented at 9:49 pm on November 2, 2024: contributor

    The toolchain file provided by vcpkg sets the CMAKE_FIND_ROOT_PATH variable, directing the find_path() and find_library() commands to the appropriate paths.

    Okay, the toolchain file seems to be C:\Program Files\Microsoft Visual Studio\2022\Community\VC\vcpkg\scripts\buildsystems\vcpkg.cmake in my case. Do you imply a path is appended to CMAKE_FIND_ROOT_PATH which enables finding depends/packages/qrencode.mk without invoking Depends itself, or how is vcpkg able to find QREncode?

  14. hebasto commented at 9:41 am on November 3, 2024: member

    The toolchain file provided by vcpkg sets the CMAKE_FIND_ROOT_PATH variable, directing the find_path() and find_library() commands to the appropriate paths.

    Okay, the toolchain file seems to be C:\Program Files\Microsoft Visual Studio\2022\Community\VC\vcpkg\scripts\buildsystems\vcpkg.cmake in my case. Do you imply a path is appended to CMAKE_FIND_ROOT_PATH which enables finding depends/packages/qrencode.mk without invoking Depends itself, or how is vcpkg able to find QREncode?

    Depends are not involved when building on Windows.

    vcpkg builds all packages specified in the vcpkg.json manifest file and installs them to the well-defined subdirectories in the build directory when invoking cmake -B build --preset vs2022-static. This process can be time-consuming, so vcpkg caches binary artifacts for reuse. Finally, vcpkg adds the installation paths of the built packages to the CMAKE_FIND_ROOT_PATH variable in the toolchain file.

  15. hodlinator commented at 8:41 am on November 4, 2024: contributor

    And vcpkg has a global index similar to Rust crates etc: https://vcpkg.io/en/package/libqrencode. That was the piece I was missing from the puzzle.

    Thanks for bearing with me! Haven’t knowingly used vcpkg for Windows dev before.

  16. hodlinator approved
  17. hodlinator commented at 9:00 am on November 4, 2024: contributor

    ACK c9baf41372b06e08976a3daf459468d3f6ad28ea

    Great to have more fully-fledged builds by default when building on Windows.

    CMake & vcpkg novice here but can’t spot anything funky going on. Guess one attack vector is modifying the global vcpkg index to make QREncode contain malicious code, but that’s implied in vcpkg builds. Official Windows releases are cross-compiled from Linux without using vcpkg, so this risk should mostly affect devs.

    Should probably remove this line from the docs:

     0diff --git a/doc/build-windows-msvc.md b/doc/build-windows-msvc.md
     1index 80c2b77f1e..34fc9dabd6 100644
     2--- a/doc/build-windows-msvc.md
     3+++ b/doc/build-windows-msvc.md
     4@@ -44,7 +44,6 @@ cmake --list-presets
     5 
     6 By default, all presets:
     7 - Set `BUILD_GUI` to `ON`.
     8-- Set `WITH_QRENCODE` to `OFF`, due to known build issues when using vcpkg's `libqrencode` package.
     9 
    10 ## Building
    
  18. DrahtBot requested review from jarolrod on Nov 4, 2024
  19. DrahtBot requested review from fanquake on Nov 4, 2024
  20. hebasto force-pushed on Nov 4, 2024
  21. hebasto commented at 11:02 am on November 4, 2024: member

    Should probably remove this line from the docs:

    Thanks! Amended.

  22. DrahtBot added the label CI failed on Nov 4, 2024
  23. hodlinator approved
  24. hodlinator commented at 2:10 pm on November 4, 2024: contributor

    re-ACK 6b493e5dca53eeafbe3e10150e7bcdc71d8ffae0

    Just variant of suggested doc change applied since prior ACK.

  25. DrahtBot removed the label CI failed on Nov 4, 2024
  26. DrahtBot added the label Needs rebase on Nov 5, 2024
  27. cmake: Add `FindQRencode` module 30089b0cb6
  28. build, msvc: Enable `libqrencode` vcpkg package 9e5089dbb0
  29. hebasto force-pushed on Nov 5, 2024
  30. hebasto commented at 4:39 pm on November 5, 2024: member
    Rebased due to the conflict with the merged bitcoin/bitcoin#31186.
  31. DrahtBot removed the label Needs rebase on Nov 5, 2024
  32. fanquake commented at 12:04 pm on November 6, 2024: member

    Guix Build:

     0ea6e6faf76005701a3251872e64ee1dd5c7e1222d28b38a273d9c1851470e927  guix-build-9e5089dbb02e/output/aarch64-linux-gnu/SHA256SUMS.part
     1b71cc1acf87a5394334149a2cd724e6acd97b2660d7f74d32b97e62f83a539d6  guix-build-9e5089dbb02e/output/aarch64-linux-gnu/bitcoin-9e5089dbb02e-aarch64-linux-gnu-debug.tar.gz
     2d4879760d76da7aa4a41fe56981a0c2992032ea41e9460ef60036dc57599e9e7  guix-build-9e5089dbb02e/output/aarch64-linux-gnu/bitcoin-9e5089dbb02e-aarch64-linux-gnu.tar.gz
     32cbd20b0b10f97fb3a577b8f47aa8e3cbfceeaa22b0c6473f40aa180c773cc60  guix-build-9e5089dbb02e/output/arm-linux-gnueabihf/SHA256SUMS.part
     47aaef6f6417ba2352568f7ee91716f795052ac34f17ed6c0591bfb1288ec01ad  guix-build-9e5089dbb02e/output/arm-linux-gnueabihf/bitcoin-9e5089dbb02e-arm-linux-gnueabihf-debug.tar.gz
     534a735acf76a3da6355a9fbf836ff67fd50ae2b9303d74a296d896d54630560d  guix-build-9e5089dbb02e/output/arm-linux-gnueabihf/bitcoin-9e5089dbb02e-arm-linux-gnueabihf.tar.gz
     6ebf89e7fb736d01d6309de8823f3d4315d72fd45184ec47b785a2ea6e0a0544c  guix-build-9e5089dbb02e/output/arm64-apple-darwin/SHA256SUMS.part
     7fbfed37f36f69e650e5ab6057acc6b2ba38c89ac42ccfab06a2907478eddff6f  guix-build-9e5089dbb02e/output/arm64-apple-darwin/bitcoin-9e5089dbb02e-arm64-apple-darwin-unsigned.tar.gz
     8bdd50fc969e68b1f73b341a112553b5f09052c859245a7447c89ad08bd50c53b  guix-build-9e5089dbb02e/output/arm64-apple-darwin/bitcoin-9e5089dbb02e-arm64-apple-darwin-unsigned.zip
     9218b02012eb23478fe84631461a024d73d0c0829e0aa3111592c4b62bbc0b8a1  guix-build-9e5089dbb02e/output/arm64-apple-darwin/bitcoin-9e5089dbb02e-arm64-apple-darwin.tar.gz
    10a19abd39e81417f52d8924058322d64416948bde6665173de459877e07be9f3f  guix-build-9e5089dbb02e/output/dist-archive/bitcoin-9e5089dbb02e.tar.gz
    11b3aea24b9f8095c1df49d8721f14d486d100fd4cd73561cc6fc37b7346701bce  guix-build-9e5089dbb02e/output/powerpc64-linux-gnu/SHA256SUMS.part
    12a1b1b5fbdf29d2454e366225d1ceab3aa36d895fad729a0afd1fffc2950151d4  guix-build-9e5089dbb02e/output/powerpc64-linux-gnu/bitcoin-9e5089dbb02e-powerpc64-linux-gnu-debug.tar.gz
    135f164dc2445368ee94991b03347605f33238292efec49a09c7852c0b79708ae3  guix-build-9e5089dbb02e/output/powerpc64-linux-gnu/bitcoin-9e5089dbb02e-powerpc64-linux-gnu.tar.gz
    1462cac6ec59a8c73f313faf4076918e6b0e3d8bc20161f6600953115aaaf8a7ad  guix-build-9e5089dbb02e/output/riscv64-linux-gnu/SHA256SUMS.part
    15541a74b9f60a51d2637a76dd665277033dbdf50dc38c7a33de5f7ee5d1e5f08c  guix-build-9e5089dbb02e/output/riscv64-linux-gnu/bitcoin-9e5089dbb02e-riscv64-linux-gnu-debug.tar.gz
    166d96e2219c8927de16cc8dbcfec8e5e2a20389b53ceb5f97cc836ef534651f16  guix-build-9e5089dbb02e/output/riscv64-linux-gnu/bitcoin-9e5089dbb02e-riscv64-linux-gnu.tar.gz
    1716cd8f5e0b32c461cd64e8ffb92421b5547570a2fd61e4a8e678c1cf006111b1  guix-build-9e5089dbb02e/output/x86_64-apple-darwin/SHA256SUMS.part
    184b205511af61f5d7341237dd93e65bcabc806a5d2a29ed18df44a2e0e54fd6dc  guix-build-9e5089dbb02e/output/x86_64-apple-darwin/bitcoin-9e5089dbb02e-x86_64-apple-darwin-unsigned.tar.gz
    193a96667d349165e78c0235ef243de6f0f7eca17685ff44ae105b7979dc016c2f  guix-build-9e5089dbb02e/output/x86_64-apple-darwin/bitcoin-9e5089dbb02e-x86_64-apple-darwin-unsigned.zip
    2021c88a8b3f13484d16edf02f774933327e53dd0232aaa53fe815db2420dcdc9a  guix-build-9e5089dbb02e/output/x86_64-apple-darwin/bitcoin-9e5089dbb02e-x86_64-apple-darwin.tar.gz
    21feb3237a8a1ba2fdbff9da527d033baea2d201b204def6bcf6b96b5a1c76dd64  guix-build-9e5089dbb02e/output/x86_64-linux-gnu/SHA256SUMS.part
    22fbf2468b460da9113733a96f9189b7b64874f5eec75bb681ba305fcdf83cd7f0  guix-build-9e5089dbb02e/output/x86_64-linux-gnu/bitcoin-9e5089dbb02e-x86_64-linux-gnu-debug.tar.gz
    236697ed2c72a6901f723e88f31af9e957d0c38f2f9bfd9a67ce3c144816d8faf1  guix-build-9e5089dbb02e/output/x86_64-linux-gnu/bitcoin-9e5089dbb02e-x86_64-linux-gnu.tar.gz
    24945b78cba4d632bad75df7190a8b11896066b0c41a0ba2dcd82b1deb099293df  guix-build-9e5089dbb02e/output/x86_64-w64-mingw32/SHA256SUMS.part
    25c9d86272833826fd8ed60375a784ee095911ca5f8c88018e1e76d4c632e70ad4  guix-build-9e5089dbb02e/output/x86_64-w64-mingw32/bitcoin-9e5089dbb02e-win64-debug.zip
    264701ffb876630d129d365c3f5bc457310d23ba60caf6e948b1af2581aca4092c  guix-build-9e5089dbb02e/output/x86_64-w64-mingw32/bitcoin-9e5089dbb02e-win64-setup-unsigned.exe
    279f55d7375e4005d4ebb84c3a6f0077ad5c5541a938b3ccb476100b169b93fa87  guix-build-9e5089dbb02e/output/x86_64-w64-mingw32/bitcoin-9e5089dbb02e-win64-unsigned.tar.gz
    28759594e4e0c907a881903f82d3bf0c2e0ca5acf89545e0879e91eebe30f2402f  guix-build-9e5089dbb02e/output/x86_64-w64-mingw32/bitcoin-9e5089dbb02e-win64.zip
    
  33. fanquake approved
  34. fanquake commented at 12:10 pm on November 6, 2024: member
    ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532
  35. DrahtBot requested review from hodlinator on Nov 6, 2024
  36. fanquake merged this on Nov 6, 2024
  37. fanquake closed this on Nov 6, 2024

  38. hebasto deleted the branch on Nov 6, 2024
  39. hodlinator commented at 1:27 pm on November 6, 2024: contributor

    Post-merge re-ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532

    Only resolving against formatting changes made to vcpkg.json in #31186 since my previous ACK.


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: 2024-11-21 09:12 UTC

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