From 3c8457c0fe600129997002fcba48f2526e513d5c Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 27 May 2018 12:39:43 +0200 Subject: [PATCH 1/3] Remove components if they don't exist anymore during update --- src/rustup-dist/src/manifestation.rs | 11 ++++++++++- src/rustup-dist/src/notifications.rs | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/rustup-dist/src/manifestation.rs b/src/rustup-dist/src/manifestation.rs index b1618d43d6..89286a5573 100644 --- a/src/rustup-dist/src/manifestation.rs +++ b/src/rustup-dist/src/manifestation.rs @@ -475,6 +475,7 @@ impl Update { rust_target_package, new_manifest, &changes, + notify_handler, ); // If this is a full upgrade then the list of components to @@ -520,6 +521,7 @@ impl Update { rust_target_package: &TargetedPackage, new_manifest: &Manifest, changes: &Changes, + notify_handler: &Fn(Notification), ) { // Add components required by the package, according to the // manifest @@ -559,7 +561,14 @@ impl Update { if component_is_present { self.final_component_list.push(existing_component.clone()); } else { - self.missing_components.push(existing_component.clone()); + // If a component is not available anymore for the target remove it + // This prevents errors when trying to update to a newer version with + // a removed component. + self.components_to_uninstall.push(existing_component.clone()); + notify_handler(Notification::ComponentUnavailable( + &existing_component.pkg, + existing_component.target.as_ref(), + )); } } } diff --git a/src/rustup-dist/src/notifications.rs b/src/rustup-dist/src/notifications.rs index 53470f1449..d138ed8abb 100644 --- a/src/rustup-dist/src/notifications.rs +++ b/src/rustup-dist/src/notifications.rs @@ -31,6 +31,7 @@ pub enum Notification<'a> { DownloadedManifest(&'a str, Option<&'a str>), DownloadingLegacyManifest, ManifestChecksumFailedHack, + ComponentUnavailable(&'a str, Option<&'a TargetTriple>), } impl<'a> From> for Notification<'a> { @@ -68,7 +69,8 @@ impl<'a> Notification<'a> { CantReadUpdateHash(_) | ExtensionNotInstalled(_) | MissingInstalledComponent(_) - | CachedFileChecksumFailed => NotificationLevel::Warn, + | CachedFileChecksumFailed + | ComponentUnavailable(_, _) => NotificationLevel::Warn, NonFatalError(_) => NotificationLevel::Error, } } @@ -132,6 +134,13 @@ impl<'a> Display for Notification<'a> { ManifestChecksumFailedHack => { write!(f, "update not yet available, sorry! try again later") } + ComponentUnavailable(pkg, toolchain) => { + if let Some(tc) = toolchain { + write!(f, "component '{}' is not available anymore on target '{}'", pkg, tc) + } else { + write!(f, "component '{}' is not available anymore", pkg) + } + } } } } From 58db76a62d668aeefec4603a754385f7f8b76464 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 27 May 2018 16:03:12 +0200 Subject: [PATCH 2/3] Allow editing all mock packages during dist tests --- src/rustup-dist/tests/dist.rs | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/rustup-dist/tests/dist.rs b/src/rustup-dist/tests/dist.rs index 55a112f9ef..c1df0adf4e 100644 --- a/src/rustup-dist/tests/dist.rs +++ b/src/rustup-dist/tests/dist.rs @@ -37,7 +37,7 @@ use tempdir::TempDir; // Creates a mock dist server populated with some test data pub fn create_mock_dist_server( path: &Path, - edit: Option<&Fn(&str, &mut MockPackage)>, + edit: Option<&Fn(&str, &mut [MockPackage])>, ) -> MockDistServer { MockDistServer { path: path.to_owned(), @@ -51,7 +51,7 @@ pub fn create_mock_dist_server( pub fn create_mock_channel( channel: &str, date: &str, - edit: Option<&Fn(&str, &mut MockPackage)>, + edit: Option<&Fn(&str, &mut [MockPackage])>, ) -> MockChannel { // Put the date in the files so they can be differentiated let contents = Arc::new(date.as_bytes().to_vec()); @@ -209,7 +209,7 @@ pub fn create_mock_channel( packages.push(bonus_component("bonus", contents.clone())); if let Some(edit) = edit { - edit(date, &mut packages[0]); + edit(date, &mut packages); } MockChannel { @@ -289,8 +289,8 @@ fn rename_component() { let dist_tempdir = TempDir::new("rustup").unwrap(); let ref url = Url::parse(&format!("file://{}", dist_tempdir.path().to_string_lossy())).unwrap(); - let edit_1 = &|_: &str, pkg: &mut MockPackage| { - let tpkg = pkg.targets + let edit_1 = &|_: &str, pkgs: &mut [MockPackage]| { + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -299,8 +299,8 @@ fn rename_component() { target: "x86_64-apple-darwin".to_string(), }); }; - let edit_2 = &|_: &str, pkg: &mut MockPackage| { - let tpkg = pkg.targets + let edit_2 = &|_: &str, pkgs: &mut [MockPackage]| { + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -347,8 +347,8 @@ fn rename_component_ignore() { let dist_tempdir = TempDir::new("rustup").unwrap(); let ref url = Url::parse(&format!("file://{}", dist_tempdir.path().to_string_lossy())).unwrap(); - let edit = &|_: &str, pkg: &mut MockPackage| { - let tpkg = pkg.targets + let edit = &|_: &str, pkgs: &mut [MockPackage]| { + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -395,8 +395,8 @@ fn rename_component_new() { let dist_tempdir = TempDir::new("rustup").unwrap(); let ref url = Url::parse(&format!("file://{}", dist_tempdir.path().to_string_lossy())).unwrap(); - let edit_2 = &|_: &str, pkg: &mut MockPackage| { - let tpkg = pkg.targets + let edit_2 = &|_: &str, pkgs: &mut [MockPackage]| { + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -526,7 +526,7 @@ fn uninstall( } fn setup( - edit: Option<&Fn(&str, &mut MockPackage)>, + edit: Option<&Fn(&str, &mut [MockPackage])>, enable_xz: bool, f: &Fn(&Url, &ToolchainDesc, &InstallPrefix, &DownloadCfg, &temp::Cfg), ) { @@ -647,9 +647,9 @@ fn upgrade() { #[test] fn force_update() { // On day 1 install the 'bonus' component, on day 2 its no longer a component - let edit = &|date: &str, pkg: &mut MockPackage| { + let edit = &|date: &str, pkgs: &mut [MockPackage]| { if date == "2016-02-01" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -735,9 +735,9 @@ fn update_preserves_extensions() { #[test] fn update_preserves_extensions_that_became_components() { - let edit = &|date: &str, pkg: &mut MockPackage| { + let edit = &|date: &str, pkgs: &mut [MockPackage]| { if date == "2016-02-01" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -747,7 +747,7 @@ fn update_preserves_extensions_that_became_components() { }); } if date == "2016-02-02" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -782,9 +782,9 @@ fn update_preserves_extensions_that_became_components() { #[test] fn update_preserves_components_that_became_extensions() { - let edit = &|date: &str, pkg: &mut MockPackage| { + let edit = &|date: &str, pkgs: &mut [MockPackage]| { if date == "2016-02-01" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -794,7 +794,7 @@ fn update_preserves_components_that_became_extensions() { }); } if date == "2016-02-02" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); @@ -1127,9 +1127,9 @@ fn remove_extension_not_in_manifest() { #[test] #[should_panic] fn remove_extension_not_in_manifest_but_is_already_installed() { - let edit = &|date: &str, pkg: &mut MockPackage| { + let edit = &|date: &str, pkgs: &mut [MockPackage]| { if date == "2016-02-01" { - let mut tpkg = pkg.targets + let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") .unwrap(); From bd5ac16e6d3a2274d55d3b6ca07fc37037d86571 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 27 May 2018 16:22:55 +0200 Subject: [PATCH 3/3] Update tests --- src/rustup-dist/tests/dist.rs | 76 ++++++++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/src/rustup-dist/tests/dist.rs b/src/rustup-dist/tests/dist.rs index c1df0adf4e..3e5e45bcfc 100644 --- a/src/rustup-dist/tests/dist.rs +++ b/src/rustup-dist/tests/dist.rs @@ -645,10 +645,11 @@ fn upgrade() { } #[test] -fn force_update() { - // On day 1 install the 'bonus' component, on day 2 its no longer a component +fn unavailable_component() { + // On day 2 the bonus component is no longer available let edit = &|date: &str, pkgs: &mut [MockPackage]| { - if date == "2016-02-01" { + // Require the bonus component every dat + { let tpkg = pkgs[0].targets .iter_mut() .find(|p| p.target == "x86_64-apple-darwin") @@ -658,6 +659,17 @@ fn force_update() { target: "x86_64-apple-darwin".to_string(), }); } + + // Mark the bonus package as unavailable in 2016-02-02 + if date == "2016-02-02" { + let bonus_pkg = pkgs.iter_mut() + .find(|p| p.name == "bonus") + .unwrap(); + + for target in &mut bonus_pkg.targets { + target.available = false; + } + } }; setup( @@ -677,18 +689,54 @@ fn force_update() { ErrorKind::RequestedComponentsUnavailable(..) => {} _ => panic!(), } - // Force update without bonus, should succeed, but bonus binary will be missing. - update_from_dist_( - url, - toolchain, - prefix, - &[], - &[], - download_cfg, - temp_cfg, - true, - ).unwrap(); + }, + ); +} + +#[test] +fn removed_component() { + // On day 1 install the 'bonus' component, on day 2 its no longer a component + let edit = &|date: &str, pkgs: &mut [MockPackage]| { + if date == "2016-02-01" { + let tpkg = pkgs[0].targets + .iter_mut() + .find(|p| p.target == "x86_64-apple-darwin") + .unwrap(); + tpkg.components.push(MockComponent { + name: "bonus".to_string(), + target: "x86_64-apple-darwin".to_string(), + }); + } + }; + + setup( + Some(edit), + false, + &|url, toolchain, prefix, download_cfg, temp_cfg| { + let received_notification = Arc::new(Cell::new(false)); + + let download_cfg = DownloadCfg { + dist_root: download_cfg.dist_root, + temp_cfg: download_cfg.temp_cfg, + download_dir: download_cfg.download_dir, + notify_handler: &|n| { + if let Notification::ComponentUnavailable("bonus", Some(_)) = n { + received_notification.set(true); + } + }, + }; + + change_channel_date(url, "nightly", "2016-02-01"); + // Update with bonus. + update_from_dist(url, toolchain, prefix, &[], &[], &download_cfg, temp_cfg).unwrap(); + assert!(utils::path_exists(&prefix.path().join("bin/bonus"))); + change_channel_date(url, "nightly", "2016-02-02"); + + // Update without bonus, should emit a notify and remove the bonus component + update_from_dist(url, toolchain, prefix, &[], &[], &download_cfg, temp_cfg).unwrap(); assert!(!utils::path_exists(&prefix.path().join("bin/bonus"))); + + assert!(received_notification.get()); }, ); }