Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp #22397

Merged
merged 1 commit into from
Sep 6, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jul 3, 2021

On master (7a49fdc) the configure script does not pick up Homebrew's miniupnpc and libnatpmp packages on macOS Apple Silicon:

% ./configure --with-miniupnpc
...
checking for miniupnpc/miniupnpc.h... no
checking for miniupnpc/upnpcommands.h... no
checking for miniupnpc/upnperrors.h... no
...
checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc."
% ./configure --with-natpmp
...
checking for natpmp.h... no
...
checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp

The preferred Homebrew prefix for Apple Silicon is /opt/homebrew. Therefore, if we do not use pkg-config to detect packages, we should set the CPPFLAGS and LDFLAGS variables for them explicitly.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hebasto
Copy link
Member Author

hebasto commented Jul 4, 2021

@jarolrod

Why can't we do something similar to what was done in #21231

Specifically:
https://github.com/bitcoin/bitcoin/pull/21231/files#diff-c9b3549bbf489196eaa62e32081bf66c102029f7cfe01c8782eb18bcf7256805R230

Could you elaborate your idea?

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 005e60e

Acking because this is a problem on master and the pr does in fact fix this. I'm not an expert on our build system but this looks a bit hacky to me.

There must be simpler way to look into opt/homebrew to find these packages. Friendly ping to @fanquake for his opinion.

Testing on an M1 machine:

master:

with upnp       = no
with natpmp     = no

pr:

with upnp       = yes
with natpmp     = yes

@hebasto
Copy link
Member Author

hebasto commented Jul 19, 2021

@jarolrod

There must be simpler way to look into opt/homebrew to find these packages.

The simpler way is to use pkg-config, but "we do not use pkg-config to detect" miniupnpc and libnatpmp packages.

@hebasto
Copy link
Member Author

hebasto commented Jul 21, 2021

Rebased on top of the recent changes in the Guix stuff.

@hebasto
Copy link
Member Author

hebasto commented Jul 21, 2021

Guix builds:

