If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
No conflicts as of last run.
maflcko
commented at 1:24 PM on February 19, 2024:
member
Missing build: prefix in title?
l0rinc renamed this: Replace Custom `MAC_OSX` Macro with Standard `__APPLE__` for Consistent macOS Detection build: Replace Custom `MAC_OSX` Macro with Standard `__APPLE__` for Consistent macOS Detection on Feb 19, 2024
DrahtBot added the label Build system on Feb 19, 2024
maflcko added the label DrahtBot Guix build requested on Feb 19, 2024
DrahtBot
commented at 6:55 AM on February 20, 2024:
contributor
DrahtBot removed the label DrahtBot Guix build requested on Feb 20, 2024
l0rinc renamed this: build: Replace Custom `MAC_OSX` Macro with Standard `__APPLE__` for Consistent macOS Detection build: replace Custom `MAC_OSX` Macro with Standard `__APPLE__` for Consistent macOS Detection on Feb 20, 2024
l0rinc renamed this: build: replace Custom `MAC_OSX` Macro with Standard `__APPLE__` for Consistent macOS Detection build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection on Feb 20, 2024
fanquake
commented at 3:27 PM on February 20, 2024:
member
This is MAC_OSX because that is what we actually mean, and using our own define gives us more control. We produce release binaries for macOS running on darwin hardware, not just any "__APPLE__ target", which, for example, would include iOS running on darwin hardware (i.e arm64e-apple-ios).
l0rinc
commented at 3:58 PM on February 20, 2024:
contributor
Hey @fanquake, thanks for your input.
How do you suggest we unify this in the codebase, since it's not used consistently currently?
This change could level the field before branching off to properly differentiate between the cases.
Empact
commented at 6:52 AM on February 27, 2024:
contributor
Fyi the current uses of __APPLE__ in the codebase are from external libs included directly into the codebase as subtrees, see get_subtrees for a list of the related dirs. tinyformat.h is a special case which is included as a single file. @paplorinc
Which is not to say this change is incorrect (I'm not sure the distinction is meaningful in our case), but that the apparent inconsistency could be misleading.
l0rinc
commented at 10:19 AM on February 27, 2024:
contributor
Thanks @Empact, I think we can ignore the internal consistency argument, and the change would still make the build more maintainable - do you agree?
Empact
commented at 6:01 PM on February 27, 2024:
contributor
I'm +0 on this change but not particularly familiar with the scope and variety of our Apple-related builds.
DrahtBot added the label Needs rebase on Mar 1, 2024
l0rinc force-pushed on Mar 9, 2024
DrahtBot removed the label Needs rebase on Mar 9, 2024
theuni
commented at 5:43 PM on April 5, 2024:
member
I think this is the right thing to do. I just checked locally and this applies to iOS as well:
clang -target arm64-apple-ios test.cpp --sysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS16.4.sdk -o testing
Where test.cpp is:
#include <sys/types.h>
#include <sys/sysctl.h>
int main()
{
int val = 0;
int have_arm_shani;
size_t len = sizeof(val);
if (sysctlbyname("hw.optional.arm.FEAT_SHA256", &val, &len, nullptr, 0) == 0) {
have_arm_shani = val != 0;
}
return have_arm_shani;
}
And it compiles fine.
So in this case, we didn't actually mean macOS, we meant apple.
theuni
commented at 5:44 PM on April 5, 2024:
member
Whoops, sorry. I spoke too fast. This is the right thing to do for sha256.cpp. I'd have to look into the rest in detail, but I'd ACK that small change.
theuni
commented at 8:12 AM on April 9, 2024:
member
Would you be interested in opening a new PR for just the crypto changes? Otherwise I will.
l0rinc
commented at 9:25 AM on April 9, 2024:
contributor
@theuni, I've split it out the crypto package to #29834, thanks for checking it!
fanquake referenced this in commit 0b4218e43c on Apr 9, 2024
l0rinc force-pushed on Apr 9, 2024
l0rinc
commented at 3:12 PM on April 9, 2024:
contributor
Rebased after the #29834 merge, please let me know what to change or verify here.
DrahtBot added the label Needs rebase on Apr 25, 2024
l0rinc force-pushed on Apr 26, 2024
DrahtBot removed the label Needs rebase on Apr 26, 2024
l0rinc force-pushed on May 11, 2024
l0rinc force-pushed on May 29, 2024
hebasto added the label Needs CMake port on Aug 16, 2024
l0rinc force-pushed on Aug 30, 2024
l0rinc renamed this: build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection build: replace custom `MAC_OSX` macro with existing `__APPLE__` on Aug 30, 2024
l0rinc
commented at 10:37 AM on August 30, 2024:
contributor
Rebased and adjusted to the CMake build following @hebasto's Needs CMake port label.
maflcko removed the label Needs CMake port on Aug 30, 2024
maflcko added the label DrahtBot Guix build requested on Aug 30, 2024
DrahtBot
commented at 11:36 AM on August 31, 2024:
contributor
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]
build: Replace MAC_OSX macro with existing __APPLE__
Adopting `__APPLE__` aligns our project with broader industry practices, including those in prominent projects such as the Linux kernel (and even our own code).
See: https://sourceforge.net/p/predef/wiki/OperatingSystems/#macos
6c6b2442ed
l0rinc force-pushed on Oct 24, 2024
l0rinc
commented at 10:33 AM on October 24, 2024:
contributor
fanquake
commented at 12:30 PM on October 24, 2024:
member
ACK6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to __APPLE__ seems ok for now.
fanquake merged this on Oct 24, 2024
fanquake closed this on Oct 24, 2024
maflcko removed the label DrahtBot Guix build requested on Oct 24, 2024
TheCharlatan
commented at 2:59 PM on October 24, 2024:
contributor
Post-merge and Guix build reproduced ACK6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e
l0rinc deleted the branch on Oct 24, 2024
DrahtBot
commented at 7:53 PM on October 24, 2024:
contributor
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]
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: 2026-05-01 15:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
