-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Replace all uses of sysroot_candidates with get_or_default_sysroot #142089
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
Conversation
And consistently use try_canonicalize rather than canonicalize.
Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used: * get_or_default_sysroot which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path., * sysroot_candidates which takes the former and additionally does another attempt at locating using librustc_driver.so except without linux multiarch handling and then returns both paths., The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the --sysroot passed in does not contain a copy of cg_llvm. Back then get_or_default_sysroot did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removing sysroot_candidates can break things is if you have a custom driver inside what looks like a sysroot including the lib/rustlib directory, but which is missing some parts of the full sysroot like eg rust-lld.
|
); | ||
} | ||
} | ||
pub fn sysroot_with_fallback(sysroot: &Path) -> SmallVec<[PathBuf; 2]> { |
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.
This is an improvement, but it still feels a bit sub-optimal.
The passed sysroot
here can only be --sysroot
or get_or_default_sysroot()
, so perhaps it makes sense to make a structure like
struct Sysroot {
explicit: Option<PathBuf>,
default: PathBuf,
}
with two methods for accessing either the main path or both, and then pass that structure around.
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.
The Session
only stores the result of materialize_sysroot()
, which does maybe_sysroot.unwrap_or_else(|| get_or_default_sysroot())
.
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
@rustbot ready |
@bors r+ |
…r=petrochenkov Replace all uses of sysroot_candidates with get_or_default_sysroot Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used: * `get_or_default_sysroot` which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path., * `sysroot_candidates` which takes the former and additionally does another attempt at locating using `librustc_driver.so` except without linux multiarch handling and then returns both paths., The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the `--sysroot` passed in does not contain a copy of cg_llvm. Back then `get_or_default_sysroot` did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removing `sysroot_candidates` can break things is if you have a custom driver inside what looks like a sysroot including the `lib/rustlib` directory, but which is missing some parts of the full sysroot like eg rust-lld. Follow up to rust-lang#138404
Rollup of 11 pull requests Successful merges: - #140774 (Affirm `-Cforce-frame-pointers=off` does not override) - #141001 (Make NonZero<char> possible) - #141700 (Atomic intrinsics : use const generic ordering, part 2) - #142008 (const-eval error: always say in which item the error occurred) - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`) - #142089 (Replace all uses of sysroot_candidates with get_or_default_sysroot) - #142108 (compiler: Add track_caller to AbiMapping::unwrap) - #142132 (`tests/ui`: A New Order [6/N]) - #142162 (UnsafePinned: update get() docs and signature to allow shared mutation) - #142171 (`tests/ui`: A New Order [7/N]) - #142179 (store `target.min_global_align` as an `Align`) r? `@ghost` `@rustbot` modify labels: rollup
…r=petrochenkov Replace all uses of sysroot_candidates with get_or_default_sysroot Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used: * `get_or_default_sysroot` which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path., * `sysroot_candidates` which takes the former and additionally does another attempt at locating using `librustc_driver.so` except without linux multiarch handling and then returns both paths., The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the `--sysroot` passed in does not contain a copy of cg_llvm. Back then `get_or_default_sysroot` did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removing `sysroot_candidates` can break things is if you have a custom driver inside what looks like a sysroot including the `lib/rustlib` directory, but which is missing some parts of the full sysroot like eg rust-lld. Follow up to rust-lang#138404
Rollup of 12 pull requests Successful merges: - #141803 (Remove rustc's notion of "preferred" alignment AKA `__alignof`) - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`) - #142089 (Replace all uses of sysroot_candidates with get_or_default_sysroot) - #142108 (compiler: Add track_caller to AbiMapping::unwrap) - #142132 (`tests/ui`: A New Order [6/N]) - #142162 (UnsafePinned: update get() docs and signature to allow shared mutation) - #142171 (`tests/ui`: A New Order [7/N]) - #142179 (store `target.min_global_align` as an `Align`) - #142183 (Added test for 30904) - #142194 (Remove all unused feature gates from the compiler) - #142199 (Do not free disk space in the `mingw-check-tidy` job) - #142210 (Run `mingw-check-tidy` on auto builds) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #142089 - bjorn3:sysroot_handling_cleanup3, r=petrochenkov Replace all uses of sysroot_candidates with get_or_default_sysroot Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used: * `get_or_default_sysroot` which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path., * `sysroot_candidates` which takes the former and additionally does another attempt at locating using `librustc_driver.so` except without linux multiarch handling and then returns both paths., The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the `--sysroot` passed in does not contain a copy of cg_llvm. Back then `get_or_default_sysroot` did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removing `sysroot_candidates` can break things is if you have a custom driver inside what looks like a sysroot including the `lib/rustlib` directory, but which is missing some parts of the full sysroot like eg rust-lld. Follow up to #138404
Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used:
get_or_default_sysroot
which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path.,sysroot_candidates
which takes the former and additionally does another attempt at locating usinglibrustc_driver.so
except without linux multiarch handling and then returns both paths.,The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the
--sysroot
passed in does not contain a copy of cg_llvm. Back thenget_or_default_sysroot
did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removingsysroot_candidates
can break things is if you have a custom driver inside what looks like a sysroot including thelib/rustlib
directory, but which is missing some parts of the full sysroot like eg rust-lld.Follow up to #138404