build: fix libnatpmp macos cross compile #21320

fanquake commented at 7:12 AM on March 1, 2021: member

Currently, our cross-compile of libnatpmp for macOS doesn't work at all. The wrong archiver is used, which produces an archive the linker doesn't like. This becomes clear when configuring:

configure:25722: checking for initnatpmp in -lnatpmp
configure:25747: env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/bin/clang++ --target=x86_64-apple-darwin18 <trim>  -Wl,-headerpad_max_install_names -Wl,-dead_strip -Wl,-dead_strip_dylibs conftest.cpp -lnatpmp   >&5
ld: archive has no table of contents for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Fix this by using the right ar (we do the same for upnp).

While we're at it, fix the build so that we are using our c/ppflags. In practice this basically means building with -O2 rather than -Os.

Note that this fixes an issue that is also fixed by #21209. However, given there are reservations about updating to use a newer libnatpmp source, we should just fix this for now.

build: fix libnatpmp macos cross compile

Currently, our cross-compile of libnatpmp for macOS doesn't work at all.
The wrong archiver is used, which produces an archive the linker doesn't like.
This becomes clear when configuring:
```bash
configure:25722: checking for initnatpmp in -lnatpmp
configure:25747: env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/bin/clang++ --target=x86_64-apple-darwin18 <trim>  -Wl,-headerpad_max_install_names -Wl,-dead_strip -Wl,-dead_strip_dylibs conftest.cpp -lnatpmp   >&5
ld: archive has no table of contents for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

Fix this by using the right `ar` (we do the same for upnp).

While we're at it, we fixe the build so that we are using our c/ppflags.
This  means building with `-O2` rather than `-Os`.

Note that this fixes an issue that is also fixed by #21209.
However, given there are reservations about updating to use a newer libnatpmp source, we should just fix this for now.

bd49ac4168

fanquake added the label Build system on Mar 1, 2021

MarcoFalke added the label Needs gitian build on Mar 1, 2021

MarcoFalke added the label Needs Guix build on Mar 1, 2021

hebasto commented at 8:11 AM on March 1, 2021: member

Concept ACK.

DrahtBot commented at 12:58 PM on March 1, 2021: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#21209 (build: use newer source for libnatpmp 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.

hebasto approved

hebasto commented at 7:59 PM on March 1, 2021: member

ACK bd49ac416881ede3e5132789defd3fec4c6f8685, tested:

cross-compiling -- on Linux Mint 20.1 (x86_64)
binaries -- on macOS Big Sur 11.2.1 (20D74)

Cross-compiling:

master (ad89812c70a9b88be642fceffb970859f6fdcba2):

$ CONFIG_SITE=$PWD/depends/x86_64-apple-darwin16/share/config.site ./configure
...
Options used to compile and link:
...
  with natpmp     = no

this PR (bd49ac416881ede3e5132789defd3fec4c6f8685):

$ CONFIG_SITE=$PWD/depends/x86_64-apple-darwin16/share/config.site ./configure
...
Options used to compile and link:
...
  with natpmp     = yes

Replacement compiler flags s /Os/O2/ could increase code size by using more optimization options. I'm ok with it.

Also added the -pipe flag which has no effect on the generated code at all, only on compiling speed.

hebasto commented at 8:15 PM on March 1, 2021: member

~Backport to 0.21 ?~

fanquake commented at 12:21 AM on March 2, 2021: member

Backport to 0.21 ?

Why? libnatpmp isn't in the 0.21 branch.

hebasto commented at 6:02 AM on March 2, 2021: member

Why? libnatpmp isn't in the 0.21 branch.

Indeed. I messed my memory. Sorry for noise.

DrahtBot commented at 8:46 PM on March 2, 2021: member

Guix builds

File	commit ad89812c70a9b88be642fceffb970859f6fdcba2<br>(master)	commit f8821ec9b8b6494699322d6dd5d1fca65f1b384b<br>(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`580b0205e5f6ab95...`	`6d912ef0a0f2a06b...`
*-aarch64-linux-gnu.tar.gz	`03136fb8a2ce5f88...`	`eff1114521173432...`
*-arm-linux-gnueabihf-debug.tar.gz	`053f6f1bcb7c905c...`	`332d0f142feb4ad7...`
*-arm-linux-gnueabihf.tar.gz	`996df42a3657a608...`	`8bcb682677c350f8...`
*-osx-unsigned.dmg	`50694aa5dbc88eac...`	`facafa690ce23f66...`
*-osx-unsigned.tar.gz	`944075ed83089652...`	`5b1939a89cb6680b...`
*-osx64.tar.gz	`b6a084a966bee2f3...`	`f6d4f89169f72586...`
*-powerpc64-linux-gnu-debug.tar.gz	`7796ea50573078f7...`	`f8bf06814e1993b6...`
*-powerpc64-linux-gnu.tar.gz	`e00672d7411277b3...`	`ff0a88d1972d59d1...`
*-powerpc64le-linux-gnu-debug.tar.gz	`d8d73f5d2ddf14f4...`	`89d4b4f8eac0fabc...`
*-powerpc64le-linux-gnu.tar.gz	`0a9826f7a9526c9d...`	`3d3cb598ad881b75...`
*-riscv64-linux-gnu-debug.tar.gz	`f4875638f5083b34...`	`846a3b9805ab47f5...`
*-riscv64-linux-gnu.tar.gz	`67500503434aeb02...`	`ff39654cf64ed45c...`
*-win-unsigned.tar.gz	`16c5b021d4b012f7...`	`fd3f415e86025a3f...`
*-win64-debug.zip	`2b448f2e34cc993f...`	`914f34e0c01cd37c...`
*-win64-setup-unsigned.exe	`6d9b4d8dd77e886a...`	`e174f622aad0fe40...`
*-win64.zip	`9369f87ed5e94646...`	`f7dbf7a7b7c54cb6...`
*-x86_64-linux-gnu-debug.tar.gz	`e82361425e5cf739...`	`ff1f39905dc48671...`
*-x86_64-linux-gnu.tar.gz	`17deed2695cc41c5...`	`46a81edd8b3b8a9f...`
*.tar.gz	`36675469b1db5989...`	`9ccb7c587f348cbf...`
guix_build.log	`ae0fc0b2f1615242...`	`7e66f2d3f9a92ca8...`
guix_build.log.diff		`5929bef20c26a83b...`

DrahtBot removed the label Needs Guix build on Mar 2, 2021

fanquake merged this on Mar 3, 2021

fanquake closed this on Mar 3, 2021

fanquake deleted the branch on Mar 3, 2021

sidhujag referenced this in commit 9c7d3e6eb6 on Mar 3, 2021

MarcoFalke removed the label Needs gitian build on Mar 5, 2021

kittywhiskers referenced this in commit d23a3c8af8 on Feb 26, 2022

DrahtBot locked this on Aug 16, 2022