depends: build miniupnpc with CMake #29707

pull fanquake wants to merge 4 commits into bitcoin:master from fanquake:miniupnpc_2_2_7 changing 7 files +76 −66
  1. fanquake commented at 5:17 pm on March 22, 2024: member
    This picks up one of the changes from #29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I’ve upstreamed https://github.com/miniupnp/miniupnp/pull/721, as well as some suggestions from the previous PR.
  2. DrahtBot commented at 5:17 pm on March 22, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK theuni, TheCharlatan
    Concept ACK hebasto

    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 Mar 22, 2024
  4. theuni commented at 5:24 pm on March 22, 2024: member
    Nice. Concept ACK.
  5. hebasto commented at 5:24 pm on March 22, 2024: member
    Concept ACK.
  6. DrahtBot added the label Needs rebase on Mar 25, 2024
  7. fanquake force-pushed on Mar 25, 2024
  8. DrahtBot removed the label Needs rebase on Mar 25, 2024
  9. fanquake force-pushed on Mar 26, 2024
  10. fanquake force-pushed on Apr 5, 2024
  11. fanquake marked this as ready for review on Apr 5, 2024
  12. DrahtBot added the label CI failed on Apr 5, 2024
  13. fanquake force-pushed on Apr 8, 2024
  14. fanquake force-pushed on Apr 9, 2024
  15. fanquake force-pushed on Apr 15, 2024
  16. fanquake commented at 3:21 pm on April 15, 2024: member
    Pushed for builds now that the mirrors are back up.
  17. DrahtBot removed the label CI failed on Apr 15, 2024
  18. hebasto commented at 12:19 pm on April 20, 2024: member

    Tested e79c54456f3e299a619a46451a4ce5a017b8da44.

    I’ve verified compiler flags for CMake vs Autotools without providing the HOST variable:

    1. A new -DMINIUPNP_STATICLIB macro, which basically is Windows-specific and has no affect on non-Windows targets.
    2. The -fno-common flag is being missed, however, it the default for both GCC and Clang.
    3. Warning flags with Autotools are -Wall -W -Wstrict-prototypes, which effectively is equivalent to -Wstrict-prototypes. CMake provides just -Wall. However, no new warnings are being emitted.

    So far so good.

    As https://github.com/miniupnp/miniupnp/pull/721 has been merged, does it make sense to bump the source to the top commit and drop the related patch?

  19. hebasto commented at 12:59 pm on April 20, 2024: member

    Continued testing e79c54456f3e299a619a46451a4ce5a017b8da44.

    Compiler flags for HOST=x86_64-w64-mingw32 are OK, including correct -D_WIN32_WINNT=0x0601.

    However, the commit e5a114ac762fb6ce331998e6d2730386b9dbc905 is broken, which is not good for the commit history:

     0$ make -C depends miniupnpc HOST=x86_64-w64-mingw32
 1make: Entering directory '/home/hebasto/git/bitcoin/depends'
 2Extracting miniupnpc...
 3/home/hebasto/git/bitcoin/depends/sources/miniupnpc-2.2.7.tar.gz: OK
 4Preprocessing miniupnpc...
 5patching file src/minisoap.c
 6patching file src/miniwget.c
 7patching file Makefile
 8Hunk [#1](/bitcoin-bitcoin/1/) succeeded at 303 with fuzz 1 (offset 5 lines).
 9Configuring miniupnpc...
10Building miniupnpc...
11make[1]: Entering directory '/home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/miniupnpc/2.2.7-40ffab43d67'
12make[1]: *** No rule to make target 'build/libminiupnpc.a'.  Stop.
13make[1]: Leaving directory '/home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/miniupnpc/2.2.7-40ffab43d67'
14make: *** [funcs.mk:297: /home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/miniupnpc/2.2.7-40ffab43d67/./.stamp_built] Error 2
15make: Leaving directory '/home/hebasto/git/bitcoin/depends'

    Even after adjusting paths, it still remains broken:

    0x86_64-w64-mingw32-gcc -pipe -std=c11 -O2 -D_WIN32_WINNT=0x0601 -DNDEBUG -D_WIN32_WINNT=0x501 -Iinclude -I. -DMINIUPNP_STATICLIB -c -o miniwget.o src/miniwget.c
1<command-line>: warning: "_WIN32_WINNT" redefined
  20. hebasto commented at 3:49 pm on April 20, 2024: member
    FWIW, I found another Windows-specific bug in the upstream: https://github.com/miniupnp/miniupnp/pull/727.
  21. DrahtBot added the label Needs rebase on Apr 25, 2024
  22. fanquake force-pushed on Apr 25, 2024
  23. fanquake commented at 3:09 pm on April 25, 2024: member

    FWIW, I found another Windows-specific bug in the upstream: https://github.com/miniupnp/miniupnp/pull/727.

    Rebased again, but can pull in this change + fixups on next push

  24. DrahtBot removed the label Needs rebase on Apr 25, 2024
  25. depends: miniupnpc 2.2.7 
    Includes a temporary patch to fix the Windows Autotools build.

See
https://miniupnp.tuxfamily.org/files/changelog.php?file=miniupnpc-2.2.7.tar.gz.
    6866b571ab
  26. depends: add upstream CMake patch to miniupnpc f5618c79d9
  27. depends: switch miniupnpc to CMake 
    Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: fanquake <fanquake@gmail.com>
    3c2d440f14
  28. depends: fix miniupnpc snprintf usage on Windows 5195baa600
  29. fanquake force-pushed on Apr 30, 2024
  30. fanquake requested review from theuni on May 1, 2024
  31. theuni approved
  32. theuni commented at 2:42 pm on May 1, 2024: member

    ACK 5195baa60087ee366290887ad982fc491e14c111.

    Compile-tested for Linux, output looks good for debug and release builds.

  33. DrahtBot requested review from hebasto on May 1, 2024
  34. TheCharlatan approved
  35. TheCharlatan commented at 4:00 pm on May 1, 2024: contributor

    utACK 5195baa60087ee366290887ad982fc491e14c111

    Guix builds (aarch64)

     0fb77af7f4d684e28e9eecc7680af6aec9f6789921d87c49673c51c917b1fe2ed  guix-build-5195baa60087/output/aarch64-linux-gnu/SHA256SUMS.part
 18962d6eb216e19c95e9f90ec1e879e309d3bbed62a1cb91b67b2155956580a50  guix-build-5195baa60087/output/aarch64-linux-gnu/bitcoin-5195baa60087-aarch64-linux-gnu-debug.tar.gz
 2f2dd00f3502e32dd50f232178aa77e63f09359ac303f2309491cb8c08d2b4f15  guix-build-5195baa60087/output/aarch64-linux-gnu/bitcoin-5195baa60087-aarch64-linux-gnu.tar.gz
 31d7800e753b22c70b8df715e6dab4f5b3ed469d5a2d56f24ceb915e6544b0140  guix-build-5195baa60087/output/arm-linux-gnueabihf/SHA256SUMS.part
 40a9b989b60ae9ccff384b06b8ee2612b97055de649ebff496b594436b29d7126  guix-build-5195baa60087/output/arm-linux-gnueabihf/bitcoin-5195baa60087-arm-linux-gnueabihf-debug.tar.gz
 5fadc3e1df58ccc3f2704d0977aef336398aefff2124a0e325803d8faad82e830  guix-build-5195baa60087/output/arm-linux-gnueabihf/bitcoin-5195baa60087-arm-linux-gnueabihf.tar.gz
 6e6f0c6b388499cb144f0c80ee2bc96b9377a7989e2977a519115c119a4bae6ce  guix-build-5195baa60087/output/arm64-apple-darwin/SHA256SUMS.part
 7c66a1167dc514386cef452645c813f3f365405acb3a9615bbd86ca5922c97f98  guix-build-5195baa60087/output/arm64-apple-darwin/bitcoin-5195baa60087-arm64-apple-darwin-unsigned.tar.gz
 817eccde10dd1a7a79bd24c471369a63b9e8d237194273737d1f1a1b573438e02  guix-build-5195baa60087/output/arm64-apple-darwin/bitcoin-5195baa60087-arm64-apple-darwin-unsigned.zip
 93de3103ebf84b7f03b70fc79b919cdc874c30cb01ff9338556491e0b096dcb57  guix-build-5195baa60087/output/arm64-apple-darwin/bitcoin-5195baa60087-arm64-apple-darwin.tar.gz
10dc6019dd78c0968b65687d1651f6fc52588e3f58b15eb83b1fb5c7d482edbf16  guix-build-5195baa60087/output/dist-archive/bitcoin-5195baa60087.tar.gz
1179772282913fd981455c52b248f862e4603c23b72f1564ae09ece6e4d1c32827  guix-build-5195baa60087/output/powerpc64-linux-gnu/SHA256SUMS.part
12960674739647971d84488bd46aa152957462ae734b08e779bb4f6af189442dc6  guix-build-5195baa60087/output/powerpc64-linux-gnu/bitcoin-5195baa60087-powerpc64-linux-gnu-debug.tar.gz
139c58aeae8577f6d81e32f2ecdce930078fd1ad55097b1001b578802d3d769f57  guix-build-5195baa60087/output/powerpc64-linux-gnu/bitcoin-5195baa60087-powerpc64-linux-gnu.tar.gz
143b6b2c5cef989031bbd7c4f65acc50936a937d6154611283e4cb830df1ca7f1c  guix-build-5195baa60087/output/riscv64-linux-gnu/SHA256SUMS.part
153139e8d02ad68dd3f3b4687b8c5797bace5a6bf5ecc0e09d51db5d213a259d00  guix-build-5195baa60087/output/riscv64-linux-gnu/bitcoin-5195baa60087-riscv64-linux-gnu-debug.tar.gz
161c06e11fc2fd0fb47fd0c66b0bbf0e4b2fdd7e517d252105490ba5c2f456e92d  guix-build-5195baa60087/output/riscv64-linux-gnu/bitcoin-5195baa60087-riscv64-linux-gnu.tar.gz
1739ec562a649dd65cb1533eb7da4f42eb87ac980af57b775369011c5b3c8b2a1a  guix-build-5195baa60087/output/x86_64-apple-darwin/SHA256SUMS.part
18b87bbdcce5b9fa84922ec23336aa465eedd5d2d7e38644c2ea5dddb6c46e72b6  guix-build-5195baa60087/output/x86_64-apple-darwin/bitcoin-5195baa60087-x86_64-apple-darwin-unsigned.tar.gz
19c621b236f852ba8a8e4b0d070194638984af13e758a39496b87776f2a4ac6510  guix-build-5195baa60087/output/x86_64-apple-darwin/bitcoin-5195baa60087-x86_64-apple-darwin-unsigned.zip
20af6ed0a67813f64a44aab312b822e2ce6024df68f4803ad3f1bc66197227b4da  guix-build-5195baa60087/output/x86_64-apple-darwin/bitcoin-5195baa60087-x86_64-apple-darwin.tar.gz
214becad22f284eb29ee337acac78f5c2b330529776331147a6312e3072783b0b8  guix-build-5195baa60087/output/x86_64-linux-gnu/SHA256SUMS.part
228bcfa52392628ac42ed37e3bb55a3af0cfdf455c2cdc1f40ae1680302055baf1  guix-build-5195baa60087/output/x86_64-linux-gnu/bitcoin-5195baa60087-x86_64-linux-gnu-debug.tar.gz
230c3b2d9047780afaa2322346507d03f4972c788c61c65523a7b4442d93da63b0  guix-build-5195baa60087/output/x86_64-linux-gnu/bitcoin-5195baa60087-x86_64-linux-gnu.tar.gz
24b7b04e109703e8cae38f5b67eada0c14511a751beeaafd05de53680c77f08f87  guix-build-5195baa60087/output/x86_64-w64-mingw32/SHA256SUMS.part
2542f5616e055991115d51c45013d60184c1e4d1e8b90fc7fcf7c37bbd41a4544a  guix-build-5195baa60087/output/x86_64-w64-mingw32/bitcoin-5195baa60087-win64-debug.zip
26540b6e8c433ddc15698ea0d8102679a859c1f884c8dc0e09910932df9fb600e9  guix-build-5195baa60087/output/x86_64-w64-mingw32/bitcoin-5195baa60087-win64-setup-unsigned.exe
27d65c00ea2e5c6e62d14808947442520d5821429384071a166e43a156708d2414  guix-build-5195baa60087/output/x86_64-w64-mingw32/bitcoin-5195baa60087-win64-unsigned.tar.gz
28e2bf352cf412d254d05676ed506465adc053f84c78588328e2db405f4f520292  guix-build-5195baa60087/output/x86_64-w64-mingw32/bitcoin-5195baa60087-win64.zip
  36. fanquake merged this on May 2, 2024
  37. fanquake closed this on May 2, 2024
  38. fanquake deleted the branch on May 2, 2024


fanquake DrahtBot theuni hebasto TheCharlatan


hebasto

Labels
Build system

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-12-23 03:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me