depends: remove dependency on Darwin libtool

theuni commented at 4:15 pm on January 11, 2024: member

Split out of #21778.

cctools’ libtool (and llvm’s clone) are unnecessary these days, and were only used (unnecessarily) in miniupnpc and libnatpnp. Both of those projects now have CMake support which eliminates their use.

This PR switches those builds to use CMake, then removes our libtool machinery.

DrahtBot commented at 4:15 pm on January 11, 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	hebasto
Concept ACK	fanquake

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:

#27897 (guix: use GCC 12.3.0 to build releases by fanquake)
#25972 (build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set by fanquake)
#25573 ([POC] guix: produce a fully -static-pie bitcoind by fanquake)
#25391 (guix: Use LTO to build releases by fanquake)
#24123 (build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix) by fanquake)
#21778 (build: LLD based macOS toolchain by fanquake)

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.

DrahtBot added the label Build system on Jan 11, 2024

fanquake commented at 4:22 pm on January 11, 2024: member

Looks good. Have tested this in the LLD PR. Will ACK after rebase and adding the PIC fix for libnatpmp.

theuni force-pushed on Jan 11, 2024

fanquake added the label DrahtBot Guix build requested on Jan 11, 2024

theuni commented at 4:23 pm on January 11, 2024: member

Rebased on master and fixed up pic.

fanquake commented at 4:40 pm on January 11, 2024: member

Actually, you’ll also need to move cmake-minimal from the darwin target into the package list for all targets, in our Guix manifest. Ideally as the first commit. Otherwise this wont Guix build (for non-darwin).

guix: make cmake-minimal available everywhere 008bf95f19

depends: switch libnatpmp to CMake edff61e95b

depends: switch miniupnpc to CMake

Patch taken from upstream:
https://github.com/miniupnp/miniupnp/pull/604

4e0b416d2e

depends: remove (darwin) libtool now that it's no longer used

Note that this is completely unrelated to gnu usage of libtool.

a28c6bbde0

theuni force-pushed on Jan 11, 2024

theuni commented at 5:08 pm on January 11, 2024: member

Actually, you’ll also need to move cmake-minimal from the darwin target into the package list for all targets, in our Guix manifest. Ideally as the first commit. Otherwise this wont Guix build (for non-darwin).

Thanks, done.

fanquake commented at 10:11 am on January 12, 2024: member

Guix build (aarch64 & x86_64):

 073062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
 10965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
 287ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.gz
 3c9bad44733b9f9a90d26e7d82b50bd2d11586723ec2fc330d47924e520d4d4a2  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/SHA256SUMS.part
 46c6d4bfd8e3db6509c0e3710b78b2e78fef7d17f7779db9ad16da4e4987cdb23  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/bitcoin-a28c6bbde0bc-arm-linux-gnueabihf-debug.tar.gz
 529de9c2f7b4196e56a30a31ced90693a5040dca8464383a4989e28bab411a73e  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/bitcoin-a28c6bbde0bc-arm-linux-gnueabihf.tar.gz
 6962d22bb3927fa32b92fb6eca5b9d783bd06eb9543910bd84a916c6664a3fc78  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/SHA256SUMS.part
 757c30bca8156b16b6d49327fb5fa67adade9126553ab4dbbd40eb154bf093bc1  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin-unsigned.tar.gz
 806b291838e73557b6f7d104216b63c1d93da3db8300bb9923e9785efcad87e1c  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin-unsigned.zip
 95046771fa256969d780c479498d22f420cfc59a7a9b52a1cf906d2f0e1d351f1  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin.tar.gz
