Remove the hard-wired legacy profile

nrc · nrc · commit feb1222d1610 · 2019-06-13T11:54:53.000+12:00
diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs
@@ -900,7 +900,11 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     }
 
     for target in &targets {
-        let new_component = Component::new("rust-std".to_string(), Some(TargetTriple::new(target)));
+        let new_component = Component::new(
+            "rust-std".to_string(),
+            Some(TargetTriple::new(target)),
+            false,
+        );
         toolchain.add_component(new_component)?;
     }
 
@@ -911,7 +915,11 @@ 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::new("rust-std".to_string(), Some(TargetTriple::new(target)));
+        let new_component = Component::new(
+            "rust-std".to_string(),
+            Some(TargetTriple::new(target)),
+            false,
+        );
 
         toolchain.remove_component(new_component)?;
     }
@@ -940,7 +948,7 @@ fn component_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     });
 
     for component in m.values_of("component").expect("") {
-        let new_component = Component::new(component.to_string(), target.clone());
+        let new_component = Component::new(component.to_string(), target.clone(), true);
 
         toolchain.add_component(new_component)?;
     }
@@ -959,7 +967,7 @@ fn component_remove(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     });
 
     for component in m.values_of("component").expect("") {
-        let new_component = Component::new(component.to_string(), target.clone());
+        let new_component = Component::new(component.to_string(), target.clone(), true);
 
         toolchain.remove_component(new_component)?;
     }
diff --git a/src/dist/config.rs b/src/dist/config.rs
@@ -67,7 +67,7 @@ impl Config {
         for (i, v) in arr.into_iter().enumerate() {
             if let toml::Value::Table(t) = v {
                 let path = format!("{}[{}]", path, i);
-                result.push(Component::from_toml(t, &path)?);
+                result.push(Component::from_toml(t, &path, false)?);
             }
         }
 
diff --git a/src/dist/dist.rs b/src/dist/dist.rs
@@ -1,5 +1,4 @@
 use crate::dist::download::DownloadCfg;
-use crate::dist::manifest::Component;
 use crate::dist::manifest::Manifest as ManifestV2;
 use crate::dist::manifestation::{Changes, Manifestation, UpdateStatus};
 use crate::dist::notifications::*;
@@ -490,14 +489,6 @@ impl Profile {
     pub fn default_name() -> &'static str {
         "default"
     }
-
-    // Non-required components that were required before profiles exist.
-    pub fn legacy_profile() -> Vec<String> {
-        ["rust-std", "rustc", "cargo", "rust-docs"]
-            .into_iter()
-            .map(|s| (*s).to_owned())
-            .collect()
-    }
 }
 
 impl Default for Profile {
@@ -619,27 +610,13 @@ fn update_from_dist_<'a>(
                 m.get_rust_version().ok(),
             ));
 
-            let add_components = profile
-                .and_then(|profile| {
-                    m.get_profile_components(profile, &toolchain.target)
-                        .ok()
-                        .map(|v| {
-                            v.iter()
-                                .map(|name| {
-                                    Component::new(name.to_owned(), Some(toolchain.target.clone()))
-                                })
-                                .collect::<Vec<Component>>()
-                        })
-                })
-                .unwrap_or_else(|| {
-                    Profile::legacy_profile()
-                        .into_iter()
-                        .map(|s| Component::new(s, Some(toolchain.target.clone())))
-                        .collect()
-                });
+            let implicit_add_components = match profile {
+                Some(profile) => m.get_profile_components(profile, &toolchain.target)?,
+                None => Vec::new(),
+            };
 
             let changes = Changes {
-                implicit_add_components: add_components,
+                implicit_add_components,
                 explicit_add_components: Vec::new(),
                 remove_components: Vec::new(),
             };
diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs
@@ -55,10 +55,19 @@ pub struct PackageBins {
     pub xz_hash: Option<String>,
 }
 
