From 9d20c5e1dd81995aa878024202a32fa7136ccfad Mon Sep 17 00:00:00 2001 From: Ivan Nejgebauer Date: Tue, 2 Aug 2016 08:55:07 +0200 Subject: [PATCH 1/3] Remove empty dirs after component uninstall --- src/rustup-dist/src/component/components.rs | 106 +++++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/src/rustup-dist/src/component/components.rs b/src/rustup-dist/src/component/components.rs index c8b53658fb..a54ce6da97 100644 --- a/src/rustup-dist/src/component/components.rs +++ b/src/rustup-dist/src/component/components.rs @@ -184,14 +184,116 @@ impl Component { // TODO: If this is the last component remove the components file // and the version file. + // Track visited directories + use std::collections::HashSet; + use std::collections::hash_set::IntoIter; + use std::fs::read_dir; + + // dirs will contain the set of longest disjoint directory paths seen + // ancestors help in filtering seen paths and constructing dirs + // All seen paths must be relative to avoid surprises + struct PruneSet { + dirs: HashSet, + ancestors: HashSet, + prefix: PathBuf, + } + + impl PruneSet { + fn seen(&mut self, mut path: PathBuf) { + if !path.is_relative() || !path.pop() { + return; + } + if self.dirs.contains(&path) || self.ancestors.contains(&path) { + return; + } + self.dirs.insert(path.clone()); + while path.pop() { + if path.file_name().is_none() { + break; + } + if self.dirs.contains(&path) { + self.dirs.remove(&path); + } + if self.ancestors.contains(&path) { + break; + } + self.ancestors.insert(path.clone()); + } + } + } + + struct PruneIter { + iter: IntoIter, + path_buf: Option, + prefix: PathBuf, + } + + impl IntoIterator for PruneSet { + type Item = PathBuf; + type IntoIter = PruneIter; + + fn into_iter(self) -> Self::IntoIter { + PruneIter { + iter: self.dirs.into_iter(), + path_buf: None, + prefix: self.prefix, + } + } + } + + // Returns only empty directories + impl Iterator for PruneIter { + type Item = PathBuf; + + fn next(&mut self) -> Option { + self.path_buf = match self.path_buf { + None => self.iter.next(), + Some(_) => { + let mut path_buf = self.path_buf.take().unwrap(); + match path_buf.file_name() { + Some(_) => if path_buf.pop() { Some(path_buf) } else { None }, + None => self.iter.next(), + } + }, + }; + if self.path_buf.is_none() { + return None; + } + let full_path = self.prefix.join(self.path_buf.as_ref().unwrap()); + let empty = match read_dir(full_path) { + Ok(dir) => dir.count() == 0, + Err(_) => false, + }; + if empty { + self.path_buf.clone() + } else { + self.next() + } + } + } + // Remove parts + let mut pset = PruneSet { + dirs: HashSet::new(), + ancestors: HashSet::new(), + prefix: self.components.prefix.abs_path(""), + }; for part in try!(self.parts()).into_iter().rev() { match &*part.0 { - "file" => try!(tx.remove_file(&self.name, part.1)), - "dir" => try!(tx.remove_dir(&self.name, part.1)), + "file" => { + try!(tx.remove_file(&self.name, part.1.clone())); + pset.seen(part.1); + }, + "dir" => { + try!(tx.remove_dir(&self.name, part.1.clone())); + pset.seen(part.1); + }, _ => return Err(ErrorKind::CorruptComponent(self.name.clone()).into()), } } + for empty_dir in pset { + try!(tx.remove_dir(&self.name, empty_dir)); + } // Remove component manifest try!(tx.remove_file(&self.name, self.rel_manifest_file())); From 082f63d5513e7fdb7bc32434aa2f2a30c495ca9c Mon Sep 17 00:00:00 2001 From: Ivan Nejgebauer Date: Tue, 2 Aug 2016 08:57:04 +0200 Subject: [PATCH 2/3] Test for empty dir removal --- tests/cli-v2.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/cli-v2.rs b/tests/cli-v2.rs index 3bb85910b9..98aa1f1bd7 100644 --- a/tests/cli-v2.rs +++ b/tests/cli-v2.rs @@ -551,6 +551,12 @@ fn remove_target() { let path = format!("toolchains/nightly-{}/lib/rustlib/{}/lib/libstd.rlib", this_host_triple(), clitools::CROSS_ARCH1); assert!(!config.rustupdir.join(path).exists()); + let path = format!("toolchains/nightly-{}/lib/rustlib/{}/lib", + this_host_triple(), clitools::CROSS_ARCH1); + assert!(!config.rustupdir.join(path).exists()); + let path = format!("toolchains/nightly-{}/lib/rustlib/{}", + this_host_triple(), clitools::CROSS_ARCH1); + assert!(!config.rustupdir.join(path).exists()); }); } From dd95c0184691646fb493581cad2a14031c5cc87b Mon Sep 17 00:00:00 2001 From: Ivan Nejgebauer Date: Sun, 7 Aug 2016 21:46:22 +0200 Subject: [PATCH 3/3] Misc small cleanups in component uninstall * Fix whitespace (no tabs) * Short-circuit the iterator on non-empty dir * Avoid repetition in the part loop --- src/rustup-dist/src/component/components.rs | 31 ++++++++++----------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/rustup-dist/src/component/components.rs b/src/rustup-dist/src/component/components.rs index a54ce6da97..9185f6e0c0 100644 --- a/src/rustup-dist/src/component/components.rs +++ b/src/rustup-dist/src/component/components.rs @@ -209,15 +209,15 @@ impl Component { self.dirs.insert(path.clone()); while path.pop() { if path.file_name().is_none() { - break; - } - if self.dirs.contains(&path) { - self.dirs.remove(&path); - } - if self.ancestors.contains(&path) { - break; - } - self.ancestors.insert(path.clone()); + break; + } + if self.dirs.contains(&path) { + self.dirs.remove(&path); + } + if self.ancestors.contains(&path) { + break; + } + self.ancestors.insert(path.clone()); } } } @@ -267,6 +267,8 @@ impl Component { if empty { self.path_buf.clone() } else { + // No dir above can be empty, go to next path in dirs + self.path_buf = None; self.next() } } @@ -280,16 +282,11 @@ impl Component { }; for part in try!(self.parts()).into_iter().rev() { match &*part.0 { - "file" => { - try!(tx.remove_file(&self.name, part.1.clone())); - pset.seen(part.1); - }, - "dir" => { - try!(tx.remove_dir(&self.name, part.1.clone())); - pset.seen(part.1); - }, + "file" => try!(tx.remove_file(&self.name, part.1.clone())), + "dir" => try!(tx.remove_dir(&self.name, part.1.clone())), _ => return Err(ErrorKind::CorruptComponent(self.name.clone()).into()), } + pset.seen(part.1); } for empty_dir in pset { try!(tx.remove_dir(&self.name, empty_dir));