10459a0433a4b275b38db0010972de6bc2bd81f107f30fdac81c205c7cd2f4202c  guix-build-a28c6bbde0bc/output/dist-archive/bitcoin-a28c6bbde0bc.tar.gz
1134c599140cd538ebedf3e1443eb9ade80a3f649cdfcb56d3adb2bba2c05d1e3d  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/SHA256SUMS.part
123d37246d4b3a9bf64746bae08e25fae938cc11a1d88d5fadd804bbc195adbf26  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64-linux-gnu-debug.tar.gz
134d594e1e7ac8c3dcb41852583a0e574bd919e8d166f1f463bb0f01792e55b35f  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64-linux-gnu.tar.gz
1413f5630869aae0b628d5a7163fed467df05b0d83e9d5ead22db1f57c3915128d  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/SHA256SUMS.part
1526577922d5c1eb9f125b87876ffb3b1572c60597fb362347db97bf2c7acc7824  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64le-linux-gnu-debug.tar.gz
161c1ecb9fca5bed82b5f342310573ad16720516d338dfdf10d8e2dfece18e61d3  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64le-linux-gnu.tar.gz
17a9dd2a4b8e612c1c514e5f08cd14b603bd7e5699319b903c84d93fc6bdd07936  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/SHA256SUMS.part
18b4306afe81a87e3b616ca4da28d17644a9ea1af1fde894a07c1f3aff685d1d38  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/bitcoin-a28c6bbde0bc-riscv64-linux-gnu-debug.tar.gz
198c3fe1305e1030fe5b8d6b519148dc4385428a685e2cbc2c6ea665ac6ebbb594  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/bitcoin-a28c6bbde0bc-riscv64-linux-gnu.tar.gz
20a7eefad4e2d73c46ffa711d16901348ee9b353ce8f5d9f3b77ff379472ff938a  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/SHA256SUMS.part
21e38858ef2a2a45cb2a882eb7921f3db28492e28c5f9ee9782346a4155f3dc983  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin-unsigned.tar.gz
228623edd712e843a1186175e89fadf1b78e67ace0f8daec1f9eb6e3981d4058f8  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin-unsigned.zip
231031b5fe622ffc1aa0adfdb57cfe271e2c09226d99e1389fcd9204c2c839c400  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin.tar.gz
2429989368d745d3fd84b02839edff446f2bc8be877eeb0067d8f65e0eab0f9093  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/SHA256SUMS.part
254f879a6d03df624ffb5c0ba9de2547ff3fea5692c184658feb38838911627214  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/bitcoin-a28c6bbde0bc-x86_64-linux-gnu-debug.tar.gz
269c5d3373134aefd4d2144c4913436e0a75fff73c977b0f54a3f9d25b18b580da  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/bitcoin-a28c6bbde0bc-x86_64-linux-gnu.tar.gz
2785d5d934d4ae09426bf6235b6e81b3ed722905a4eaf5169212d712fac08754a4  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/SHA256SUMS.part
28d7a6e9bea26b1b856d8017d8f1fb17b5868080e2d0971e746b2d1157f6c2cb1e  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-debug.zip
29b93f1e6c307cefbba41bb274d6e49673530f930e37cd9919317da0678a0d1c3c  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-setup-unsigned.exe
307cb9624ca622e5430dc4aef39ca30a3858ee558c2eff8a77e00bbc212b8661e1  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-unsigned.tar.gz
31f3f1fba109ebfe22fa74930a80902e868b0640413cc1d52297d060977e1e6e4d  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64.zip

DrahtBot commented at 11:26 am on January 12, 2024: contributor

Guix builds (on x86_64)

File	commit 4baa162dbb3c6464e196a4a21fe63794859021b4(master)	commit d51f474cde8511107ec4524b3bac51381d5987a4(master and this pull)
SHA256SUMS.part	`ed7581de976a7f8f...`	`6db248e85f53dff6...`
*-aarch64-linux-gnu-debug.tar.gz	`906c468cce7f7696...`	`e32209d82910bdc6...`
*-aarch64-linux-gnu.tar.gz	`328f06ece720f6a8...`	`486842d1032af8dd...`
*-arm-linux-gnueabihf-debug.tar.gz	`16d184b07787265a...`	`29ce22105bdb89d8...`
*-arm-linux-gnueabihf.tar.gz	`74dbdd6eec952600...`	`5098c9870212bad3...`
*-arm64-apple-darwin-unsigned.tar.gz	`3e1fe9185837c6d5...`	`ab67a804c82b182e...`
*-arm64-apple-darwin-unsigned.zip	`5c58e1cccccd5bbd...`	`c2e3767b6eceab80...`
*-arm64-apple-darwin.tar.gz	`7f1912cc0dab857c...`	`0484d5f5945cf5cf...`
*-powerpc64-linux-gnu-debug.tar.gz	`ad504aa6cf46a010...`	`700bc50b60c09f9f...`
*-powerpc64-linux-gnu.tar.gz	`e6835c285c384cd1...`	`74ca9258b5c90bbe...`
*-powerpc64le-linux-gnu-debug.tar.gz	`436a64f3c52bcc50...`	`b2ddfea22148971c...`
*-powerpc64le-linux-gnu.tar.gz	`723337e91f6c7aab...`	`392fffc7e40a0ed3...`
*-riscv64-linux-gnu-debug.tar.gz	`6438d68de4712930...`	`ec830d4f9fed79c2...`
*-riscv64-linux-gnu.tar.gz	`8694ec5acf8e3f2e...`	`344556e7d32b2206...`
*-x86_64-apple-darwin-unsigned.tar.gz	`41f58ac8864e403d...`	`d023ba2774afad53...`
*-x86_64-apple-darwin-unsigned.zip	`83a152bccb3e86e2...`	`f0040b89efb49781...`
*-x86_64-apple-darwin.tar.gz	`1038f4e9637125b2...`	`11a9a8cd3546b2e5...`
*-x86_64-linux-gnu-debug.tar.gz	`e500bc3ea05a3528...`	`c25bcd3f88aa2c6c...`
*-x86_64-linux-gnu.tar.gz	`f0f11506b593a248...`	`eb45a762d9886af6...`
*.tar.gz	`b757aef5514ef31b...`	`9a4b350538e6a2db...`
guix_build.log	`3d9641ff8342529f...`	`a9482d2bbe5eb7e2...`
guix_build.log.diff		`e31b2c39aa6b09fe...`