$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
f58f25cb7746687eddf59b1e97deb8b031b6097a80cf34465e64e14bff5b70ca  guix-build-068d2cea60f3/output/aarch64-linux-gnu/SHA256SUMS.part
a82b5a49b96d8d7ec327b891db3ee41b9f19155ebd9f459f77c6e9e70dfd26fd  guix-build-068d2cea60f3/output/aarch64-linux-gnu/bitcoin-068d2cea60f3-aarch64-linux-gnu-debug.tar.gz
9609823f91e1d003474ed24e1abd57af53d73332b2d40459d6ac5fe03e973ad8  guix-build-068d2cea60f3/output/aarch64-linux-gnu/bitcoin-068d2cea60f3-aarch64-linux-gnu.tar.gz
e5ec39ec59b6803dbd40745c4624c71d9de3357ae5c08d622818156c25eb10b2  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/SHA256SUMS.part
72d49e54b8ea12da6da5ad8480f95e6b378c7e2442436e3bfd505db6f9dfa115  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/bitcoin-068d2cea60f3-arm-linux-gnueabihf-debug.tar.gz
95228fb8fbee6ed092707cb76db0f592fffbcecb38f0030976df1db4fe2aab14  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/bitcoin-068d2cea60f3-arm-linux-gnueabihf.tar.gz
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  guix-build-068d2cea60f3/output/dist-archive/SKIPATTEST.TAG
a0ca44188aeb859b07b772549933717943f525d18f9c1517d69571a994d5ef21  guix-build-068d2cea60f3/output/dist-archive/bitcoin-068d2cea60f3.tar.gz
fd03066327cf0a2eaca2654fec2f439d85928f22d130d39573dd9423db28b401  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/SHA256SUMS.part
20fe924cb9944f460d4a0a38d371d4424d95cc243df62c9d1ecd13e56eb84e81  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/bitcoin-068d2cea60f3-powerpc64-linux-gnu-debug.tar.gz
a4c1e2582aed3c0542a83cad43d4b95683c48bca93f1e1360748164de1a17c07  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/bitcoin-068d2cea60f3-powerpc64-linux-gnu.tar.gz
24b809e9fc2a1879132ff3e98519fa4063ffdf4f58dcc4d56670023a96e664db  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/SHA256SUMS.part
12a5b4cd60212cb35e8d2b397573947d58a4b3a4458afb5cafc4a0226e48cbad  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/bitcoin-068d2cea60f3-powerpc64le-linux-gnu-debug.tar.gz
99a28ec60d16486fe634a5657b5da53331c32cb8d22933d3dcd84b545146182d  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/bitcoin-068d2cea60f3-powerpc64le-linux-gnu.tar.gz
e1334323c034c6dbb7f5ff974a47267c5310786591efd99c11db059a2025078c  guix-build-068d2cea60f3/output/riscv64-linux-gnu/SHA256SUMS.part
ba8d1150632a624410e186733d258ad44aa9265f81620b18e0ec2dcf312c3e2f  guix-build-068d2cea60f3/output/riscv64-linux-gnu/bitcoin-068d2cea60f3-riscv64-linux-gnu-debug.tar.gz
af79b74aa6c97cf3e44e042c45ee40d6b147ef5eccde0c2ea31500ee3ebb80ad  guix-build-068d2cea60f3/output/riscv64-linux-gnu/bitcoin-068d2cea60f3-riscv64-linux-gnu.tar.gz
022470de00875763563be893f65ebfddd57e7cd1e99aa83acd66b5092200542b  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/SHA256SUMS.part
619de3c3e1fd3f6f98252c614b15b964de5f326129614c1cd72f1e451ed1b17c  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx-unsigned.dmg
49eb100ea32d786432b4a6b6cf1a10045352a44adad6bba00ea05bf965abe653  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx-unsigned.tar.gz
18dea96c65e350e7e460b5294ab2086bed09d880dd16a37c77afecb261ab00b0  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx64.tar.gz
d3d98765e4cb5f452f99020c0f8f05713a25f58695e4d3559403e0b98b7000d7  guix-build-068d2cea60f3/output/x86_64-linux-gnu/SHA256SUMS.part
4b92763daa77255f5944fb3a70f7755820ac2c3992ec3037e004f9c82d564d19  guix-build-068d2cea60f3/output/x86_64-linux-gnu/bitcoin-068d2cea60f3-x86_64-linux-gnu-debug.tar.gz
922a56e7141b6d70d2aa9b96b78d847c76e0c5dd3532e31cbc81e1e015170a36  guix-build-068d2cea60f3/output/x86_64-linux-gnu/bitcoin-068d2cea60f3-x86_64-linux-gnu.tar.gz
2ca1290902bcecc74acf4e98cfecd6c778fff3b3115b12251a3b90870df47b2c  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/SHA256SUMS.part
4cc52df718adbde0f4847e28e52c2e323d80c7d171f9af26683e1865993d9e1c  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win-unsigned.tar.gz
9cf614924f4d708e63aea8d4ff4da648b10cdca7600be215ffe23ce613652f90  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64-debug.zip
b868b39e72f377d1a23990f25ef884b2f1d2cf3b0275fe6c291e469d3f3d28f7  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64-setup-unsigned.exe
7dd7bc927cc4f3f9d9a01c05088c2a7df4ede02ba0a72b9efab6436146929e6f  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64.zip

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK 068d2ce

Tested again after rebase. Confirmed clean rebase between 005e60e and 068d2ce.

on an Apple M1 Machine w/ macOS 11.4:
master:

with upnp       = no
with natpmp     = no

pr:

with upnp       = yes
with natpmp     = yes

Guix Hashes:
My guix hashes match @hebasto

