Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deal cleanly with malformed default-host #1578

Merged
merged 7 commits into from
Jan 22, 2019
Merged

Deal cleanly with malformed default-host #1578

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift click to select a range
26972a5
Remove panic on malformed default host
kinnison Dec 14, 2018
87c36da
Allow configuration to smoke-test itself
kinnison Dec 14, 2018
5481115
Fix minor typo in warning
kinnison Dec 16, 2018
983b5a6
Add a log macro at debug level
kinnison Dec 16, 2018
e6a2dc9
Verify the provided toolchain and host triple on install
kinnison Dec 16, 2018
239b34a
Permit 'none' as a toolchain
kinnison Dec 16, 2018
36ec58f
rustup: Prevent setting an invalid default host
kinnison Jan 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Verify the provided toolchain and host triple on install 
When rustup-init is running, we need to verify that the provided
host triple and toolchain channel can be used together to determine
a full toolchain triple (quadruple?).  If we can't do this during
the sanity checks, error out early.
  • Loading branch information
@kinnison
kinnison committed Jan 16, 2019
commit e6a2dc91e1867a7466b585bccd2ea55f99fd5087
25 changes: 25 additions & 0 deletions src/rustup-cli/self_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 223,7 @@ fn canonical_cargo_home() -> Result<String> {
/// and adding `CARGO_HOME`/bin to PATH.
pub fn install(no_prompt: bool, verbose: bool, mut opts: InstallOpts) -> Result<()> {
do_pre_install_sanity_checks()?;
do_pre_install_options_sanity_checks(&opts)?;
check_existence_of_rustc_or_cargo_in_path(no_prompt)?;
do_anti_sudo_check(no_prompt)?;

Expand Down Expand Up @@ -463,6 464,30 @@ fn do_pre_install_sanity_checks() -> Result<()> {
Ok(())
}

fn do_pre_install_options_sanity_checks(opts: &InstallOpts) -> Result<()> {
// Verify that the installation options are vaguely sane
(|| {
let host_triple = dist::TargetTriple::from_str(&opts.default_host_triple);
let partial_channel = dist::PartialToolchainDesc::from_str(&opts.default_toolchain)?;
let resolved = partial_channel.resolve(&host_triple)?.to_string();
debug!(
"Successfully resolved installation toolchain as: {}",
resolved
);
Ok(())
})()
.map_err(|e: Box<std::error::Error>| {
format!(
"Pre-checks for host and toolchain failed: {}\n\
If you are unsure of suitable values, the 'stable' toolchain is the default.\n\
Valid host triples look something like: {}",
e,
dist::TargetTriple::from_host_or_build()
)
})?;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should move this checking to its own function - do_pre_install_sanity_checks is about checking the user's system, rather than the installation options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've created a function called do_pre_install_options_sanity_checks() and moved the content into there, though for now I'm calling it from the same spot as do_pre_install_sanity_checks() unless I find something better.

Ok(())
}

// If the user is trying to install with sudo, on some systems this will
// result in writing root-owned files to the user's home directory, because
// sudo is configured not to change $HOME. Don't let that bogosity happen.
Expand Down