DrahtBot removed the label DrahtBot Guix build requested on Jan 12, 2024

TheCharlatan commented at 10:45 am on January 13, 2024: contributor

Guix builds (x86):

 073062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
 10965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
 287ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745  guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.gz
 3c9bad44733b9f9a90d26e7d82b50bd2d11586723ec2fc330d47924e520d4d4a2  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/SHA256SUMS.part
 46c6d4bfd8e3db6509c0e3710b78b2e78fef7d17f7779db9ad16da4e4987cdb23  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/bitcoin-a28c6bbde0bc-arm-linux-gnueabihf-debug.tar.gz
 529de9c2f7b4196e56a30a31ced90693a5040dca8464383a4989e28bab411a73e  guix-build-a28c6bbde0bc/output/arm-linux-gnueabihf/bitcoin-a28c6bbde0bc-arm-linux-gnueabihf.tar.gz
 6962d22bb3927fa32b92fb6eca5b9d783bd06eb9543910bd84a916c6664a3fc78  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/SHA256SUMS.part
 757c30bca8156b16b6d49327fb5fa67adade9126553ab4dbbd40eb154bf093bc1  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin-unsigned.tar.gz
 806b291838e73557b6f7d104216b63c1d93da3db8300bb9923e9785efcad87e1c  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin-unsigned.zip
 95046771fa256969d780c479498d22f420cfc59a7a9b52a1cf906d2f0e1d351f1  guix-build-a28c6bbde0bc/output/arm64-apple-darwin/bitcoin-a28c6bbde0bc-arm64-apple-darwin.tar.gz