f58f25cb7746687eddf59b1e97deb8b031b6097a80cf34465e64e14bff5b70ca  guix-build-068d2cea60f3/output/aarch64-linux-gnu/SHA256SUMS.part
a82b5a49b96d8d7ec327b891db3ee41b9f19155ebd9f459f77c6e9e70dfd26fd  guix-build-068d2cea60f3/output/aarch64-linux-gnu/bitcoin-068d2cea60f3-aarch64-linux-gnu-debug.tar.gz
9609823f91e1d003474ed24e1abd57af53d73332b2d40459d6ac5fe03e973ad8  guix-build-068d2cea60f3/output/aarch64-linux-gnu/bitcoin-068d2cea60f3-aarch64-linux-gnu.tar.gz
e5ec39ec59b6803dbd40745c4624c71d9de3357ae5c08d622818156c25eb10b2  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/SHA256SUMS.part
72d49e54b8ea12da6da5ad8480f95e6b378c7e2442436e3bfd505db6f9dfa115  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/bitcoin-068d2cea60f3-arm-linux-gnueabihf-debug.tar.gz
95228fb8fbee6ed092707cb76db0f592fffbcecb38f0030976df1db4fe2aab14  guix-build-068d2cea60f3/output/arm-linux-gnueabihf/bitcoin-068d2cea60f3-arm-linux-gnueabihf.tar.gz
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  guix-build-068d2cea60f3/output/dist-archive/SKIPATTEST.TAG
a0ca44188aeb859b07b772549933717943f525d18f9c1517d69571a994d5ef21  guix-build-068d2cea60f3/output/dist-archive/bitcoin-068d2cea60f3.tar.gz
fd03066327cf0a2eaca2654fec2f439d85928f22d130d39573dd9423db28b401  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/SHA256SUMS.part
20fe924cb9944f460d4a0a38d371d4424d95cc243df62c9d1ecd13e56eb84e81  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/bitcoin-068d2cea60f3-powerpc64-linux-gnu-debug.tar.gz
a4c1e2582aed3c0542a83cad43d4b95683c48bca93f1e1360748164de1a17c07  guix-build-068d2cea60f3/output/powerpc64-linux-gnu/bitcoin-068d2cea60f3-powerpc64-linux-gnu.tar.gz
24b809e9fc2a1879132ff3e98519fa4063ffdf4f58dcc4d56670023a96e664db  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/SHA256SUMS.part
12a5b4cd60212cb35e8d2b397573947d58a4b3a4458afb5cafc4a0226e48cbad  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/bitcoin-068d2cea60f3-powerpc64le-linux-gnu-debug.tar.gz
99a28ec60d16486fe634a5657b5da53331c32cb8d22933d3dcd84b545146182d  guix-build-068d2cea60f3/output/powerpc64le-linux-gnu/bitcoin-068d2cea60f3-powerpc64le-linux-gnu.tar.gz
e1334323c034c6dbb7f5ff974a47267c5310786591efd99c11db059a2025078c  guix-build-068d2cea60f3/output/riscv64-linux-gnu/SHA256SUMS.part
ba8d1150632a624410e186733d258ad44aa9265f81620b18e0ec2dcf312c3e2f  guix-build-068d2cea60f3/output/riscv64-linux-gnu/bitcoin-068d2cea60f3-riscv64-linux-gnu-debug.tar.gz
af79b74aa6c97cf3e44e042c45ee40d6b147ef5eccde0c2ea31500ee3ebb80ad  guix-build-068d2cea60f3/output/riscv64-linux-gnu/bitcoin-068d2cea60f3-riscv64-linux-gnu.tar.gz
022470de00875763563be893f65ebfddd57e7cd1e99aa83acd66b5092200542b  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/SHA256SUMS.part
619de3c3e1fd3f6f98252c614b15b964de5f326129614c1cd72f1e451ed1b17c  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx-unsigned.dmg
49eb100ea32d786432b4a6b6cf1a10045352a44adad6bba00ea05bf965abe653  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx-unsigned.tar.gz
18dea96c65e350e7e460b5294ab2086bed09d880dd16a37c77afecb261ab00b0  guix-build-068d2cea60f3/output/x86_64-apple-darwin18/bitcoin-068d2cea60f3-osx64.tar.gz
d3d98765e4cb5f452f99020c0f8f05713a25f58695e4d3559403e0b98b7000d7  guix-build-068d2cea60f3/output/x86_64-linux-gnu/SHA256SUMS.part
4b92763daa77255f5944fb3a70f7755820ac2c3992ec3037e004f9c82d564d19  guix-build-068d2cea60f3/output/x86_64-linux-gnu/bitcoin-068d2cea60f3-x86_64-linux-gnu-debug.tar.gz
922a56e7141b6d70d2aa9b96b78d847c76e0c5dd3532e31cbc81e1e015170a36  guix-build-068d2cea60f3/output/x86_64-linux-gnu/bitcoin-068d2cea60f3-x86_64-linux-gnu.tar.gz
2ca1290902bcecc74acf4e98cfecd6c778fff3b3115b12251a3b90870df47b2c  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/SHA256SUMS.part
4cc52df718adbde0f4847e28e52c2e323d80c7d171f9af26683e1865993d9e1c  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win-unsigned.tar.gz
9cf614924f4d708e63aea8d4ff4da648b10cdca7600be215ffe23ce613652f90  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64-debug.zip
b868b39e72f377d1a23990f25ef884b2f1d2cf3b0275fe6c291e469d3f3d28f7  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64-setup-unsigned.exe
7dd7bc927cc4f3f9d9a01c05088c2a7df4ede02ba0a72b9efab6436146929e6f  guix-build-068d2cea60f3/output/x86_64-w64-mingw32/bitcoin-068d2cea60f3-win64.zip

