Skip to content

Default-enable llvm_tools_enabled when no config.toml is present - #133207

Merged
bors merged 3 commits into
rust-lang:masterfrom
jieyouxu:macos-objcopy
Nov 20, 2024
Merged

Default-enable llvm_tools_enabled when no config.toml is present#133207
bors merged 3 commits into
rust-lang:masterfrom
jieyouxu:macos-objcopy

Conversation

@jieyouxu

@jieyouxujieyouxu commented Nov 19, 2024

Copy link
Copy Markdown
Member

Fixes#133195. cc @wesleywiser could you double check if with this patch and no config.toml that you can run ./x test tests/ui --stage 1?

llvm-objcopy is usually required by cg_ssa on macOS to workaround bad strips.

cc @bjorn3 I hope this doesn't break cg_clif...

r? bootstrap

@rustbotrustbot 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 19, 2024
@rustbot

rustbot commented Nov 19, 2024

Copy link
Copy Markdown
Collaborator

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@bjorn3

bjorn3 commented Nov 19, 2024

Copy link
Copy Markdown
Member

I hope this doesn't break cg_clif...

Does this disable the option?

rust/compiler/rustc_codegen_cranelift/scripts/setup_rust_fork.sh

Lines 32 to 33 in e6c1e14

rustc = "$(pwd)/../dist/bin/rustc-clif"
cargo = "$(rustup which cargo)"
If not, it either should, or the config file there will need to be changed to explicitly disable it.

Kobzol
Kobzol reviewed Nov 19, 2024
View reviewed changes
Comment threadsrc/bootstrap/src/core/config/config.rs
Comment threadsrc/bootstrap/src/utils/change_tracker.rs Outdated
@rustbot

rustbot commented Nov 19, 2024

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@jieyouxu

jieyouxu commented Nov 19, 2024

Copy link
Copy Markdown
MemberAuthor

If not, it either should, or the config file there will need to be changed to explicitly disable it.

I modified the config there to explicitly disable llvm tools.

@jieyouxu

jieyouxu commented Nov 19, 2024

Copy link
Copy Markdown
MemberAuthor

Changes since last review (diff):

  • Fixed change log entry
  • Adjusted cg_clif's config.toml to disable llvm tools

bjorn3
bjorn3 reviewed Nov 19, 2024
View reviewed changes
Comment threadcompiler/rustc_codegen_cranelift/scripts/setup_rust_fork.sh Outdated
@Kobzol

Kobzol commented Nov 19, 2024

Copy link
Copy Markdown
Member

LGTM, if bjorn3 is ok with the latest change.

@jieyouxu

jieyouxu commented Nov 19, 2024

Copy link
Copy Markdown
MemberAuthor

@bors r=Kobzol,bjorn3 rollup

@bors

bors commented Nov 19, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit 4b5c883 has been approved by Kobzol,bjorn3

It is now in the queue for this repository.

@borsbors 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 19, 2024
wesleywiser
wesleywiser approved these changes Nov 19, 2024
View reviewed changes

@wesleywiserwesleywiser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm this fixes building without config.toml for me. Thanks @jieyouxu! 💛

@matthiaskrgrmatthiaskrgr mentioned this pull request Nov 19, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2024
@bors
Auto merge ofrust-lang#133219- matthiaskrgr:rollup-hnuq0zf, r=matth…
875df37
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#123947 (Add vec_deque::Iter::as_slices and friends)
- rust-lang#125405 (Add std::thread::add_spawn_hook.)
- rust-lang#133175 (ci: use free runner in dist-i686-msvc)
- rust-lang#133183 (Mention std::fs::remove_dir_all in std::fs::remove_dir)
- rust-lang#133188 (Add `visit` methods to ast nodes that already have `walk`s on ast visitors)
- rust-lang#133201 (Remove `TokenKind::InvalidPrefix`)
- rust-lang#133207 (Default-enable `llvm_tools_enabled` when no `config.toml` is present)
- rust-lang#133213 (Correct the tier listing of `wasm32-wasip2`)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 49aec06 into rust-lang:masterNov 20, 2024
@rustbotrustbot added this to the 1.84.0 milestone Nov 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 20, 2024
@rust-timer
Unrolled build forrust-lang#133207
166cf0a
Rollup merge of rust-lang#133207 - jieyouxu:macos-objcopy, r=Kobzol,bjorn3
Default-enable `llvm_tools_enabled` when no `config.toml` is present
Fixesrust-lang#133195. cc `@wesleywiser` could you double check if with this patch and no `config.toml` that you can run `./x test tests/ui --stage 1`?
`llvm-objcopy` is usually required by cg_ssa on macOS to workaround bad `strip`s.
cc `@bjorn3` I hope this doesn't break cg_clif...
r? bootstrap
@jieyouxu
jieyouxu deleted the macos-objcopy branch November 20, 2024 06:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KobzolKobzolKobzol left review comments
@wesleywiserwesleywiserwesleywiser approved these changes
@bjorn3bjorn3bjorn3 approved these changes

Assignees

@KobzolKobzol

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-bootstrapRelevant 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.

Bootstrap without creating a config.toml fails with missing rust-objcopy

6 participants

@jieyouxu@rustbot@bjorn3@Kobzol@bors@wesleywiser