Merge pull request #2002 from jonhoo/skip-missing

Correctly handle update across missing nightlies

src/dist/dist.rs

@@ -564,6 +564,7 @@ pub fn update_from_dist<'a>(
     profile: Option<Profile>,
     prefix: &InstallPrefix,
     force_update: bool,
+    old_date: Option<&str>,
 ) -> Result<Option<String>> {
     let fresh_install = !prefix.path().exists();
     let hash_exists = update_hash.map(Path::exists).unwrap_or(false);
@@ -582,6 +583,7 @@ pub fn update_from_dist<'a>(
         profile,
         prefix,
         force_update,
+        old_date,
     );
 
     // Don't leave behind an empty / broken installation directory
@@ -600,11 +602,29 @@ fn update_from_dist_<'a>(
     profile: Option<Profile>,
     prefix: &InstallPrefix,
     force_update: bool,
+    old_date: Option<&str>,
 ) -> Result<Option<String>> {
     let mut toolchain = toolchain.clone();
     let mut fetched = String::new();
     let mut first_err = None;
     let backtrack = toolchain.channel == "nightly" && toolchain.date.is_none();
+
+    // We never want to backtrack further back than the nightly that's already installed.
+    //
+    // If no nightly is installed, it makes no sense to backtrack beyond the first ever manifest,
+    // which is 2014-12-20 according to
+    // https://static.rust-lang.org/cargo-dist/index.html.
+    //
+    // We could arguably use the date of the first rustup release here, but that would break a
+    // bunch of the tests, which (inexplicably) use 2015-01-01 as their manifest dates.
+    let first_manifest = old_date
+        .map(|date| {
+            Utc.from_utc_date(
+                &NaiveDate::parse_from_str(date, "%Y-%m-%d").expect("Malformed manifest date"),
+            )
+        })
+        .unwrap_or_else(|| Utc.from_utc_date(&NaiveDate::from_ymd(2014, 12, 20)));
+
     loop {
         match try_update_from_dist_(
             download,
@@ -629,6 +649,9 @@ fn update_from_dist_<'a>(
                     if first_err.is_none() {
                         first_err = Some(e);
                     }
+                } else if let ErrorKind::MissingReleaseForToolchain(..) = e.kind() {
+                    // no need to even print anything for missing nightlies,
+                    // since we don't really "skip" them
                 } else if let Some(e) = first_err {
                     // if we fail to find a suitable nightly, we abort the search and give the
                     // original "components unavailable for download" error.
@@ -641,23 +664,28 @@ fn update_from_dist_<'a>(
                 // the components that the user currently has installed. Let's try the previous
                 // nightlies in reverse chronological order until we find a nightly that does,
                 // starting at one date earlier than the current manifest's date.
-                //
-                // NOTE: we don't need to explicitly check for the case where the next nightly to
-                // try is _older_ than the current nightly, since we know that the user's current
-                // nightlys supports the components they have installed, and thus would always
-                // terminate the search.
-                toolchain.date = Some(
-                    Utc.from_utc_date(
+                let try_next = Utc
+                    .from_utc_date(
                         &NaiveDate::parse_from_str(
                             toolchain.date.as_ref().unwrap_or(&fetched),
                             "%Y-%m-%d",
                         )
                         .expect("Malformed manifest date"),
                     )
-                    .pred()
-                    .format("%Y-%m-%d")
-                    .to_string(),
-                );
+                    .pred();
+
+                if try_next < first_manifest {
+                    // Wouldn't be an update if we go further back than the user's current nightly.
+                    if let Some(e) = first_err {
+                        break Err(e);
+                    } else {
+                        // In this case, all newer nightlies are missing, which means there are no
+                        // updates, so the user is already at the latest nightly.
+                        break Ok(None);
+                    }
+                }
+
+                toolchain.date = Some(try_next.format("%Y-%m-%d").to_string());
             }
         }
     }
@@ -721,7 +749,9 @@ fn try_update_from_dist_<'a>(
     let manifest = match dl_v1_manifest(download, toolchain) {
         Ok(m) => m,
         Err(Error(crate::ErrorKind::DownloadNotExists { .. }, _)) => {
-            return Err(format!("no release found for '{}'", toolchain.manifest_name()).into());
+            return Err(Error::from(ErrorKind::MissingReleaseForToolchain(
+                toolchain.manifest_name(),
+            )));
         }
         Err(e @ Error(ErrorKind::ChecksumFailed { .. }, _)) => {
             return Err(e);

src/errors.rs

@@ -275,6 +275,10 @@ error_chain! {
             description("missing package for the target of a rename")
             display("server sent a broken manifest: missing package for the target of a rename {}", name)
         }
+        MissingReleaseForToolchain(name: String) {
+            description("missing release for a toolchain")
+            display("no release found for '{}'", name)
+        }
         RequestedComponentsUnavailable(c: Vec<Component>, manifest: Manifest, toolchain: String) {
             description("some requested components are unavailable to download")
             display("{}", component_unavailable_msg(&c, &manifest, &toolchain))

src/install.rs

@@ -26,6 +26,8 @@ pub enum InstallMethod<'a> {
         bool,
         // toolchain already exists
         bool,
+        // currently installed date
+        Option<&'a str>,
     ),
 }
 
@@ -54,7 +56,15 @@ impl<'a> InstallMethod<'a> {
                 InstallMethod::tar_gz(src, path, &temp_cfg, notify_handler)?;
                 Ok(true)
             }
-            InstallMethod::Dist(toolchain, profile, update_hash, dl_cfg, force_update, exists) => {
+            InstallMethod::Dist(
+                toolchain,
+                profile,
+                update_hash,
+                dl_cfg,
+                force_update,
+                exists,
+                old_date,
+            ) => {
                 let prefix = &InstallPrefix::from(path.to_owned());
                 let maybe_new_hash = dist::update_from_dist(
                     dl_cfg,
@@ -63,6 +73,7 @@ impl<'a> InstallMethod<'a> {
                     if exists { None } else { profile },
                     prefix,
                     force_update,
+                    old_date,
                 )?;
 
                 if let Some(hash) = maybe_new_hash {

src/toolchain.rs

@@ -164,13 +164,15 @@ impl<'a> Toolchain<'a> {
 
     pub fn install_from_dist(&self, force_update: bool) -> Result<UpdateStatus> {
         let update_hash = self.update_hash()?;
+        let old_date = self.get_manifest().ok().and_then(|m| m.map(|m| m.date));
         self.install(InstallMethod::Dist(
             &self.desc()?,
             self.cfg.get_profile()?,
             update_hash.as_ref().map(|p| &**p),
             self.download_cfg(),
             force_update,
             self.exists(),
+            old_date.as_ref().map(|s| &**s),
         ))
     }
 
@@ -183,6 +185,7 @@ impl<'a> Toolchain<'a> {
             self.download_cfg(),
             false,
             false,
+            None,
         ))
     }
     pub fn is_custom(&self) -> bool {
@@ -470,7 +473,7 @@ impl<'a> Toolchain<'a> {
         })
     }
 
-    pub fn show_version(&self) -> Result<Option<String>> {
+    pub fn get_manifest(&self) -> Result<Option<Manifest>> {
         if !self.exists() {
             return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
         }
@@ -481,7 +484,11 @@ impl<'a> Toolchain<'a> {
         let prefix = InstallPrefix::from(self.path.to_owned());
         let manifestation = Manifestation::open(prefix, toolchain.target.clone())?;
 
-        match manifestation.load_manifest()? {
+        manifestation.load_manifest()
+    }
+
+    pub fn show_version(&self) -> Result<Option<String>> {
+        match self.get_manifest()? {
             Some(manifest) => Ok(Some(manifest.get_rust_version()?.to_string())),
             None => Ok(None),
         }

tests/cli-misc.rs

@@ -771,14 +771,16 @@ fn update_nightly_even_with_incompat() {
 
 #[test]
 fn nightly_backtrack_skips_missing() {
-    clitools::setup(Scenario::Unavailable, &|config| {
-        set_current_dist_date(config, "2015-01-01");
+    clitools::setup(Scenario::MissingNightly, &|config| {
+        set_current_dist_date(config, "2019-09-16");
         expect_ok(config, &["rustup", "default", "nightly"]);
 
         expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-1");
+        expect_ok(config, &["rustup", "component", "add", "rls"]);
+        expect_component_executable(config, "rls");
 
-        // nightly is missing on latest
-        set_current_dist_date(config, "2015-01-02");
+        // rls is missing on latest, nightly is missing on second-to-latest
+        set_current_dist_date(config, "2019-09-18");
 
         // update should not change nightly, and should not error
         expect_ok(config, &["rustup", "update", "nightly", "--no-self-update"]);

tests/mock/clitools.rs

@@ -52,6 +52,7 @@ pub enum Scenario {
     Unavailable,      // Two dates, v2 manifests, everything unavailable in second date.
     UnavailableRls,   // Two dates, v2 manifests, RLS unavailable in first date, restored on second.
     MissingComponent, // Three dates, v2 manifests, RLS available in first and last, not middle
+    MissingNightly,   // Three dates, v2 manifests, RLS available in first, middle missing nightly
 }
 
 pub static CROSS_ARCH1: &str = "x86_64-unknown-linux-musl";
@@ -564,6 +565,11 @@ fn create_mock_dist_server(path: &Path, s: Scenario) {
             Release::new("nightly", "1.37.0", "2019-09-13", "2"),
             Release::new("nightly", "1.37.0", "2019-09-14", "3").with_rls(RlsStatus::Unavailable),
         ],
+        Scenario::MissingNightly => vec![
+            Release::new("nightly", "1.37.0", "2019-09-16", "1"),
+            Release::stable("1.37.0", "2019-09-17"),
+            Release::new("nightly", "1.37.0", "2019-09-18", "2").with_rls(RlsStatus::Unavailable),
+        ],
         Scenario::Unavailable => vec![
             Release::new("nightly", "1.2.0", "2015-01-01", "1"),
             Release::beta("1.1.0", "2015-01-01"),
@@ -606,6 +612,7 @@ fn create_mock_dist_server(path: &Path, s: Scenario) {
         | Scenario::MultiHost
         | Scenario::Unavailable
         | Scenario::UnavailableRls
+        | Scenario::MissingNightly
         | Scenario::MissingComponent => vec![ManifestVersion::V2],
     };