@hebasto
Copy link
Member Author

hebasto commented Jul 21, 2021

@fanquake @promag (as you have M1-based stuff at your disposal)

Do you mind having a look into this PR?

Copy link
Contributor

@Zero-1729 Zero-1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 068d2ce

Tested on an M1 Machine running macOS 11.4. I Can confirm the patch works; the new Hombrew path (/opt/homebrew/) is searched for the appropriate packages when configuring with miniupnpc or natpmp.

Currently on master:

$ ./configure --with-miniupnpc
...
checking for miniupnpc/miniupnpc.h... no
checking for miniupnpc/upnpcommands.h... no
checking for miniupnpc/upnperrors.h... no
...
checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc."
$ ./configure --with-natpmp
...
checking for natpmp.h... no
...
checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp

After this patch:

$ ./configure --with-miniupnpc
...
checking for miniupnpc/miniupnpc.h... yes
checking for upnpDiscover in -lminiupnpc... yes
checking for miniupnpc/upnpcommands.h... yes
checking for upnpDiscover in -lminiupnpc... (cached) yes
checking for miniupnpc/upnperrors.h... yes
checking for upnpDiscover in -lminiupnpc... (cached) yes
checking whether miniUPnPc API version is supported... yes
...
checking whether to build with support for UPnP... yes
checking whether to build with UPnP enabled by default... no
...
Options used to compile and link:
...
with upnp       = yes
...
target os       = darwin
build os        = darwin20.5.0
$ ./configure --with-natpmp
...
checking for natpmp.h... yes
checking for initnatpmp in -lnatpmp... yes
...
checking whether to build with support for NAT-PMP... yes
checking whether to build with NAT-PMP enabled by default... no
...
Options used to compile and link:
...
with natpmp     = yes
...
target os       = darwin
build os        = darwin20.5.0

@RandyMcMillan
Copy link
Contributor

tACK 068d2ce

Also proactively tested with #22506 here -> pr22397-xmac

Remote Desktop Picture July 22, 2021 at 12 56 50 PM EDT

configure.ac Outdated
@@ -699,6 +699,28 @@ case $host in
if $BREW list --versions qt5 >/dev/null; then
export PKG_CONFIG_PATH="$($BREW --prefix qt5 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
fi

dnl The preferred Homebrew prefix for Apple Silicon is /opt/homebrew.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment could be improved, it mentions Apple Silicon, but the code introduced here isn't specific to it. That also means these flags are going to get added to all macOS (host) builds, even when they aren't necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Updated.

The preferred Homebrew prefix for Apple Silicon is /opt/homebrew.
Therefore, if we do not use pkg-config to detect packages, we should set
the CPPFLAGS and LDFLAGS variables for them explicitly.
@hebasto
Copy link
Member Author

hebasto commented Jul 29, 2021

Updated 068d2ce -> 2445df4 (pr22397.03 -> pr22397.04, diff):

