Skip to content

Conversation

Noratrieb
Copy link
Member

This environment variable should always be set by bootstrap in rustc_llvm_env. The fallback is quite ugly and complicated, so removing it is nice.

cargo.env("LLVM_CONFIG", &llvm_config);

I tried finding when this was added in git history, but it pointed all the way to "add build scripts" at which point I stopped digging more. This has always been here.

cc @nikic @cuviper in case you happen to be aware of a deeper reason behind this

r? bootstrap

This environment variable should always be set by bootstrap in
`rustc_llvm_env`. The fallback is quite ugly and complicated, so
removing it is nice.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 31, 2024
@cuviper
Copy link
Member

cuviper commented Apr 1, 2024

This environment variable should always be set by bootstrap in rustc_llvm_env.

That function is not called in check mode, but this build scripts also quits early in that case.

I tried finding when this was added in git history, but it pointed all the way to "add build scripts" at which point I stopped digging more.

i.e. 4da4970, and I think this is what that was mimicking:

rust/mk/main.mk

Line 301 in 4da4970

LLVM_CONFIG_$(1):=$$(CFG_LLVM_INST_DIR_$(1))/bin/llvm-config$$(X_$(1))

... but the fallback probably hasn't been functional in a long time.

@clubby789
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 6, 2024

📌 Commit d651bae has been approved by clubby789

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 Apr 6, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#123294 (Require LLVM_CONFIG to be set in rustc_llvm/build.rs)
 - rust-lang#123467 (MSVC targets should use COFF as their archive format)
 - rust-lang#123498 (explaining `DefKind::Field`)
 - rust-lang#123519 (Improve cfg and check-cfg configuration)
 - rust-lang#123525 (CFI: Don't rewrite ty::Dynamic directly)
 - rust-lang#123526 (Do not ICE when calling incorrectly defined `transmute` intrinsic)
 - rust-lang#123528 (Hide async_gen_internals from standard library documentation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dea28d8 into rust-lang:master Apr 6, 2024
@rustbot rustbot added this to the 1.79.0 milestone Apr 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2024
Rollup merge of rust-lang#123294 - Nilstrieb:reuqire-llvm-config, r=clubby789

Require LLVM_CONFIG to be set in rustc_llvm/build.rs

This environment variable should always be set by bootstrap in `rustc_llvm_env`. The fallback is quite ugly and complicated, so removing it is nice.

https://github.com/rust-lang/rust/blob/bf71daedc29e7a240261acd1516378047e311a6f/src/bootstrap/src/core/build_steps/compile.rs#L1166

I tried finding when this was added in git history, but it pointed all the way to "add build scripts" at which point I stopped digging more. This has always been here.

cc `@nikic` `@cuviper` in case you happen to be aware of a deeper reason behind this

r? bootstrap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants