Skip to content

Disable combining LLD with external llvm-config #139853

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 18, 2025

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Apr 15, 2025

When an external llvm-config is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using rust-lld nor copy it to the target sysroot.

Fixes: #139477

CC @cuviper

r? @onur-ozkan

try-job: dist-x86_64-linux
try-job: dist-aarch64-linux

@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 Apr 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2025

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

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

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

r=me with/without the nit once CI is green

@@ -2397,6 +2397,15 @@ impl Config {
);
}

let has_external_llvm_config = !config.llvm_from_ci
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We could move Builder::is_system_llvm to Config since it doesn't rely on any Builder specific data (other than is_builder_target function which can also be moved).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel very comfortable calling Config functions in Config::parse, because it's essentially its constructor, and some of the attributes used in the functions might not be initialized properly yet when the function is called. That being said, we access specific properties on the config variable anyway, so that's probably not so different. Moved the functions.

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Apr 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2025

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 15, 2025

@rustbot ready

@rust-log-analyzer

This comment has been minimized.

@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 15, 2025

📌 Commit 689b4c9 has been approved by onur-ozkan

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 15, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2025
Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC `@cuviper`

r? `@onur-ozkan`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#136926 (Stabilize `-Zdwarf-version` as `-Cdwarf-version`)
 - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139867 (Fix some tidy paper cuts)
 - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block)
 - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#139884 (Update books)
 - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2025
Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC ``@cuviper``

r? ``@onur-ozkan``
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options)
 - rust-lang#139823 (Fix some bootstrap papercuts)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139867 (Fix some tidy paper cuts)
 - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block)
 - rust-lang#139876 (Make CodeStats' type_sizes public)
 - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#139884 (Update books)
 - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks)
 - rust-lang#139893 (Add test for issue 125668)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar
Copy link
Contributor

I think this failed in rollup: #139895 (comment)

@Zalathar
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 16, 2025
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 16, 2025

Hmm, it failed in post-opt tests. This might be a bit annoying, because this is not external LLVM, but a "local" LLVM, and here we do in fact want to use LLD. I think that we should just print a warning instead of panicking when LLD is configured, and just avoid putting it into the sysroot in that case. @onur-ozkan WDYT?

@onur-ozkan
Copy link
Member

Hmm, it failed in post-opt tests. This might be a bit annoying, because this is not external LLVM, but a "local" LLVM, and here we do in fact want to use LLD. I think that we should just print a warning instead of panicking when LLD is configured, and just avoid putting it into the sysroot in that case. @onur-ozkan WDYT?

Sounds good to me.

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 16, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC `@cuviper`

r? `@onur-ozkan`

try-job: dist-x86_64-linux
try-job: dist-aarch64-linux
@bors
Copy link
Collaborator

bors commented Apr 16, 2025

⌛ Trying commit a870bba with merge b862ceb...

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

☀️ Try build successful - checks-actions
Build commit: b862ceb (b862ceb19c9f0acb5dbb6879d7d38231164fd424)

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 16, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 16, 2025
@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

📌 Commit a870bba has been approved by onur-ozkan

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 16, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 17, 2025
Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC `@cuviper`

r? `@onur-ozkan`

try-job: dist-x86_64-linux
try-job: dist-aarch64-linux
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139114 (Implement `pin!()` using `super let`)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 17, 2025
Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC `@cuviper`

r? `@onur-ozkan`

try-job: dist-x86_64-linux
try-job: dist-aarch64-linux
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138528 (deref patterns: implement implicit deref patterns)
 - rust-lang#139393 (rustdoc-json: Output target feature information)
 - rust-lang#139553 (sync::mpsc: prevent double free on `Drop`)
 - rust-lang#139615 (Remove `name_or_empty`)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139913 (rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions))
 - rust-lang#139942 (Ignore aix for tests/ui/erros/pic-linker.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ac00a37 into rust-lang:master Apr 18, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 18, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
Rollup merge of rust-lang#139853 - Kobzol:lld-llvm-config, r=onur-ozkan

Disable combining LLD with external llvm-config

When an external `llvm-config` is used, we don't really know anything about the external LLD, as we don't build it ourselves. Therefore, we probably shouldn't allow using `rust-lld` nor copy it to the target sysroot.

Fixes: rust-lang#139477

CC ``@cuviper``

r? ``@onur-ozkan``

try-job: dist-x86_64-linux
try-job: dist-aarch64-linux
@Kobzol Kobzol deleted the lld-llvm-config branch April 18, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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
Development

Successfully merging this pull request may close these issues.

FTBFS tarball rust 1.86.0: generated a symlink in a tarball: rust-lld
6 participants