... it mentions Apple Silicon, but the code introduced here isn't specific to it. That also means these flags are going to get added to all macOS (host) builds, even when they aren't necessary.

@hebasto
Copy link
Member Author

hebasto commented Jul 29, 2021

Guix builds:

$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
68494ee7382a1c0125c3f4c135f32ffb25789a6adc99b250eb8a02ad6302e6e3  guix-build-2445df4eb36b/output/aarch64-linux-gnu/SHA256SUMS.part
4cb8f30bc9eac086391f886969237ebdd47c84f5bc3370deebb9d397b4467a63  guix-build-2445df4eb36b/output/aarch64-linux-gnu/bitcoin-2445df4eb36b-aarch64-linux-gnu-debug.tar.gz
4d5995f680b504400f7851b9db412294aa9efadbb87a711abcc6fee31167e56f  guix-build-2445df4eb36b/output/aarch64-linux-gnu/bitcoin-2445df4eb36b-aarch64-linux-gnu.tar.gz
46d6337d02c53da14991afbe9b6b84121fcd06374f4144cc8cb85137cc9a8ce3  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/SHA256SUMS.part
90bb08c733ac0808e2224e3b5ea9b166cbed2850924582ff0063be09b86e6f82  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/bitcoin-2445df4eb36b-arm-linux-gnueabihf-debug.tar.gz
8aab610762d674c8db0de4e76712d5d79d0de3e24bb37810287313d3942b7fd0  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/bitcoin-2445df4eb36b-arm-linux-gnueabihf.tar.gz
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  guix-build-2445df4eb36b/output/dist-archive/SKIPATTEST.TAG
894d082d230643381b63d31620fd4fe76c031de41c49edef9ef3de19740bb860  guix-build-2445df4eb36b/output/dist-archive/bitcoin-2445df4eb36b.tar.gz
9e5180ccee019607391373797a1b048f3a971124bb4ebc19395d9c92b2d798a1  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/SHA256SUMS.part
daa9c3ae79a157cba9ed059a0a63077dd5d9da432f4aec9874814235959435c9  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/bitcoin-2445df4eb36b-powerpc64-linux-gnu-debug.tar.gz
25fd2dffbbc717aee2cfad8f371fb90130b79271d301b772de6ededf22e8c1ee  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/bitcoin-2445df4eb36b-powerpc64-linux-gnu.tar.gz
2d7fa025b249cffd98fd7860dd1ee0efd2659941b12b5f0165e5b50c71761c52  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/SHA256SUMS.part
a68763b780b942912410b986fade2f39de1fa5d1827a89aed5afba13b9774126  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/bitcoin-2445df4eb36b-powerpc64le-linux-gnu-debug.tar.gz
159cfdfd2915604aa94b5154e5a5fc2acf7ec800644ade880e076c2549ae241a  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/bitcoin-2445df4eb36b-powerpc64le-linux-gnu.tar.gz
c9be12a9afb44f18dbbee9dcad588c6261df6a6933007e8bdc51b471089b08ae  guix-build-2445df4eb36b/output/riscv64-linux-gnu/SHA256SUMS.part
6a17f1d98e6fa1a8254e1e75720b93c1c5d1c8bc96f256a67a3677146535a07f  guix-build-2445df4eb36b/output/riscv64-linux-gnu/bitcoin-2445df4eb36b-riscv64-linux-gnu-debug.tar.gz
149df6c74388d1fe0941d5070e70a07125399a6a67910cc85f7d03637853c6fe  guix-build-2445df4eb36b/output/riscv64-linux-gnu/bitcoin-2445df4eb36b-riscv64-linux-gnu.tar.gz
a0d7e056abdc2f11f1aac5ae60188992925cb07cfb61411baac3893d8c2a49f5  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/SHA256SUMS.part
4926af32d669f82a70f4e092bc6c8da1ea09ffe02c8a8ab3388627e61b787f17  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx-unsigned.dmg
16796d7c530d4d56af88cc8de1a4488ffc376482a129e46af12093887f2b76f4  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx-unsigned.tar.gz
9845d32d2b8a00e0cd7c51cc07e88aecb7cc48de92fc827eddf749ea8951de50  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx64.tar.gz
74eb5703182e303d2fea043202208f802134bd425113a0439225d689e70c2509  guix-build-2445df4eb36b/output/x86_64-linux-gnu/SHA256SUMS.part
a3eb9fabc0d4ea53fa41555a3a3d5380e47a7629aed20e7463d92cd005ff1e17  guix-build-2445df4eb36b/output/x86_64-linux-gnu/bitcoin-2445df4eb36b-x86_64-linux-gnu-debug.tar.gz
13c1cc6fb583476dc52e904d39f8a0de125b20c7016fea614349836756335cb5  guix-build-2445df4eb36b/output/x86_64-linux-gnu/bitcoin-2445df4eb36b-x86_64-linux-gnu.tar.gz
8502bc1a8129d1c2da750f88a5afd429778689f7b4c9a572df466985f317ebb2  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/SHA256SUMS.part
1ceb92ab15922a7ce40041cfd14d561ba0b8cfac278ce6df1f4b45f94f344792  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win-unsigned.tar.gz
4531669ea4ff2f448c785bf797ed62b15a69729f24d86cc8a72da888d304a687  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64-debug.zip
69de6d1aac5862bd5f3646f344efeacac104f97e8a66409cc21c208043d283eb  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64-setup-unsigned.exe
9a131b9adb3b3e25e937eb93afda333c86143328cea11e71ed4da10a79893794  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64.zip

