Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only copy, rename and link llvm-objcopy if llvm tools are enabled #132720

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Only copy, rename and link llvm-objcopy if llvm tools are enabled #132720

merged 1 commit into from
Nov 11, 2024

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Nov 7, 2024

Fixes #132719.

cc @bjorn3 who reported the bootstrapping problem for cg_clif.
cc @davidtwco in case this might be problematic for linux -> macOS cross-compile builds, but seems very unlikely.
cc @albertlarsan68 (co-reviewed #131405)

r? bootstrap

@jieyouxu @bjorn3
Only copy, rename and link llvm-objcopy if llvm tools are enabled
cf9cec3 
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 7, 2024
This was referenced Nov 7, 2024
Closed
Merged
@durin42
Copy link
Contributor

durin42 commented Nov 7, 2024

@rustbot label llvm-main

This fixes our LLVM HEAD builder so we need the label to get back to green while we wait for this to land.

@rustbot rustbot added the llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) label Nov 7, 2024
@durin42
Copy link
Contributor

durin42 commented Nov 7, 2024

This LG to me, but I'll let someone that knows bootstrap better take a look

icmccorm added a commit to BorrowSanitizer/rust that referenced this pull request Nov 9, 2024
@icmccorm
Provided default config and applied fix from rust-lang#132720
75a9f01
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 11, 2024

📌 Commit cf9cec3 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors 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 Nov 11, 2024
@jieyouxu jieyouxu mentioned this pull request Nov 11, 2024
Closed
@ehuss
Copy link
Contributor

ehuss commented Nov 11, 2024

@bors p=5

Fixes regression for local development.

@bors
Copy link
Contributor

bors commented Nov 11, 2024

⌛ Testing commit cf9cec3 with merge de27914...

@bors
Copy link
Contributor

bors commented Nov 11, 2024

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing de27914 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 11, 2024
@bors bors merged commit de27914 into rust-lang:master Nov 11, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 11, 2024
@jieyouxu jieyouxu deleted the do-not-strip branch November 11, 2024 19:50
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (de27914): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.5%, secondary -2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -2.5% [-3.0%, -2.0%] 2
Improvements ✅
(secondary)		 -2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) -2.5% [-3.0%, -2.0%] 2

Cycles

Results (secondary -2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 785.736s -> 786.551s (0.10%)
Artifact size: 335.37 MiB -> 335.37 MiB (-0.00%)

mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@bors @mati865
Auto merge of rust-lang#132720 - jieyouxu:do-not-strip, r=davidtwco
f218374 
Only copy, rename and link `llvm-objcopy` if llvm tools are enabled

Fixes rust-lang#132719.

cc `@bjorn3` who reported the bootstrapping problem for cg_clif.
cc `@davidtwco` in case this might be problematic for linux -> macOS cross-compile builds, but seems very unlikely.
cc `@albertlarsan68` (co-reviewed rust-lang#131405)

r? bootstrap
@hoodmane hoodmane mentioned this pull request Nov 12, 2024
Merged
@wesleywiser wesleywiser mentioned this pull request Nov 19, 2024
Closed
icmccorm added a commit to BorrowSanitizer/rust that referenced this pull request Nov 22, 2024
@icmccorm
Provided default config and applied fix from rust-lang#132720
8be8837
@cuviper cuviper mentioned this pull request Dec 13, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
@matthiaskrgr
Rollup merge of rust-lang#134240 - cuviper:dist-llvm-tools, r=jieyouxu
cb7115a 
Only dist `llvm-objcopy` if llvm tools are enabled

This uses the same condition that rust-lang#132720 added in the compilation phase.

r? `@jieyouxu`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#134240 - cuviper:dist-llvm-tools, r=jieyouxu
a53a3cc 
Only dist `llvm-objcopy` if llvm tools are enabled

This uses the same condition that rust-lang#132720 added in the compilation phase.

r? ``@jieyouxu``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#134240 - cuviper:dist-llvm-tools, r=jieyouxu
1a8c4be 
Only dist `llvm-objcopy` if llvm tools are enabled

This uses the same condition that rust-lang#132720 added in the compilation phase.

r? ```@jieyouxu```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
@rust-timer
Unrolled build for rust-lang#134240
a28b229 
Rollup merge of rust-lang#134240 - cuviper:dist-llvm-tools, r=jieyouxu

Only dist `llvm-objcopy` if llvm tools are enabled

This uses the same condition that rust-lang#132720 added in the compilation phase.

r? ``@jieyouxu``
@jieyouxu jieyouxu mentioned this pull request Jan 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@albertlarsan68 albertlarsan68

Labels
llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.

Shipping llvm-strip w/ a symlink breaks when bootstrap compiler doesn't contain any LLVM tools
8 participants
@jieyouxu @durin42 @davidtwco @bors @ehuss @rust-timer @rustbot @albertlarsan68