Skip to content

Commit cc773b2

Browse files
kinnisonkinnison
authored
Merge pull request #2240 from rbtcollins/guard-components
Make list_components safer to work with
2 parents 4656d95 + b0a037e commit cc773b2

File tree

3 files changed

+79
-15
lines changed

3 files changed

+79
-15
lines changed

‎src/cli/common.rs

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,17 @@ where
318318

319319
pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
320320
let mut t = term2::stdout();
321-
for component in toolchain.list_components()? {
321+
let components = match toolchain.list_components()? {
322+
// XXX: long term move this error to cli ? the normal .into doesn't work
323+
// because Result here is the wrong sort and expression type ascription
324+
// isn't a feature yet.
325+
None => Err(rustup::Error(
326+
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
327+
error_chain::State::default(),
328+
)),
329+
Some(components) => Ok(components),
330+
}?;
331+
for component in components {
322332
if component.component.short_name_in_manifest() == "rust-std" {
323333
let target = component
324334
.component
@@ -340,7 +350,17 @@ pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
340350

341351
pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
342352
let mut t = term2::stdout();
343-
for component in toolchain.list_components()? {
353+
let components = match toolchain.list_components()? {
354+
// XXX: long term move this error to cli ? the normal .into doesn't work
355+
// because Result here is the wrong sort and expression type ascription
356+
// isn't a feature yet.
357+
None => Err(rustup::Error(
358+
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
359+
error_chain::State::default(),
360+
)),
361+
Some(components) => Ok(components),
362+
}?;
363+
for component in components {
344364
if component.component.short_name_in_manifest() == "rust-std" {
345365
let target = component
346366
.component
@@ -357,7 +377,17 @@ pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
357377

358378
pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
359379
let mut t = term2::stdout();
360-
for component in toolchain.list_components()? {
380+
let components = match toolchain.list_components()? {
381+
// XXX: long term move this error to cli ? the normal .into doesn't work
382+
// because Result here is the wrong sort and expression type ascription
383+
// isn't a feature yet.
384+
None => Err(rustup::Error(
385+
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
386+
error_chain::State::default(),
387+
)),
388+
Some(components) => Ok(components),
389+
}?;
390+
for component in components {
361391
let name = component.name;
362392
if component.installed {
363393
t.attr(term2::Attr::Bold)?;
@@ -373,7 +403,17 @@ pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
373403

374404
pub fn list_installed_components(toolchain: &Toolchain<'_>) -> Result<()> {
375405
let mut t = term2::stdout();
376-
for component in toolchain.list_components()? {
406+
let components = match toolchain.list_components()? {
407+
// XXX: long term move this error to cli ? the normal .into doesn't work
408+
// because Result here is the wrong sort and expression type ascription
409+
// isn't a feature yet.
410+
None => Err(rustup::Error(
411+
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
412+
error_chain::State::default(),
413+
)),
414+
Some(components) => Ok(components),
415+
}?;
416+
for component in components {
377417
if component.installed {
378418
writeln!(t, "{}", component.name)?;
379419
}

‎src/cli/rustup_mode.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustup::dist::dist::{PartialTargetTriple, PartialToolchainDesc, Profile, Tar
1010
use rustup::dist::manifest::Component;
1111
use rustup::utils::utils::{self, ExitCode};
1212
use rustup::Notification;
13-
use rustup::{command, Cfg, Toolchain};
13+
use rustup::{command, Cfg, ComponentStatus, Toolchain};
1414
use std::error::Error;
1515
use std::fmt;
1616
use std::io::Write;
@@ -934,7 +934,8 @@ fn show(cfg: &Cfg) -> Result<()> {
934934
let active_targets = if let Ok(ref at) = active_toolchain {
935935
if let Some((ref t, _)) = *at {
936936
match t.list_components() {
937-
Ok(cs_vec) => cs_vec
937+
Ok(None) => vec![],
938+
Ok(Some(cs_vec)) => cs_vec
938939
.into_iter()
939940
.filter(|c| c.component.short_name_in_manifest() == "rust-std")
940941
.filter(|c| c.installed)
@@ -1079,6 +1080,18 @@ fn target_list(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
10791080

10801081
fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
10811082
let toolchain = explicit_or_dir_toolchain(cfg, m)?;
1083+
// XXX: long term move this error to cli ? the normal .into doesn't work
1084+
// because Result here is the wrong sort and expression type ascription
1085+
// isn't a feature yet.
1086+
// list_components *and* add_component would both be inappropriate for
1087+
// custom toolchains.
1088+
if toolchain.is_custom() {
1089+
return Err(rustup::Error(
1090+
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
1091+
error_chain::State::default(),
1092+
)
1093+
.into());
1094+
}
10821095

10831096
let mut targets: Vec<String> = m
10841097
.values_of("target")
@@ -1092,7 +1105,7 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
10921105
}
10931106

10941107
targets.clear();
1095-
for component in toolchain.list_components()? {
1108+
for component in toolchain.list_components()?.unwrap() {
10961109
if component.component.short_name_in_manifest() == "rust-std"
10971110
&& component.available
10981111
&& !component.installed
@@ -1312,9 +1325,18 @@ const DOCS_DATA: &[(&str, &str, &str,)] = &[
13121325

13131326
fn doc(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
13141327
let toolchain = explicit_or_dir_toolchain(cfg, m)?;
1315-
if !toolchain.is_custom() {
1316-
for cstatus in &toolchain.list_components()? {
1317-
if cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed {
1328+
match toolchain.list_components()? {
1329+
None => { /* custom - no validation */ }
1330+
Some(components) => {
1331+
if let [_] = components
1332+
.into_iter()
1333+
.filter(|cstatus| {
1334+
cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
1335+
})
1336+
.take(1)
1337+
.collect::<Vec<ComponentStatus>>()
1338+
.as_slice()
1339+
{
13181340
info!(
13191341
"`rust-docs` not installed in toolchain `{}`",
13201342
toolchain.name()

‎src/toolchain.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -526,14 +526,16 @@ impl<'a> Toolchain<'a> {
526526
}
527527
}
528528

529-
pub fn list_components(&self) -> Result<Vec<ComponentStatus>> {
529+
pub fn list_components(&self) -> Result<Option<Vec<ComponentStatus>>> {
530530
if !self.exists() {
531531
return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
532532
}
533533

534534
let toolchain = &self.name;
535-
let toolchain = ToolchainDesc::from_str(toolchain)
536-
.chain_err(|| ErrorKind::ComponentsUnsupported(self.name.to_string()))?;
535+
let toolchain = match ToolchainDesc::from_str(toolchain).ok() {
536+
None => return Ok(None),
537+
Some(toolchain) => toolchain,
538+
};
537539

538540
let prefix = InstallPrefix::from(self.path.to_owned());
539541
let manifestation = Manifestation::open(prefix, toolchain.target.clone())?;
@@ -586,7 +588,7 @@ impl<'a> Toolchain<'a> {
586588

587589
res.sort_by(|a, b| a.component.cmp(&b.component));
588590

589-
Ok(res)
591+
Ok(Some(res))
590592
} else {
591593
Err(ErrorKind::ComponentsUnsupported(self.name.to_string()).into())
592594
}
@@ -605,7 +607,7 @@ impl<'a> Toolchain<'a> {
605607
const MAX_DISTANCE: usize = 3;
606608

607609
let components = self.list_components();
608-
if let Ok(components) = components {
610+
if let Ok(Some(components)) = components {
609611
let short_name_distance = components
610612
.iter()
611613
.filter(|c| !only_installed || c.installed)

0 commit comments

Comments
 (0)