Copy link
Contributor

@Zero-1729 Zero-1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-tACK 2445df4 (re-tested on an M1 Machine running macOS 11.4).

Clean rebase, the patch still works as expected. More importantly, flags now only affect Apple Silicon machines (great catch @fanquake!).

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK 2445df4

re-tested functionality again after changes on an M1 mac. Since my last review we have moved the logic under a aarch64 host case. This wouldn't branch out to any other aarch64 cases since we are under a darwin target. This is one way to resolve feedback on the comment used here :).

Additionally, i tested to ensure there are no weird artifacts arising from this change on an intel mac.

master:

with upnp       = no
with natpmp     = no

pr:

with upnp       = yes
with natpmp     = yes

Guix Hashes:
Here are my guix hashes, mine match @hebasto second round of hashes

find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

68494ee7382a1c0125c3f4c135f32ffb25789a6adc99b250eb8a02ad6302e6e3  guix-build-2445df4eb36b/output/aarch64-linux-gnu/SHA256SUMS.part
4cb8f30bc9eac086391f886969237ebdd47c84f5bc3370deebb9d397b4467a63  guix-build-2445df4eb36b/output/aarch64-linux-gnu/bitcoin-2445df4eb36b-aarch64-linux-gnu-debug.tar.gz
4d5995f680b504400f7851b9db412294aa9efadbb87a711abcc6fee31167e56f  guix-build-2445df4eb36b/output/aarch64-linux-gnu/bitcoin-2445df4eb36b-aarch64-linux-gnu.tar.gz
46d6337d02c53da14991afbe9b6b84121fcd06374f4144cc8cb85137cc9a8ce3  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/SHA256SUMS.part
90bb08c733ac0808e2224e3b5ea9b166cbed2850924582ff0063be09b86e6f82  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/bitcoin-2445df4eb36b-arm-linux-gnueabihf-debug.tar.gz
8aab610762d674c8db0de4e76712d5d79d0de3e24bb37810287313d3942b7fd0  guix-build-2445df4eb36b/output/arm-linux-gnueabihf/bitcoin-2445df4eb36b-arm-linux-gnueabihf.tar.gz
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  guix-build-2445df4eb36b/output/dist-archive/SKIPATTEST.TAG
894d082d230643381b63d31620fd4fe76c031de41c49edef9ef3de19740bb860  guix-build-2445df4eb36b/output/dist-archive/bitcoin-2445df4eb36b.tar.gz
9e5180ccee019607391373797a1b048f3a971124bb4ebc19395d9c92b2d798a1  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/SHA256SUMS.part
daa9c3ae79a157cba9ed059a0a63077dd5d9da432f4aec9874814235959435c9  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/bitcoin-2445df4eb36b-powerpc64-linux-gnu-debug.tar.gz
25fd2dffbbc717aee2cfad8f371fb90130b79271d301b772de6ededf22e8c1ee  guix-build-2445df4eb36b/output/powerpc64-linux-gnu/bitcoin-2445df4eb36b-powerpc64-linux-gnu.tar.gz
2d7fa025b249cffd98fd7860dd1ee0efd2659941b12b5f0165e5b50c71761c52  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/SHA256SUMS.part
a68763b780b942912410b986fade2f39de1fa5d1827a89aed5afba13b9774126  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/bitcoin-2445df4eb36b-powerpc64le-linux-gnu-debug.tar.gz
159cfdfd2915604aa94b5154e5a5fc2acf7ec800644ade880e076c2549ae241a  guix-build-2445df4eb36b/output/powerpc64le-linux-gnu/bitcoin-2445df4eb36b-powerpc64le-linux-gnu.tar.gz
c9be12a9afb44f18dbbee9dcad588c6261df6a6933007e8bdc51b471089b08ae  guix-build-2445df4eb36b/output/riscv64-linux-gnu/SHA256SUMS.part
6a17f1d98e6fa1a8254e1e75720b93c1c5d1c8bc96f256a67a3677146535a07f  guix-build-2445df4eb36b/output/riscv64-linux-gnu/bitcoin-2445df4eb36b-riscv64-linux-gnu-debug.tar.gz
149df6c74388d1fe0941d5070e70a07125399a6a67910cc85f7d03637853c6fe  guix-build-2445df4eb36b/output/riscv64-linux-gnu/bitcoin-2445df4eb36b-riscv64-linux-gnu.tar.gz
a0d7e056abdc2f11f1aac5ae60188992925cb07cfb61411baac3893d8c2a49f5  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/SHA256SUMS.part
4926af32d669f82a70f4e092bc6c8da1ea09ffe02c8a8ab3388627e61b787f17  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx-unsigned.dmg
16796d7c530d4d56af88cc8de1a4488ffc376482a129e46af12093887f2b76f4  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx-unsigned.tar.gz
9845d32d2b8a00e0cd7c51cc07e88aecb7cc48de92fc827eddf749ea8951de50  guix-build-2445df4eb36b/output/x86_64-apple-darwin18/bitcoin-2445df4eb36b-osx64.tar.gz
74eb5703182e303d2fea043202208f802134bd425113a0439225d689e70c2509  guix-build-2445df4eb36b/output/x86_64-linux-gnu/SHA256SUMS.part
a3eb9fabc0d4ea53fa41555a3a3d5380e47a7629aed20e7463d92cd005ff1e17  guix-build-2445df4eb36b/output/x86_64-linux-gnu/bitcoin-2445df4eb36b-x86_64-linux-gnu-debug.tar.gz
13c1cc6fb583476dc52e904d39f8a0de125b20c7016fea614349836756335cb5  guix-build-2445df4eb36b/output/x86_64-linux-gnu/bitcoin-2445df4eb36b-x86_64-linux-gnu.tar.gz
8502bc1a8129d1c2da750f88a5afd429778689f7b4c9a572df466985f317ebb2  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/SHA256SUMS.part
1ceb92ab15922a7ce40041cfd14d561ba0b8cfac278ce6df1f4b45f94f344792  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win-unsigned.tar.gz
4531669ea4ff2f448c785bf797ed62b15a69729f24d86cc8a72da888d304a687  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64-debug.zip
69de6d1aac5862bd5f3646f344efeacac104f97e8a66409cc21c208043d283eb  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64-setup-unsigned.exe
9a131b9adb3b3e25e937eb93afda333c86143328cea11e71ed4da10a79893794  guix-build-2445df4eb36b/output/x86_64-w64-mingw32/bitcoin-2445df4eb36b-win64.zip

