Skip to content

Change handling of renames #1549

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

Merged
merged 8 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/rustup-cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ pub fn rustc_version(toolchain: &Toolchain) -> String {
pub fn list_targets(toolchain: &Toolchain) -> Result<()> {
let mut t = term2::stdout();
for component in toolchain.list_components()? {
if component.component.pkg == "rust-std" {
if component.component.name_in_manifest() == "rust-std" {
let target = component
.component
.target
Expand All @@ -310,7 +310,7 @@ pub fn list_targets(toolchain: &Toolchain) -> Result<()> {
pub fn list_components(toolchain: &Toolchain) -> Result<()> {
let mut t = term2::stdout();
for component in toolchain.list_components()? {
let name = component.component.name();
let name = component.name;
if component.required {
let _ = t.attr(term2::Attr::Bold);
let _ = writeln!(t, "{} (default)", name);
Expand Down
22 changes: 5 additions & 17 deletions src/rustup-cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ fn show(cfg: &Cfg) -> Result<()> {
match t.list_components() {
Ok(cs_vec) => cs_vec
.into_iter()
.filter(|c| c.component.pkg == "rust-std")
.filter(|c| c.component.name_in_manifest() == "rust-std")
.filter(|c| c.installed)
.collect(),
Err(_) => vec![],
Expand Down Expand Up @@ -778,10 +778,7 @@ fn target_add(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
let toolchain = explicit_or_dir_toolchain(cfg, m)?;

for target in m.values_of("target").expect("") {
let new_component = Component {
pkg: "rust-std".to_string(),
target: Some(TargetTriple::from_str(target)),
};
let new_component = Component::new("rust-std".to_string(), Some(TargetTriple::from_str(target)));

toolchain.add_component(new_component)?;
}
Expand All @@ -793,10 +790,7 @@ fn target_remove(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
let toolchain = explicit_or_dir_toolchain(cfg, m)?;

for target in m.values_of("target").expect("") {
let new_component = Component {
pkg: "rust-std".to_string(),
target: Some(TargetTriple::from_str(target)),
};
let new_component = Component::new("rust-std".to_string(), Some(TargetTriple::from_str(target)));

toolchain.remove_component(new_component)?;
}
Expand All @@ -823,10 +817,7 @@ fn component_add(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
});

for component in m.values_of("component").expect("") {
let new_component = Component {
pkg: component.to_string(),
target: target.clone(),
};
let new_component = Component::new(component.to_string(), target.clone());

toolchain.add_component(new_component)?;
}
Expand All @@ -847,10 +838,7 @@ fn component_remove(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
});

for component in m.values_of("component").expect("") {
let new_component = Component {
pkg: component.to_string(),
target: target.clone(),
};
let new_component = Component::new(component.to_string(), target.clone());

toolchain.remove_component(new_component)?;
}
Expand Down
9 changes: 1 addition & 8 deletions src/rustup-cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use common::{self, Confirm};
use errors::*;
use rustup_dist::dist;
use rustup_utils::utils;
use rustup::{TOOLS, DUP_TOOLS};
use same_file::Handle;
use std::env;
use std::env::consts::EXE_SUFFIX;
Expand Down Expand Up @@ -192,14 +193,6 @@ doing then it is fine to continue installation without the build
tools, but otherwise, install the C++ build tools before proceeding.
"#;

static TOOLS: &'static [&'static str] =
&["rustc", "rustdoc", "cargo", "rust-lldb", "rust-gdb", "rls", "cargo-clippy"];

// Tools which are commonly installed by Cargo as well as rustup. We take a bit
// more care with these to ensure we don't overwrite the user's previous
// installation.
static DUP_TOOLS: &'static [&'static str] = &["rustfmt", "cargo-fmt"];

static UPDATE_ROOT: &'static str = "https://static.rust-lang.org/rustup";

/// `CARGO_HOME` suitable for display, possibly with $HOME
Expand Down
36 changes: 11 additions & 25 deletions src/rustup-dist/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::io::{self, Write};
use temp;
use toml;
use rustup_utils;
use manifest::Component;
use manifest::{Component, Manifest};

error_chain! {
links {
Expand Down Expand Up @@ -82,15 +82,9 @@ error_chain! {
ComponentFilePermissionsFailed {
description("error setting file permissions during install")
}
ComponentDownloadFailed(c: Component) {
ComponentDownloadFailed(c: String) {
description("component download failed")
display("component download failed for {}{}", c.pkg, {
if let Some(ref t) = c.target {
format!("-{}", t)
} else {
"".to_owned()
}
})
display("component download failed for {}", c)
}
Parsing(e: toml::de::Error) {
description("error parsing manifest")
Expand All @@ -99,50 +93,42 @@ error_chain! {
description("unsupported manifest version")
display("manifest version '{}' is not supported", v)
}
MissingPackageForComponent(c: Component) {
MissingPackageForComponent(name: String) {
description("missing package for component")
display("server sent a broken manifest: missing package for component {}", c.name())
display("server sent a broken manifest: missing package for component {}", name)
}
MissingPackageForRename(name: String) {
description("missing package for the target of a rename")
display("server sent a broken manifest: missing package for the target of a rename {}", name)
}
RequestedComponentsUnavailable(c: Vec<Component>) {
RequestedComponentsUnavailable(c: Vec<Component>, manifest: Manifest) {
description("some requested components are unavailable to download")
display("{}", component_unavailable_msg(&c))
display("{}", component_unavailable_msg(&c, &manifest))
}
}
}

fn component_unavailable_msg(cs: &[Component]) -> String {
fn component_unavailable_msg(cs: &[Component], manifest: &Manifest) -> String {
assert!(!cs.is_empty());

let mut buf = vec![];

fn format_component(c: &Component) -> String {
if let Some(ref t) = c.target {
format!("'{}' for '{}'", c.pkg, t)
} else {
format!("'{}'", c.pkg)
}
}

if cs.len() == 1 {
let _ = write!(
buf,
"component {} is unavailable for download",
format_component(&cs[0])
&cs[0].description(manifest)
);
} else {
use itertools::Itertools;
let same_target = cs.iter()
.all(|c| c.target == cs[0].target || c.target.is_none());
if same_target {
let mut cs_strs = cs.iter().map(|c| format!("'{}'", c.pkg));
let mut cs_strs = cs.iter().map(|c| format!("'{}'", c.short_name(manifest)));
let cs_str = cs_strs.join(", ");
let _ = write!(buf, "some components unavailable for download: {}", cs_str);
} else {
let mut cs_strs = cs.iter().map(format_component);
let mut cs_strs = cs.iter().map(|c| c.description(manifest));
let cs_str = cs_strs.join(", ");
let _ = write!(buf, "some components unavailable for download: {}", cs_str);
}
Expand Down
69 changes: 53 additions & 16 deletions src/rustup-dist/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub struct Manifest {
pub date: String,
pub packages: HashMap<String, Package>,
pub renames: HashMap<String, String>,
pub reverse_renames: HashMap<String, String>,
}

#[derive(Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -57,7 +58,7 @@ pub struct PackageBins {

#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct Component {
pub pkg: String,
pkg: String,
pub target: Option<TargetTriple>,
}

Expand All @@ -78,11 +79,13 @@ impl Manifest {
if !SUPPORTED_MANIFEST_VERSIONS.contains(&&*version) {
return Err(ErrorKind::UnsupportedVersion(version).into());
}
let (renames, reverse_renames) = Self::table_to_renames(&mut table, path)?;
Ok(Manifest {
manifest_version: version,
date: get_string(&mut table, "date", path)?,
packages: Self::table_to_packages(&mut table, path)?,
renames: Self::table_to_renames(table, path)?,
renames,
reverse_renames,
})
}
pub fn to_toml(self) -> toml::value::Table {
Expand Down Expand Up @@ -127,19 +130,22 @@ impl Manifest {
}

fn table_to_renames(
mut table: toml::value::Table,
table: &mut toml::value::Table,
path: &str,
) -> Result<HashMap<String, String>> {
let mut result = HashMap::new();
let rename_table = get_table(&mut table, "rename", path)?;
) -> Result<(HashMap<String, String>, HashMap<String, String>)> {
let mut renames = HashMap::new();
let mut reverse_renames = HashMap::new();
let rename_table = get_table(table, "rename", path)?;

for (k, v) in rename_table {
if let toml::Value::Table(mut t) = v {
result.insert(k.to_owned(), get_string(&mut t, "to", path)?);
let to = get_string(&mut t, "to", path)?;
renames.insert(k.to_owned(), to.clone());
reverse_renames.insert(to, k.to_owned());
}
}

Ok(result)
Ok((renames, reverse_renames))
}
fn renames_to_table(renames: HashMap<String, String>) -> toml::value::Table {
let mut result = toml::value::Table::new();
Expand All @@ -164,9 +170,9 @@ impl Manifest {
fn validate_targeted_package(&self, tpkg: &TargetedPackage) -> Result<()> {
for c in tpkg.components.iter().chain(tpkg.extensions.iter()) {
let cpkg = self.get_package(&c.pkg)
.chain_err(|| ErrorKind::MissingPackageForComponent(c.clone()))?;
.chain_err(|| ErrorKind::MissingPackageForComponent(c.short_name(self)))?;
let _ctpkg = cpkg.get_target(c.target.as_ref())
.chain_err(|| ErrorKind::MissingPackageForComponent(c.clone()))?;
.chain_err(|| ErrorKind::MissingPackageForComponent(c.short_name(self)))?;
}
Ok(())
}
Expand Down Expand Up @@ -194,6 +200,16 @@ impl Manifest {

Ok(())
}

// If the component should be renamed by this manifest, then return a new
// component with the new name. If not, return `None`.
pub fn rename_component(&self, component: &Component) -> Option<Component> {
self.renames.get(&component.pkg).map(|r| {
let mut c = component.clone();
c.pkg = r.clone();
c
})
}
}

impl Package {
Expand Down Expand Up @@ -359,6 +375,15 @@ impl TargetedPackage {
}

impl Component {
pub fn new(pkg: String, target: Option<TargetTriple>) -> Component {
Component { pkg, target }
}
pub fn wildcard(&self) -> Component {
Component {
pkg: self.pkg.clone(),
target: None,
}
}
pub fn from_toml(mut table: toml::value::Table, path: &str) -> Result<Self> {
Ok(Component {
pkg: get_string(&mut table, "pkg", path)?,
Expand All @@ -384,18 +409,30 @@ impl Component {
result.insert("pkg".to_owned(), toml::Value::String(self.pkg));
result
}
pub fn name(&self) -> String {
pub fn name(&self, manifest: &Manifest) -> String {
let pkg = self.short_name(manifest);
if let Some(ref t) = self.target {
format!("{}-{}", self.pkg, t)
format!("{}-{}", pkg, t)
} else {
format!("{}", pkg)
}
}
pub fn short_name(&self, manifest: &Manifest) -> String {
if let Some(from) = manifest.reverse_renames.get(&self.pkg) {
from.to_owned()
} else {
format!("{}", self.pkg)
self.pkg.clone()
}
}
pub fn description(&self) -> String {
pub fn description(&self, manifest: &Manifest) -> String {
let pkg = self.short_name(manifest);
if let Some(ref t) = self.target {
format!("'{}' for target '{}'", self.pkg, t)
format!("'{}' for target '{}'", pkg, t)
} else {
format!("'{}'", self.pkg)
format!("'{}'", pkg)
}
}
pub fn name_in_manifest(&self) -> &String {
&self.pkg
}
}
Loading