build: Quiet warnings in symlinked headers installed from homebrew #26070
Conversation
|
c-i was happy with this so I'm going to drop the test-commit. |
theuni
force-pushed
the
fix-brew-usrlocal
branch
from
56b768c to
b50a4b7
Compare
maflcko
changed the title
|
Concept ACK - picked into #26056. |
|
Concept ACK. |
| dnl It's safe to add these paths even if the functionality is disabled by | ||
| dnl the user (--without-wallet or --without-gui for example). | ||
|
|
||
| dnl Homebrew may create symlinks in /usr/local/include for some packages. |
There was a problem hiding this comment.
The /usr/local path is relevant for macOS Intel systems only. macOS M1 uses the /opt/homebrew one.
Actually, the latter systems do not require this fix. But a comment update seems enough.
There was a problem hiding this comment.
I believe this would almost be safe enough to do everywhere (not just mac/homebrew), and possibly even for /usr/include because it should have no unintended side-effects, and for gcc it will just fail the test.
I'm not suggesting that we do that, just pointing out that it should be pretty much harmless even if it's not needed.
There was a problem hiding this comment.
I think the wording here is ok; we don't need to call out differences in defaults between x86_64 and arm64 installations of homebrew. Also, just because /opt/homebrew is the preferred prefix for arm64, it's not clear to me that a brew installation couldn't be in /usr/local on arm64, which this would also accomodate.
…lled from homebrew b50a4b7 build: quiet warnings in system headers installed from homebrew (Cory Fields) Pull request description: From the included comment: Homebrew may create symlinks in `/usr/local/include` for some packages. Because MacOS's clang internally adds `-I /usr/local/include` to its search paths, this will negate efforts to use `-isystem` for those packages, as they will be found first in `/usr/local`. Use the internal `-internal-isystem` option to system-ify all `/usr/local/include` paths without adding it to the list of search paths in case it's not already there. This fixes the issue explained here: bitcoin#26056 (comment) ~Also temporarily includes bitcoin#26056 as a test. I will remove that commit if/when c-i is happy, and fanquake can rebase it post-merge.~ I've removed this commit now that c-i succeeded with it. ACKs for top commit: hebasto: ACK b50a4b7, tested as a part of bitcoin#26056 on macOS Monterey 12.6 (21G115, both Intel and Apple M1) + Apple clang 14.0.0: Tree-SHA512: 163aa359d27c31d52b444252762e32dd8a11acc043cf1a2aa953f902d1dab77ece52e2dfedcce637e6a1dda47e4c566bfeb8d3b092f82bfc73923843b7bc619c
4 participants
From the included comment:
Homebrew may create symlinks in
/usr/local/includefor some packages. Because MacOS's clang internally adds-I /usr/local/includeto its search paths, this will negate efforts to use-isystemfor those packages, as they will be found first in/usr/local. Use the internal-internal-isystemoption to system-ify all/usr/local/includepaths without adding it to the list of search paths in case it's not already there.This fixes the issue explained here: #26056 (comment)
Also temporarily includes #26056 as a test. I will remove that commit if/when c-i is happy, and fanquake can rebase it post-merge.I've removed this commit now that c-i succeeded with it.