10459a0433a4b275b38db0010972de6bc2bd81f107f30fdac81c205c7cd2f4202c  guix-build-a28c6bbde0bc/output/dist-archive/bitcoin-a28c6bbde0bc.tar.gz
1134c599140cd538ebedf3e1443eb9ade80a3f649cdfcb56d3adb2bba2c05d1e3d  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/SHA256SUMS.part
123d37246d4b3a9bf64746bae08e25fae938cc11a1d88d5fadd804bbc195adbf26  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64-linux-gnu-debug.tar.gz
134d594e1e7ac8c3dcb41852583a0e574bd919e8d166f1f463bb0f01792e55b35f  guix-build-a28c6bbde0bc/output/powerpc64-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64-linux-gnu.tar.gz
1413f5630869aae0b628d5a7163fed467df05b0d83e9d5ead22db1f57c3915128d  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/SHA256SUMS.part
1526577922d5c1eb9f125b87876ffb3b1572c60597fb362347db97bf2c7acc7824  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64le-linux-gnu-debug.tar.gz
161c1ecb9fca5bed82b5f342310573ad16720516d338dfdf10d8e2dfece18e61d3  guix-build-a28c6bbde0bc/output/powerpc64le-linux-gnu/bitcoin-a28c6bbde0bc-powerpc64le-linux-gnu.tar.gz
17a9dd2a4b8e612c1c514e5f08cd14b603bd7e5699319b903c84d93fc6bdd07936  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/SHA256SUMS.part
18b4306afe81a87e3b616ca4da28d17644a9ea1af1fde894a07c1f3aff685d1d38  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/bitcoin-a28c6bbde0bc-riscv64-linux-gnu-debug.tar.gz
198c3fe1305e1030fe5b8d6b519148dc4385428a685e2cbc2c6ea665ac6ebbb594  guix-build-a28c6bbde0bc/output/riscv64-linux-gnu/bitcoin-a28c6bbde0bc-riscv64-linux-gnu.tar.gz
20a7eefad4e2d73c46ffa711d16901348ee9b353ce8f5d9f3b77ff379472ff938a  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/SHA256SUMS.part
21e38858ef2a2a45cb2a882eb7921f3db28492e28c5f9ee9782346a4155f3dc983  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin-unsigned.tar.gz
228623edd712e843a1186175e89fadf1b78e67ace0f8daec1f9eb6e3981d4058f8  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin-unsigned.zip
231031b5fe622ffc1aa0adfdb57cfe271e2c09226d99e1389fcd9204c2c839c400  guix-build-a28c6bbde0bc/output/x86_64-apple-darwin/bitcoin-a28c6bbde0bc-x86_64-apple-darwin.tar.gz
2429989368d745d3fd84b02839edff446f2bc8be877eeb0067d8f65e0eab0f9093  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/SHA256SUMS.part
254f879a6d03df624ffb5c0ba9de2547ff3fea5692c184658feb38838911627214  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/bitcoin-a28c6bbde0bc-x86_64-linux-gnu-debug.tar.gz
269c5d3373134aefd4d2144c4913436e0a75fff73c977b0f54a3f9d25b18b580da  guix-build-a28c6bbde0bc/output/x86_64-linux-gnu/bitcoin-a28c6bbde0bc-x86_64-linux-gnu.tar.gz
2785d5d934d4ae09426bf6235b6e81b3ed722905a4eaf5169212d712fac08754a4  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/SHA256SUMS.part
28d7a6e9bea26b1b856d8017d8f1fb17b5868080e2d0971e746b2d1157f6c2cb1e  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-debug.zip
29b93f1e6c307cefbba41bb274d6e49673530f930e37cd9919317da0678a0d1c3c  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-setup-unsigned.exe
307cb9624ca622e5430dc4aef39ca30a3858ee558c2eff8a77e00bbc212b8661e1  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64-unsigned.tar.gz
31f3f1fba109ebfe22fa74930a80902e868b0640413cc1d52297d060977e1e6e4d  guix-build-a28c6bbde0bc/output/x86_64-w64-mingw32/bitcoin-a28c6bbde0bc-win64.zip

hebasto commented at 10:46 am on January 13, 2024: member

Concept ACK.

in depends/packages/miniupnpc.mk:32 in a28c6bbde0

37 
38 define $(package)_stage_cmds
39-	mkdir -p $($(package)_staging_prefix_dir)/include/miniupnpc $($(package)_staging_prefix_dir)/lib &&\
40-	install *.h $($(package)_staging_prefix_dir)/include/miniupnpc &&\
41-	install libminiupnpc.a $($(package)_staging_prefix_dir)/lib
42+	$(MAKE) DESTDIR=$($(package)_staging_dir) install

hebasto commented at 3:32 pm on January 13, 2024:

nit: Using the underlying build system call directly makes CMake re-build all targets again. That could be avoided by using CMake’s invocation like that:

0	cmake --install . --prefix $($(package)_staging_prefix_dir)

Could be verified by building sequentially the miniupnpc_built and miniupnpc_staged targets.

fanquake commented at 5:17 pm on March 22, 2024:

Incorporated into #29707.

in depends/packages/miniupnpc.mk:15 in a28c6bbde0

15-$(package)_build_opts_mingw32=-f Makefile.mingw CFLAGS="$($(package)_cflags) -D_WIN32_WINNT=0x0601"
16-$(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)"
17+$(package)_config_opts = -DUPNPC_BUILD_SAMPLE=OFF -DUPNPC_BUILD_SHARED=OFF
18+$(package)_config_opts += -DUPNPC_BUILD_STATIC=ON -DUPNPC_BUILD_TESTS=OFF
19+$(package)_config_opts += -DCMAKE_POSITION_INDEPENDENT_CODE=ON
20+$(package)_cppflags_mingw32=-U_WIN32_WINNT -D_WIN32_WINNT=0x0601

