refactor: move sysroot lookup to GlobalContext - #17276
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| let mut ret = PathBuf::new(); | ||
| // clone_from avoids reallocation | ||
| ret.clone_from(sysroot); | ||
| ret |
There was a problem hiding this comment.
This would be true if ret had existed before, but it doesn't.
| } | ||
|
|
||
| /// Use the rustc executable to fetch the sysroot path. | ||
| pub fn sysroot(&self, gctx: &GlobalContext) -> CargoResult<PathBuf> { |
There was a problem hiding this comment.
Is this supposed to be in the previous commit?
| /// Path to the sysroot. | ||
| pub sysroot: PathBuf, |
There was a problem hiding this comment.
We aren't saving this off but we are still looking it up in new to derive sysroot_target_libdir.
Should we remove the lookup here?
What does this PR try to resolve?
I split this out from a refactor of #16675, as part of work on build-std=always. In this PR, and in other build-std work I have in progress, it's useful to know the sysroot path earlier than the
compiler/stage of Cargo. Because there's one rustc instance per Cargo invocation, and because the sysroot path remains fixed per rustc and isn't dependent on the target, this should be safe to move out of TargetInfo.How to test and review this PR?
The first commit shows the logic being moved, and the second commit shows updating all the users of this logic.
Getting the sysroot is already heavily tested in Cargo, hence why I didn't add a new test for this.