dnl The preferred Homebrew prefix for Apple Silicon is /opt/homebrew.
dnl Therefore, as we do not use pkg-config to detect miniupnpc and libnatpmp
dnl packages, we should set the CPPFLAGS and LDFLAGS variables for them
dnl explicitly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

completely nitpicking here, but is there any reason to have this on its own line?

@laanwj
Copy link
Member

laanwj commented Sep 6, 2021

It's a bit unfortunate that package management on MacOS requires so much custom functionality, that there's no way to abstract things like with pkg-config. As for the aarch64-specific handling it feels like some of the issues worked around here are upstream problems, with regard to directory structure not architecture.

But if it's necessary it's necessary …

The simpler way is to use pkg-config, but "we do not use pkg-config to detect" miniupnpc and libnatpmp packages.

Why not? I guess the default "make install" for them doesn't install a pkgconfig .pc file and this is added by linux distros? Or another reason?
Edit: this was discussed on IRC in #bitcoin-core-builds, the reason is inconsistent shipping/upstream of the pkg-config files.

@laanwj laanwj merged commit 6718fbe into bitcoin:master Sep 6, 2021
@hebasto hebasto deleted the 210703-brew-arm branch September 6, 2021 15:59
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 7, 2021
…npc and libnatpmp

2445df4 build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp (Hennadii Stepanov)

