build: use -no_exported_symbols on macOS
#29072
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
Closed
b9ec822 to
5f1113a
Compare
hebasto
reviewed
Jan 2, 2024
5f1113a to
aeecba4
Compare
aeecba4 to
438851e
Compare
|
Based on #21778, so the flag is usable in release builds. |
438851e to
d2ea115
Compare
|
Rebased, but drafted while based on #21778. |
This was referenced May 23, 2024
d2ea115 to
6213ac9
Compare
|
Binary size comparison for
|
6213ac9 to
44b6c7b
Compare
|
@theuni do you have any thoughts here? |
44b6c7b to
ff83833
Compare
|
Does |
Oh whoops, already asked and answered. Nevermind! |
This reduces the size of the binary by 2-3% when building with `--enable-reduce-exports`.
ff83833 to
81d4dc8
Compare
|
Guix Build (aarch64): 45f5a3e38b5e2f8ef7f83f8b1e509d60cb933c52a0110dd6de6e43252869a62f guix-build-81d4dc8e8739/output/arm64-apple-darwin/SHA256SUMS.part
94e4578d894e61d6b96278a293e2c17fc19a8431bd77c996346fc7e126fc6b3e guix-build-81d4dc8e8739/output/arm64-apple-darwin/bitcoin-81d4dc8e8739-arm64-apple-darwin-unsigned.tar.gz
8f94554c9fdf356cbefd926944ece1ac39af26a80039d245e350b9194861af4a guix-build-81d4dc8e8739/output/arm64-apple-darwin/bitcoin-81d4dc8e8739-arm64-apple-darwin-unsigned.zip
434880815054e5e858fcc8c219b1932cf769fef0d1a30cae58b0ed99f06dddf6 guix-build-81d4dc8e8739/output/arm64-apple-darwin/bitcoin-81d4dc8e8739-arm64-apple-darwin.tar.gz
5318e5bc920ea58e11736fd5ae1df761d0ea07ace76d609413375c1bb677f5e1 guix-build-81d4dc8e8739/output/dist-archive/bitcoin-81d4dc8e8739.tar.gz
a9352792d12c202385b2c170aa4cad3837a9f59f427a00e17e98f6b5b7c902d4 guix-build-81d4dc8e8739/output/x86_64-apple-darwin/SHA256SUMS.part
d619422056dae796cca302498a227f7ed3b6db80c9068dc48f6e330f5b0c7da5 guix-build-81d4dc8e8739/output/x86_64-apple-darwin/bitcoin-81d4dc8e8739-x86_64-apple-darwin-unsigned.tar.gz
18af2ca7d2765d0b1fc925e37d2dadf3536d56dec381788e01fb3d908e1410a9 guix-build-81d4dc8e8739/output/x86_64-apple-darwin/bitcoin-81d4dc8e8739-x86_64-apple-darwin-unsigned.zip
5dde2d2f531a3d93789313e31c6f69ecefb80ba564a8f16854b0165d4abbda62 guix-build-81d4dc8e8739/output/x86_64-apple-darwin/bitcoin-81d4dc8e8739-x86_64-apple-darwin.tar.gz |
|
Guix build (aarch64): |
hebasto
reviewed
Jul 16, 2024
There was a problem hiding this comment.
Tested 81d4dc8 on macOS 14.5 Sonoma (Apple Silicon):
- the master branch @ 9c5cdf0:
% nm --extern-only src/bitcoind | wc -l
579
- this PR:
% nm --extern-only src/bitcoind | wc -l
537
I don't know the reasons, but bitcoind still exports
0000000100000000 T __mh_execute_header
The new -no_exported_symbols linker flag is not printed in the configure summary.
That is expected. See the PR description.
That's expected. We don't print the RE LD flags. |
theuni
approved these changes
Jul 16, 2024
|
Ported to the CMake-based build system in hebasto#271. |
hebasto
added a commit
to hebasto/bitcoin
that referenced
this pull request
Jul 20, 2024
4228b9b fixup! cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov) Pull request description: This PR ports bitcoin#29072. ACKs for top commit: fanquake: ACK 4228b9b Tree-SHA512: 86c3f4db941f2e5c7625c98192d1f23fda68a82feb6e40334055376e2cd44aa8c7bc9904cfe326835211270a95aab3525f90a052343f12cec67ef877a601e854
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Nov 15, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Nov 16, 2024
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Nov 17, 2024
PastaPastaPasta
added a commit
to dashpay/dash
that referenced
this pull request
Nov 17, 2024
, bitcoin#28622, bitcoin#28880, bitcoin#29185, bitcoin#29170, bitcoin#29233, bitcoin#29298, bitcoin#29598, bitcoin#29732, bitcoin#29890, bitcoin#29739, bitcoin#30074, bitcoin#30198, bitcoin#29072 (toolchain backports: part 2) 1506d9d merge bitcoin#29072: use `-no_exported_symbols` on macOS (Kittywhiskers Van Gogh) 9247960 merge bitcoin#30198: qt 5.15.14 and fix macOS build with Clang 18 (Kittywhiskers Van Gogh) 5585e7a merge bitcoin#30074: use ENV flags in get_arch (Kittywhiskers Van Gogh) decd420 merge bitcoin#29739: swap cctools otool for llvm-objdump (Kittywhiskers Van Gogh) 0f8c420 merge bitcoin#29890: remove some tools when cross-compiling for macOS (Kittywhiskers Van Gogh) 936da1a merge bitcoin#29732: qt 5.15.13 (Kittywhiskers Van Gogh) c294b47 revert: patch qt to make placeholders differ from actual text (Kittywhiskers Van Gogh) af7090c merge bitcoin#29598: don't use -h with touch on OpenBSD (Kittywhiskers Van Gogh) ebf8ff2 merge bitcoin#29298: patch libtool out of libnatpmp/miniupnpc (Kittywhiskers Van Gogh) 070b876 merge bitcoin#29233: depends move macOS C(XX) FLAGS out of C & CXX (Kittywhiskers Van Gogh) d838481 revert dash#2398: Force fvisibility=hidden when compiling on macos (Kittywhiskers Van Gogh) 59a18f9 merge bitcoin#29170: add macho branch protection check (Kittywhiskers Van Gogh) cb024d9 merge bitcoin#29185: remove `--enable-lto` (Kittywhiskers Van Gogh) 6d75a81 merge bitcoin#28880: switch to using LLVM 17.x for macOS builds (Kittywhiskers Van Gogh) 7b0a1f2 merge bitcoin#28622: use macOS 14 SDK (Xcode 15.0) (Kittywhiskers Van Gogh) 02eb735 merge bitcoin#24948: fix typo in permissions (Kittywhiskers Van Gogh) 2739107 merge bitcoin#24534: make gen-sdk deterministic (Kittywhiskers Van Gogh) ab10bf9 merge bitcoin#24241: cleanup doc on need of Developer Account to obtain macOS SDK (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6384 * Dependency for #6389 * The Qt patch introduced in [dash#5596](#5596), `fix_qt_placeholders.patch`, was a portion of a suggested workaround for QTBUG-92199 ([source](https://bugreports.qt.io/browse/QTBUG-92199?focusedId=669719&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-669719)) but since then, a fix ([here](https://codereview.qt-project.org/c/qt/qtbase/+/434310)) has made its way to 5.15.12 and we are upgrading to 5.15.14 from 5.15.11. So we can safely remove this patch. ## Breaking Changes None expected ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 1506d9d PastaPastaPasta: utACK 1506d9d Tree-SHA512: df8e4ea0ce9e7b269d248518698f0566b5eca1a54cdfb53f5b213b90fb5177e5a5df44eaeb6f3fc014cd93351c9245736bb2fd52bc2af4ae274d8fa93e601b07
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces the size of the binary by ~1% when building with
--enable-reduce-exports.Can be tested with
dyld_info -exports src/bitcoind. The only exported symbol should be__mh_execute_header.