-#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[derive(Clone, Debug, Eq, Ord, PartialOrd, Hash)]
 pub struct Component {
     pkg: String,
     pub target: Option<TargetTriple>,
+    // Older Rustup distinguished between components (which are essential) and
+    // extensions (which are not).
+    is_extension: bool,
+}
+
+impl PartialEq for Component {
+    fn eq(&self, other: &Component) -> bool {
+        self.pkg == other.pkg && self.target == other.target
+    }
 }
 
 impl Manifest {
@@ -165,7 +174,10 @@ impl Manifest {
         path: &str,
     ) -> Result<HashMap<Profile, Vec<String>>> {
         let mut result = HashMap::new();
-        let profile_table = get_table(table, "profiles", path)?;
+        let profile_table = match get_table(table, "profiles", path) {
+            Ok(t) => t,
+            Err(_) => return Ok(result),
+        };
 
         for (k, v) in profile_table {
             if let toml::Value::Array(a) = v {
@@ -201,11 +213,29 @@ impl Manifest {
         self.get_package("rust").map(|p| &*p.version)
     }
 
+    pub fn get_legacy_components(&self, target: &TargetTriple) -> Result<Vec<Component>> {
+        // Build a profile from the components/extensions.
+        let result = self
+            .get_package("rust")?
+            .get_target(Some(target))?
+            .components
+            .iter()
+            .filter(|c| !c.is_extension && c.target.as_ref().map(|t| t == target).unwrap_or(true))
+            .map(|c| c.clone())
+            .collect();
+
+        return Ok(result);
+    }
     pub fn get_profile_components(
         &self,
         profile: Profile,
         target: &TargetTriple,
-    ) -> Result<Vec<String>> {
+    ) -> Result<Vec<Component>> {
+        // An older manifest with no profiles section.
+        if self.profiles.is_empty() {
+            return self.get_legacy_components(target);
+        }
+
         let profile = self
             .profiles
             .get(&profile)
@@ -214,8 +244,13 @@ impl Manifest {
         let rust_pkg = self.get_package("rust")?.get_target(Some(target))?;
         let result = profile
             .into_iter()
-            .filter(|s| rust_pkg.components.iter().any(|c| &c.pkg == *s))
-            .map(|s| s.to_owned())
+            .filter(|s| {
+                rust_pkg
+                    .components
+                    .iter()
+                    .any(|c| &c.pkg == *s && c.target.as_ref().map(|t| t == target).unwrap_or(true))
+            })
+            .map(|s| Component::new(s.to_owned(), Some(target.clone()), false))
             .collect();
         Ok(result)
     }
@@ -357,10 +392,11 @@ impl TargetedPackage {
         let extensions = get_array(&mut table, "extensions", path)?;
 
         let mut components =
-            Self::toml_to_components(components, &format!("{}{}.", path, "components"))?;
+            Self::toml_to_components(components, &format!("{}{}.", path, "components"), false)?;
         components.append(&mut Self::toml_to_components(
             extensions,
             &format!("{}{}.", path, "extensions"),
+            true,
         )?);
 
         if get_bool(&mut table, "available", path)? {
@@ -381,11 +417,14 @@ impl TargetedPackage {
         }
     }
     pub fn into_toml(self) -> toml::value::Table {
-        let components = Self::components_to_toml(self.components);
         let mut result = toml::value::Table::new();
+        let (components, extensions) = Self::components_to_toml(self.components);
         if !components.is_empty() {
             result.insert("components".to_owned(), toml::Value::Array(components));
         }
+        if !extensions.is_empty() {
+            result.insert("extensions".to_owned(), toml::Value::Array(extensions));
+        }
         if let Some(bins) = self.bins.clone() {
             result.insert("hash".to_owned(), toml::Value::String(bins.hash));
             result.insert("url".to_owned(), toml::Value::String(bins.url));
@@ -404,38 +443,56 @@ impl TargetedPackage {
         self.bins.is_some()
     }
 
-    fn toml_to_components(arr: toml::value::Array, path: &str) -> Result<Vec<Component>> {
+    fn toml_to_components(
+        arr: toml::value::Array,
+        path: &str,
+        is_extension: bool,
+    ) -> Result<Vec<Component>> {
         let mut result = Vec::new();
 
         for (i, v) in arr.into_iter().enumerate() {
             if let toml::Value::Table(t) = v {
                 let path = format!("{}[{}]", path, i);
-                result.push(Component::from_toml(t, &path)?);
+                result.push(Component::from_toml(t, &path, is_extension)?);
             }
         }
 
         Ok(result)
     }
-    fn components_to_toml(components: Vec<Component>) -> toml::value::Array {
-        let mut result = toml::value::Array::new();
-        for v in components {
-            result.push(toml::Value::Table(v.into_toml()));
+    fn components_to_toml(data: Vec<Component>) -> (toml::value::Array, toml::value::Array) {
+        let mut components = toml::value::Array::new();
+        let mut extensions = toml::value::Array::new();
+        for v in data {
+            if v.is_extension {
+                extensions.push(toml::Value::Table(v.into_toml()));
+            } else {
+                components.push(toml::Value::Table(v.into_toml()));
+            }
         }
-        result
+        (components, extensions)
     }
 }
 
 impl Component {
-    pub fn new(pkg: String, target: Option<TargetTriple>) -> Component {
-        Component { pkg, target }
+    pub fn new(pkg: String, target: Option<TargetTriple>, is_extension: bool) -> Component {
+        Component {
+            pkg,
+            target,
+            is_extension,
+        }
     }
     pub fn wildcard(&self) -> Component {
         Component {
             pkg: self.pkg.clone(),
             target: None,
+            is_extension: false,
         }
     }
-    pub fn from_toml(mut table: toml::value::Table, path: &str) -> Result<Self> {
+    pub fn from_toml(
+        mut table: toml::value::Table,
+        path: &str,
+        is_extension: bool,
+    ) -> Result<Self> {
         Ok(Component {
             pkg: get_string(&mut table, "pkg", path)?,
             target: get_string(&mut table, "target", path).map(|s| {
@@ -445,6 +502,7 @@ impl Component {
                     Some(TargetTriple::new(&s))
                 }
             })?,
+            is_extension,
         })
     }
     pub fn into_toml(self) -> toml::value::Table {
diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs
@@ -124,8 +124,8 @@ impl Manifestation {
         let update = Update::build_update(self, new_manifest, &changes, &config, notify_handler)?;
 
         if update.nothing_changes() {
-            // TODO changes and ,manifest are empty?
-            eprintln!("update: {:?}", update);
+            // TODO
+            eprintln!("no changes: {:?} {:?}", update, config);
             return Ok(UpdateStatus::Unchanged);
         }
 
@@ -488,10 +488,16 @@ impl Update {
         changes.check_invariants(&config);
 
         // The list of components already installed, empty if a new install
-        let starting_list = config
-            .as_ref()
-            .map(|c| c.components.clone())
-            .unwrap_or_default();
+        let installed_components = manifestation.installation.list()?;
+        let looks_like_v1 = config.is_none() && !installed_components.is_empty();
+        let starting_list = if looks_like_v1 {
+            new_manifest.get_legacy_components(&manifestation.target_triple)?
+        } else {
+            config
+                .as_ref()
+                .map(|c| c.components.clone())
+                .unwrap_or_default()
+        };
 
         let mut result = Update {
             components_to_uninstall: vec![],
diff --git a/tests/cli-v2.rs b/tests/cli-v2.rs
@@ -473,6 +473,7 @@ fn upgrade_v1_to_v2() {
         // Delete the v2 manifest so the first day we install from the v1s
         fs::remove_file(config.distdir.join("dist/channel-rust-nightly.toml.sha256")).unwrap();
         expect_ok(config, &["rustup", "default", "nightly"]);
+        expect_stdout_ok(config, &["rustc", "--version"], "hash-n-1");
         set_current_dist_date(config, "2015-01-02");
         expect_ok(config, &["rustup", "update", "nightly", "--no-self-update"]);
         expect_stdout_ok(config, &["rustc", "--version"], "hash-n-2");
diff --git a/tests/dist.rs b/tests/dist.rs
diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs
diff --git a/tests/mock/dist.rs b/tests/mock/dist.rs

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ impl Config {`
`67`	`67`	`for (i, v) in arr.into_iter().enumerate() {`
`68`	`68`	`if let toml::Value::Table(t) = v {`
`69`	`69`	`let path = format!("{}[{}]", path, i);`
`70`		`- result.push(Component::from_toml(t, &path)?);`
	`70`	`+ result.push(Component::from_toml(t, &path, false)?);`
`71`	`71`	`}`
`72`	`72`	`}`
`73`	`73`