Pull request description:

  On master (7a49fdc) the `configure` script does not pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple Silicon:

  ```
  % ./configure --with-miniupnpc
  ...
  checking for miniupnpc/miniupnpc.h... no
  checking for miniupnpc/upnpcommands.h... no
  checking for miniupnpc/upnperrors.h... no
  ...
  checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc."
  ```

  ```
  % ./configure --with-natpmp
  ...
  checking for natpmp.h... no
  ...
  checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp
  ```

  The preferred Homebrew [prefix for Apple Silicon](https://docs.brew.sh/Installation) is `/opt/homebrew`. Therefore, if we do not use `pkg-config` to detect packages, we should set the `CPPFLAGS` and `LDFLAGS` variables for them explicitly.

ACKs for top commit:
  Zero-1729:
    re-tACK 2445df4 (re-tested on an M1 Machine running macOS 11.4).
  jarolrod:
    re-ACK 2445df4

Tree-SHA512: d623d26492f463812bf66ca519847ff4b23d517466b6c51c3caf3642a582d02e5f03ce57915742b29f01bf9bceb731a3978ef9a5fdc82e568bcb62548eda758a
kwvg added a commit to kwvg/dash that referenced this pull request Feb 26, 2022
laanwj added a commit to bitcoin-core/gui that referenced this pull request Apr 28, 2022
…iupnpc and libnatpmp. Again :)

1659034 build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `libnatpmp` package (Hennadii Stepanov)
65cddf6 build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `miniupnpc` package (Hennadii Stepanov)
bbbcb96 build, refactor: Fix indentation (Hennadii Stepanov)

Pull request description:

  Apparently, bitcoin/bitcoin#24391 broke the [ability](bitcoin/bitcoin#22397) of the `configure` script to pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple M1.

  This PR fixes it.

ACKs for top commit:
  promag:
    Tested ACK 1659034
  jarolrod:
    tACK 1659034

Tree-SHA512: 93988f59f425890d60582b93d4ac5b2ad03011a5c6dbb44678a3ca591da7518c1c741bc1045b2c763bbe887947f32293b38d55fd7a96f09d2092ad34baa1db21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 29, 2022
…nd libnatpmp. Again :)

1659034 build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `libnatpmp` package (Hennadii Stepanov)
65cddf6 build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `miniupnpc` package (Hennadii Stepanov)
bbbcb96 build, refactor: Fix indentation (Hennadii Stepanov)

Pull request description:

  Apparently, bitcoin#24391 broke the [ability](bitcoin#22397) of the `configure` script to pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple M1.

  This PR fixes it.

ACKs for top commit:
  promag:
    Tested ACK 1659034
  jarolrod:
    tACK 1659034

Tree-SHA512: 93988f59f425890d60582b93d4ac5b2ad03011a5c6dbb44678a3ca591da7518c1c741bc1045b2c763bbe887947f32293b38d55fd7a96f09d2092ad34baa1db21
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants