This hasn’t been a problem for current native depends packages because are passing their own --sysroot values, and hasn’t been a problem for current host packages because they use darwin_ commands instead of build_darwin_ commands. But the current build_darwin_CC and build_darwin_CXX commands are still unnecessarily fragile, and incompatible with new native depends packages added in #18677.
Cory Fields (theuni) suggested in #16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But Michael (fanquake) pointed out in #18677 (review) that it would be inconsistent to switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option.
depends: Add --sysroot option to mac os native compile flags
Catalina SDK clang stopped automatically searching the SDK include paths when
invoked without --sysroot:
https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-594600985
https://github.com/Homebrew/homebrew-core/issues/45061
This hasn't been a problem for current native depends packages because are
passing their own --sysroot values, and hasn't been a problem for current host
packages because they use `darwin_` commands instead of `build_darwin_`
commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands
are still unnecessarily fragile, and incompatible with new native depends
packages added in https://github.com/bitcoin/bitcoin/pull/18677.
Cory Fields <cory-nospam-@coryfields.com> suggested in
https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-595393546 switching
compiler from SDK clang to native clang (from $PATH) to avoid this problem.
This is easy and makes a certain amount of sense for building native packages,
as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in
https://github.com/bitcoin/bitcoin/pull/18677#discussion_r409934309 that it
would be inconsistent use switch to non-SDK compilers while still using other
SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to
be just adding the missing --sysroot option.
1e94a2bcbc
fanquake added the label
Build system
on Apr 22, 2020
MarcoFalke added the label
Needs Guix build
on Apr 22, 2020
MarcoFalke removed the label
Needs Guix build
on Apr 22, 2020
MarcoFalke added the label
Needs gitian build
on Apr 22, 2020
DrahtBot
commented at 4:45 am on April 23, 2020:
member
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
No conflicts as of last run.
fanquake
commented at 1:13 pm on April 25, 2020:
member
Thanks for splitting this out. Will review.
DrahtBot
commented at 10:20 pm on April 26, 2020:
member
DrahtBot removed the label
Needs gitian build
on Apr 26, 2020
fanquake
commented at 11:48 am on April 27, 2020:
member
ACK1e94a2bcbc5ff8ae61eed9f31317ea534649116d - I think this change is ok, and I prefer it to the previous patch. Thanks for the summary in the PR description. I played around with Xcode and the CLT; I think previously I didn’t fully grok the slight differences between the two.
This output was produced from a test.cpp containing just #include <stdlib.h>.
When you pass -isysroot, to CLT Clang, /usr/local/include and /Library/Frameworks are dropped from the include search paths, and /usr/include is added. Note that -target-sdk-version=SDK_VERSION is also added to the compile flags.
CLT Clang -isysroot vs Xcode Clang:
Invoking CLT Clang with -isysroot is then seemingly equivalent to Xcode Clang, except for Xcode Clang additionally searching /usr/local/include.
dongcarl
commented at 12:07 pm on April 27, 2020:
member
Is /Library/Developer/CommandLineTools a symlink to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain or vice versa?
fanquake
commented at 12:33 pm on April 27, 2020:
member
Is /Library/Developer/CommandLineTools a symlink to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain or vice versa?
No, they are standalone, and there are decent differences between the content of either. Xcode has a bunch more Swift modules, Apple simulator components and dylibs bundled with it. For example, you can see a diff of /Library/Developer/CommandLineTools/usr and /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr here: https://gist.github.com/fanquake/22267d783ac76fbe3eba110d14433e7c.
The macOS SDKs inside either seem to be identical. i.e: diffoscope /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk --exclude-directory-metadata
ryanofsky
commented at 2:36 pm on April 27, 2020:
member
ACK1e94a2b - I think this change is ok, and I prefer it to the previous patch.
The current change 1e94a2bcbc5ff8ae61eed9f31317ea534649116d is the original fix I made for the issue before Cory’s suggestion in #16367 (comment).
It seems safe and good. But Cory’s suggestion to just use clang and presumably ld, nm, etc directly instead of going through an SDK for native packages (which are internal build & code generation tools not distributed or used at runtime) seems it should be a nice simplification and cleanup and more durable fix. I could be wrong but I’d assume a more typical way of building non-gui unix tools on mac is:
My reason for going back to the direct bugfix 1e94a2bcbc5ff8ae61eed9f31317ea534649116d instead of my half-implementation fba09de775f1eff68d2f72650bccc418cbe87cf8 of Cory suggestion is just that I know the direct fix works and I don’t have a great way of testing other alternatives. But probably an extension of fba09de775f1eff68d2f72650bccc418cbe87cf8 would be a better way to go in the long run.
fanquake
commented at 1:26 pm on May 2, 2020:
member
@dongcarl did you have something specific you wanted to follow up with here in regards to the symlink query?
dongcarl
commented at 5:50 pm on May 4, 2020:
member
@fanquake No not really. My initial impression here is that we should remove all the xcrun invocations if we can, as doing that makes explicit we’re expecting a standard toolchain for building (open source) applications from source. Will need to test that it works though.
fanquake
commented at 12:37 pm on May 7, 2020:
member
@dongcarl Ok, lets follow up with that then. Want to open an issue to track? I’m going to merge this now to unblock Russ and #18677.
fanquake merged this
on May 7, 2020
fanquake closed this
on May 7, 2020
sidhujag referenced this in commit
d51641ba68
on May 12, 2020
jasonbcox referenced this in commit
ce5bf0a880
on Sep 23, 2020
random-zebra referenced this in commit
64da8d9167
on Jul 2, 2021
kittywhiskers referenced this in commit
059728233a
on Jul 15, 2021
kittywhiskers referenced this in commit
018a788ef6
on Jul 15, 2021
UdjinM6 referenced this in commit
66d80b6a30
on Jul 19, 2021
UdjinM6 referenced this in commit
1f6adef97c
on Jul 19, 2021
kittywhiskers referenced this in commit
10adf0fe6a
on Jul 20, 2021
kittywhiskers referenced this in commit
be5837ec51
on Jul 20, 2021
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-19 00:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