hebasto commented at 3:45 pm on January 13, 2024:

nit: Maybe add a comment that this line might be revised in the future considering https://github.com/miniupnp/miniupnp/pull/659?

fanquake commented at 10:08 am on January 15, 2024:

It’s weird that miniupnpc would add a special build system option for setting a single, Windows-only define. It’s not really clear what the benefits of that are over just using CPPFLAGS, which should always work. The only rationale from the PR author for not doing that was I don't like it..

fanquake commented at 5:18 pm on March 22, 2024:

Also incoroprated this in #29707.

in depends/patches/miniupnpc/fix_mingw_staticlib.patch:16 in a28c6bbde0

11+
12+diff --git a/miniupnpc/CMakeLists.txt b/miniupnpc/CMakeLists.txt
13+index 433fc732..007ac729 100644
14+--- a/miniupnpc/CMakeLists.txt
15++++ b/miniupnpc/CMakeLists.txt
16+@@ -99,7 +99,7 @@ if (UPNPC_BUILD_STATIC)

hebasto commented at 3:49 pm on January 13, 2024:

0@@ -116,7 +116,7 @@ if (UPNPC_BUILD_STATIC)

to avoid Hunk [#1](/bitcoin-bitcoin/1/) succeeded at 116 (offset 17 lines).

in depends/packages/libnatpmp.mk:22 in a28c6bbde0

23 endef
24 
25 define $(package)_stage_cmds
26   mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib &&\
27-  install *.h $($(package)_staging_prefix_dir)/include &&\
28+  install ../*.h $($(package)_staging_prefix_dir)/include &&\

hebasto commented at 4:04 pm on January 13, 2024:

nit: Maybe skip headers that are used only to build the package:

0  install ../natpmp.h  ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include &&\

?

hebasto approved

hebasto commented at 4:26 pm on January 13, 2024: member

ACK a28c6bbde0bcd064869cf861d92a6e23df4a7b73

DrahtBot requested review from fanquake on Jan 13, 2024

DrahtBot added the label CI failed on Jan 15, 2024

fanquake commented at 1:47 pm on January 18, 2024: member

Looks good.

I spoke too soon. There are multiple issues here.

The main one is differences in the upstream Autotools & CMake buildsystems that make master and this PR produce completely different output. One example, when building miniupnpc, its Autotools build sets MINIUPNPC_GET_SRC_ADDR, which means recvfrom is used. However its CMake build does not, meaning recv is used instead. i.e:

0# master
1nm -C depends/aarch64-unknown-linux-gnu/lib/libminiupnpc.a | grep recv
2                 U recvfrom
3# this PR
4nm -C depends/aarch64-unknown-linux-gnu/lib64/libminiupnpc.a | grep recv
5                 U recv

So CMake isn’t a drop-in replacement; and I’m assuming there will be other subtle differences (which may or may not matter to us). Anything we find are probably bugs that should be fixed upstream, unless they are not concerned about keeping the two build systems in sync?

Another issue is the same one that we had to fix in #28846. The upstream CMake build system uses GNUInstallDirs. So as you can see in the output above, with this PR, libs might end up in /lib64, depending on the system, which means we won’t find them:

0checking for miniupnpc/miniupnpc.h... yes
1checking for miniupnpc/upnpcommands.h... yes
2checking for miniupnpc/upnperrors.h... yes
3checking for upnpDiscover in -lminiupnpc... no
4...
5  with upnp       = no

theuni commented at 9:24 pm on January 23, 2024: member

Closing in favor of #29298.

theuni closed this on Jan 23, 2024

fanquake referenced this in commit 478ac185be on Jan 29, 2024

fanquake commented at 4:40 pm on March 22, 2024: member

However its CMake build does not, meaning recv is used instead. i.e:

Sent a change upstream https://github.com/miniupnp/miniupnp/pull/721

theuni commented at 4:57 pm on March 22, 2024: member

Nice, thanks!

depends: remove dependency on Darwin libtool #29232

Code Coverage

Reviews

Conflicts

Guix builds (on x86_64)