Skip to content

Conversation

@petrochenkov
Copy link
Contributor

rust-lld is supposed to live inside sysroot, so it doesn't change the behavior of the function, only removes a potential micro-optimization.

#149178 (comment)
r? @mati865

…mingw`

`rust-lld` is supposed to live in sysroot, so it doesn't change the behavior of the function, only removes a potential micro-optimization.
@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 Dec 3, 2025
@mati865
Copy link
Member

mati865 commented Dec 3, 2025

Shouldn't hurt.

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 3, 2025

📌 Commit 388c42e has been approved by mati865

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 Dec 3, 2025
@petrochenkov
Copy link
Contributor Author

@bors rollup

// Assume `-C linker=rust-lld` as self-contained mode
if linker == Path::new("rust-lld") {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Below only the sysroot from which we get the standard library is checked, not the fallback sysroot that rustc itself is contained in. rust-lld is likely to be contained only in the latter if the former is overriden.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fairly sure from my tests the sysroot would point at ~/.rustup/toolchains/<toolchain>.

If I read

fn default_from_rustc_driver_dll() -> Result<PathBuf, String> {
right, it would indeed stirp all the subdirectories.

Copy link
Member

@bjorn3 bjorn3 Dec 3, 2025

Choose a reason for hiding this comment

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

The code below only strips sess.opts.sysroot.path(), which only returns the value explicitly passed to --sysroot if passed, only returning the fallback sysroot when it isn't passed.

/// Return explicit sysroot if it was passed with `--sysroot`, or default sysroot otherwise.
pub fn path(&self) -> &Path {
self.explicit.as_deref().unwrap_or(&self.default)
}
You need sess.opts.sysroot.all_paths() to check both.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, but is --sysroot ever passed to rustc when building natively (host == target)?

Copy link
Member

Choose a reason for hiding this comment

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

There are legitimate reasons to pass it when building natively. For example miri does for it's copy of the standard library that contains MIR for all functions (though it doesn't need a linker). cargo build -Zbuild-std doesn't use --sysroot currently, but I think it should pass --sysroot /dev/null or something like that to prevent accidentally picking up libraries from the sysroot. The only thing that leads to is lang item conflicts. --sysroot /dev/null would give nicer error messages.

Copy link
Member

Choose a reason for hiding this comment

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

--sysroot /dev/null would break all cases of implicit self-contained detection, not just rust-lld. I mean, there is no obvious issue caused by this PR, not that there is no general issue with implicit self-contained detection.

Copy link
Member

Choose a reason for hiding this comment

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

Right. The self-contained components are themself only looked up in --sysroot. Didn't think of that.

bors added a commit that referenced this pull request Dec 3, 2025
Rollup of 5 pull requests

Successful merges:

 - #148918 (Remove an outdated test)
 - #149244 (Fix std::mem::drop rustdoc misleading statement)
 - #149532 (Rename supertrait item shadowing lints)
 - #149541 (Various never type test improvements)
 - #149590 (linker: Remove special case for `rust-lld` in `detect_self_contained_mingw`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2cc93b8 into rust-lang:main Dec 3, 2025
2 of 3 checks passed
rust-timer added a commit that referenced this pull request Dec 3, 2025
Rollup merge of #149590 - petrochenkov:norlld, r=mati865

linker: Remove special case for `rust-lld` in `detect_self_contained_mingw`

`rust-lld` is supposed to live inside sysroot, so it doesn't change the behavior of the function, only removes a potential micro-optimization.

#149178 (comment)
r? `@mati865`
@rustbot rustbot added this to the 1.93.0 milestone Dec 3, 2025
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