Fix up setting strip = true in Cargo.toml makes build scripts fail in… by monkeydbobo · Pull Request #130781 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits1 Checks6 Files changed
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
[ Show hidden characters]({{ revealButtonHref }})
Fix issue: #110536
Strip binary is PATH dependent which breaks builds in MacOS.
For example, on my Mac, the output of 'which strip' is '/opt/homebrew/opt/binutils/bin/strip', which leads to incorrect 'strip' results. Therefore, just like on other systems, it is also necessary to specify 'stripcmd' on macOS. However, it seems that there is a bug in binutils bugzilla-Bug 31571, which leads to the problem mentioned above.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) some time within the next two weeks.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
@rustbot author: the review is finished, PR author should check the comments and take action accordingly@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
📌 Commit 802bf71 has been approved by davidtwco
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 6 pull requests
Successful merges:
- rust-lang#130735 (Simple validation for unsize coercion in MIR validation)
- rust-lang#130781 (Fix up setting strip = true in Cargo.toml makes build scripts fail in…)
- rust-lang#130811 (add link from random() helper fn to extensive DefaultRandomSource docs)
- rust-lang#130819 (Add
must_useattribute tolen_utf8andlen_utf16.) - rust-lang#130832 (fix some cfg logic around optimize_for_size and 16-bit targets)
- rust-lang#130842 (Add tracking issue for io_error_inprogress)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#130781 - monkeydbobo:mdb/fix_up_cross_compile_osx, r=davidtwco
Fix up setting strip = true in Cargo.toml makes build scripts fail in…
Fix issue: rust-lang#110536 Strip binary is PATH dependent which breaks builds in MacOS. For example, on my Mac, the output of 'which strip' is '/opt/homebrew/opt/binutils/bin/strip', which leads to incorrect 'strip' results. Therefore, just like on other systems, it is also necessary to specify 'stripcmd' on macOS. However, it seems that there is a bug in binutils bugzilla-Bug 31571, which leads to the problem mentioned above.
rvolosatovs added a commit to rvolosatovs/nixify that referenced this pull request
rvolosatovs added a commit to rvolosatovs/nixify that referenced this pull request
rvolosatovs added a commit to rvolosatovs/nixify that referenced this pull request
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…=jieyouxu,albertlarsan68
bootstrap/codegen_ssa: ship llvm-strip and use it for -Cstrip
Fixes rust-lang#131206.
- Includes
llvm-strip(a symlink tollvm-objcopy) in the compiler dist artifact so that it can be used for-Cstripinstead of the system tooling. - Uses
llvm-stripinstead of/usr/bin/stripfor macOS. macOS needs a specific linker and the system one is preferred, hence rust-lang#130781 but that doesn't work when cross-compiling, so use thellvm-striputility instead.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#131405 - davidtwco:hardcoded-strip-macos, r=jieyouxu,albertlarsan68
bootstrap/codegen_ssa: ship llvm-strip and use it for -Cstrip
Fixes rust-lang#131206.
- Includes
llvm-strip(a symlink tollvm-objcopy) in the compiler dist artifact so that it can be used for-Cstripinstead of the system tooling. - Uses
llvm-stripinstead of/usr/bin/stripfor macOS. macOS needs a specific linker and the system one is preferred, hence rust-lang#130781 but that doesn't work when cross-compiling, so use thellvm-striputility instead.
bors added a commit to rust-lang-ci/rust that referenced this pull request
[WIP] Use /usr/bin/strip on macOS -> iOS cross as a temporary mitigation
Mitigation for <rust-lang#138212>. This reintroduces the "hard-coded strip" workaround (but only for iOS) that was initially introduced in rust-lang#130781.
Looks like LLVM 20's initial llvm-objcopy version that we ship as rust-objcopy may be producing stripped iOS binaries with invalid offsets that fail iOS platform consistency checks.
For the time being, use macOS system strip at /usr/bin/strip which should be available (unlike Linux -> macOS cross where this is not guaranteed to be available, partially why we are shipping llvm-objcopy in the first place^linux-darwin-cross) that shouldn't have this offset problem.
Review and testing advice
I don't have access to either macOS or iOS platforms, and I can't really write a test for this. This will need to be tested manually.
r? @davidtwco (or reroll)
cc *-apple-ios target maintainers @badboy @deg4uss3r @madsmtm
try-job: dist-apple-various try-job: dist-x86_64-apple
bors added a commit to rust-lang-ci/rust that referenced this pull request
Use /usr/bin/strip on macOS -> iOS cross as a temporary workaround
Crude workaround for <rust-lang#138212> to unblock nightly macOS-to-iOS cross builds. This reintroduces the "hard-coded strip" workaround (but only for iOS) that was initially introduced in rust-lang#130781.
Looks like LLVM 20's initial llvm-objcopy version that we ship as rust-objcopy may be producing stripped iOS binaries with invalid offsets that fail iOS platform consistency checks.
For the time being, use macOS system strip when cross-compiling from macOS -> iOS at /usr/bin/strip which should be available (unlike Linux -> macOS cross where this is not guaranteed to be available, partially why we are shipping llvm-objcopy in the first place^linux-darwin-cross) that shouldn't have this offset problem.
Alternatives
- We could also try to use a
stripfrom PATH. However, that can regress iOS users if they have broken homebrew binutils strip in their PATH that take precedence over a "good" strip. - We could also wait for upstream
llvm-objcopyfixes.
Review and testing advice
I don't have access to either macOS or iOS platforms, and I can't really write a test for this. This will need to be tested manually.
r? @davidtwco (or reroll)
cc *-apple-ios target maintainers @badboy @deg4uss3r @madsmtm
try-job: dist-apple-various try-job: dist-x86_64-apple try-job: dist-aarch64-apple
Labels
Operating system: macOS
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.