-
Notifications
You must be signed in to change notification settings - Fork 13k
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
improvements on initial sysroot and libdir finding logics #132782
Conversation
Signed-off-by: onur-ozkan <[email protected]>
rustbot has assigned @albertlarsan68. Use |
This PR modifies If appropriate, please update |
src/bootstrap/src/lib.rs
Outdated
// FIXME(Zalathar): Determining this path occasionally fails locally for | ||
// unknown reasons, so we print some extra context to help track down why. | ||
let find_initial_libdir = || { | ||
let initial_libdir = | ||
initial_target_dir.parent()?.parent()?.strip_prefix(&initial_sysroot).ok()?; | ||
Some(initial_libdir.to_path_buf()) | ||
}; | ||
let Some(initial_libdir) = find_initial_libdir() else { | ||
panic!( | ||
"couldn't determine `initial_libdir`: | ||
- config.initial_rustc: {rustc:?} | ||
- initial_target_libdir_str: {initial_target_libdir_str:?} | ||
- initial_target_dir: {initial_target_dir:?} | ||
- initial_sysroot: {initial_sysroot:?} | ||
", | ||
rustc = config.initial_rustc, | ||
); | ||
}; | ||
let initial_libdir = initial_target_dir | ||
.ancestors() | ||
.nth(2) | ||
.unwrap() | ||
.strip_prefix(&config.initial_sysroot) | ||
.unwrap() | ||
.to_path_buf(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have never experienced this failing before and it should be even more stable now.
cc @Zalathar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still see this fail occasionally (on macOS only), but I have already collected plenty of examples of the failure so I don’t really need this code any more.
If you replace the unwrap with something like .expect("couldn’t determine initial libdir")
, I’m fine with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, the failure looks like this: #130138 (comment)
Two different invocations of the same compiler binary produce different sysroot paths. It probably has something to do with hardlink bookkeeping on macOS specifically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see above that we aren’t invoking rustc multiple times anymore, so yeah from a bootstrap perspective this should avoid the problem I was seeing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you replace the unwrap with something like .expect("couldn’t determine initial libdir"), I’m fine with that.
Signed-off-by: onur-ozkan <[email protected]>
r? bootstrap (due to inactivity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks like a nice cleanup.
@bors r+ rollup |
improvements on initial sysroot and libdir finding logics Stabilized initial sysroot and libdir path resolution logic to work without dry-run conditions and utilized initial sysroot more broadly.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#132782 (improvements on initial sysroot and libdir finding logics) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133466 (Fix typos in pin.rs) - rust-lang#133492 (bootstrap: allow skipping steps with start of path) - rust-lang#133501 (support revealing defined opaque post borrowck) - rust-lang#133530 (Use consistent wording in docs, use is zero instead of is 0) - rust-lang#133538 (Better diagnostic for fn items in variadic functions) - rust-lang#133590 (Rename `-Zparse-only`) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#132782 (improvements on initial sysroot and libdir finding logics) - rust-lang#133466 (Fix typos in pin.rs) - rust-lang#133492 (bootstrap: allow skipping steps with start of path) - rust-lang#133501 (support revealing defined opaque post borrowck) - rust-lang#133530 (Use consistent wording in docs, use is zero instead of is 0) - rust-lang#133538 (Better diagnostic for fn items in variadic functions) - rust-lang#133590 (Rename `-Zparse-only`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132782 - onur-ozkan:cleanup, r=jieyouxu improvements on initial sysroot and libdir finding logics Stabilized initial sysroot and libdir path resolution logic to work without dry-run conditions and utilized initial sysroot more broadly.
Stabilized initial sysroot and libdir path resolution logic to work without dry-run conditions and utilized initial sysroot more broadly.