Skip to content
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.

Sign up for GitHub

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

Remove components if they don't exist anymore during an update #1419

Merged
merged 3 commits into from
May 27, 2018
Merged

Remove components if they don't exist anymore during an update #1419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c8457c
Remove components if they don't exist anymore during update
pietroalbini May 27, 2018
58db76a
Allow editing all mock packages during dist tests
pietroalbini May 27, 2018
bd5ac16
Update tests
pietroalbini May 27, 2018
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
11 changes: 10 additions & 1 deletion src/rustup-dist/src/manifestation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
));
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/rustup-dist/src/notifications.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<rustup_utils::Notification<'a>> for Notification<'a> {
Expand Down Expand Up @@ -68,7 +69,8 @@ impl<'a> Notification<'a> {
CantReadUpdateHash(_)
| ExtensionNotInstalled(_)
| MissingInstalledComponent(_)
| CachedFileChecksumFailed => NotificationLevel::Warn,
| CachedFileChecksumFailed
| ComponentUnavailable(_, _) => NotificationLevel::Warn,
NonFatalError(_) => NotificationLevel::Error,
}
}
Expand Down Expand Up @@ -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)
}
}
}
}
}
120 changes: 84 additions & 36 deletions src/rustup-dist/tests/dist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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),
) {
Expand Down Expand Up @@ -645,11 +645,12 @@ 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| {
if date == "2016-02-01" {
let mut tpkg = pkg.targets
fn unavailable_component() {
// On day 2 the bonus component is no longer available
let edit = &|date: &str, pkgs: &mut [MockPackage]| {
// Require the bonus component every dat
{
let tpkg = pkgs[0].targets
.iter_mut()
.find(|p| p.target == "x86_64-apple-darwin")
.unwrap();
Expand All @@ -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(
Expand All @@ -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());
},
);
}
Expand Down Expand Up @@ -735,9 +783,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();
Expand All @@ -747,7 +795,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();
Expand Down Expand Up @@ -782,9 +830,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();
Expand All @@ -794,7 +842,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();
Expand Down Expand Up @@ -1127,9 +1175,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();
Expand Down