From 0fe48c3c21bb0def214a72758a91cd7747aaea1e Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Mon, 4 Feb 2019 14:41:20 +0100 Subject: [PATCH] Various cosmetic improvements. --- src/combiner.rs | 41 +++++++++++++++++++++-------------------- src/generator.rs | 2 +- src/remove_dir_all.rs | 2 +- src/scripter.rs | 6 +++--- src/tarballer.rs | 22 +++++++++++----------- src/util.rs | 22 +++++++++++----------- 6 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/combiner.rs b/src/combiner.rs index faab1c4..2d22992 100644 --- a/src/combiner.rs +++ b/src/combiner.rs @@ -1,47 +1,48 @@ use std::io::{Read, Write}; use std::path::Path; + use flate2::read::GzDecoder; use tar::Archive; use crate::errors::*; +use crate::util::*; use super::Scripter; use super::Tarballer; -use crate::util::*; actor!{ #[derive(Debug)] pub struct Combiner { - /// The name of the product, for display + /// The name of the product, for display. product_name: String = "Product", - /// The name of the package, tarball + /// The name of the package tarball. package_name: String = "package", - /// The directory under lib/ where the manifest lives + /// The directory under lib/ where the manifest lives. rel_manifest_dir: String = "packagelib", - /// The string to print after successful installation + /// The string to print after successful installation. success_message: String = "Installed.", - /// Places to look for legacy manifests to uninstall + /// Places to look for legacy manifests to uninstall. legacy_manifest_dirs: String = "", - /// Installers to combine + /// Installers to combine. input_tarballs: String = "", - /// Directory containing files that should not be installed + /// Directory containing files that should not be installed. non_installed_overlay: String = "", - /// The directory to do temporary work + /// The directory to do temporary work. work_dir: String = "./workdir", - /// The location to put the final image and tarball + /// The location to put the final image and tarball. output_dir: String = "./dist", } } impl Combiner { - /// Combine the installer tarballs + /// Combines the installer tarballs. pub fn run(self) -> Result<()> { create_dir_all(&self.work_dir)?; @@ -51,7 +52,7 @@ impl Combiner { } create_dir_all(&package_dir)?; - // Merge each installer into the work directory of the new installer + // Merge each installer into the work directory of the new installer. let components = create_new_file(package_dir.join("components"))?; for input_tarball in self.input_tarballs.split(',').map(str::trim).filter(|s| !s.is_empty()) { // Extract the input tarballs @@ -64,7 +65,7 @@ impl Combiner { let pkg_name = Path::new(pkg_name).file_name().unwrap(); let pkg_dir = Path::new(&self.work_dir).join(&pkg_name); - // Verify the version number + // Verify the version number. let mut version = String::new(); open_file(pkg_dir.join("rust-installer-version")) .and_then(|mut file| file.read_to_string(&mut version).map_err(Error::from)) @@ -73,37 +74,37 @@ impl Combiner { bail!("incorrect installer version in {}", input_tarball); } - // Copy components to the new combined installer + // Copy components to the new combined installer. let mut pkg_components = String::new(); open_file(pkg_dir.join("components")) .and_then(|mut file| file.read_to_string(&mut pkg_components).map_err(Error::from)) .chain_err(|| format!("failed to read components in '{}'", input_tarball))?; for component in pkg_components.split_whitespace() { - // All we need to do is copy the component directory. We could + // All we need to do is copy the component directory. We could // move it, but rustbuild wants to reuse the unpacked package // dir for OS-specific installers on macOS and Windows. let component_dir = package_dir.join(&component); create_dir(&component_dir)?; copy_recursive(&pkg_dir.join(&component), &component_dir)?; - // Merge the component name + // Merge the component name. writeln!(&components, "{}", component) .chain_err(|| "failed to write new components")?; } } drop(components); - // Write the installer version + // Write the installer version. let version = package_dir.join("rust-installer-version"); writeln!(create_new_file(version)?, "{}", crate::RUST_INSTALLER_VERSION) .chain_err(|| "failed to write new installer version")?; - // Copy the overlay + // Copy the overlay. if !self.non_installed_overlay.is_empty() { copy_recursive(self.non_installed_overlay.as_ref(), &package_dir)?; } - // Generate the install script + // Generate the install script. let output_script = package_dir.join("install.sh"); let mut scripter = Scripter::default(); scripter.product_name(self.product_name) @@ -113,7 +114,7 @@ impl Combiner { .output_script(path_to_str(&output_script)?); scripter.run()?; - // Make the tarballs + // Make the tarballs. create_dir_all(&self.output_dir)?; let output = Path::new(&self.output_dir).join(&self.package_name); let mut tarballer = Tarballer::default(); diff --git a/src/generator.rs b/src/generator.rs index e39a7ad..60e54f2 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -45,7 +45,7 @@ actor!{ } impl Generator { - /// Generate the actual installer tarball + /// Generates the actual installer tarball pub fn run(self) -> Result<()> { create_dir_all(&self.work_dir)?; diff --git a/src/remove_dir_all.rs b/src/remove_dir_all.rs index 61f64b0..70c16a2 100644 --- a/src/remove_dir_all.rs +++ b/src/remove_dir_all.rs @@ -57,7 +57,7 @@ mod win { // already need write permission in this dir to delete the directory. And it // should be on the same volume. // - // To handle files with names like `CON` and `morse .. .`, and when a + // To handle files with names like `CON` and `morse .. .`, and when a // directory structure is so deep it needs long path names the path is first // converted to a `//?/`-path with `get_path()`. // diff --git a/src/scripter.rs b/src/scripter.rs index 3991604..fae464c 100644 --- a/src/scripter.rs +++ b/src/scripter.rs @@ -27,14 +27,14 @@ actor!{ } impl Scripter { - /// Generate the actual installer script + /// Generates the actual installer script pub fn run(self) -> Result<()> { // Replace dashes in the success message with spaces (our arg handling botches spaces) - // (TODO: still needed? kept for compatibility for now...) + // TODO: still needed? Kept for compatibility for now. let product_name = self.product_name.replace('-', " "); // Replace dashes in the success message with spaces (our arg handling botches spaces) - // (TODO: still needed? kept for compatibility for now...) + // TODO: still needed? Kept for compatibility for now. let success_message = self.success_message.replace('-', " "); let script = TEMPLATE diff --git a/src/tarballer.rs b/src/tarballer.rs index ded46da..9dfaa1c 100644 --- a/src/tarballer.rs +++ b/src/tarballer.rs @@ -15,24 +15,24 @@ use crate::util::*; actor!{ #[derive(Debug)] pub struct Tarballer { - /// The input folder to be compressed + /// The input folder to be compressed. input: String = "package", - /// The prefix of the tarballs + /// The prefix of the tarballs. output: String = "./dist", - /// The folder in which the input is to be found + /// The folder in which the input is to be found. work_dir: String = "./workdir", } } impl Tarballer { - /// Generate the actual tarballs + /// Generates the actual tarballs pub fn run(self) -> Result<()> { let tar_gz = self.output.clone() + ".tar.gz"; let tar_xz = self.output.clone() + ".tar.xz"; - // Remove any existing files + // Remove any existing files. for file in &[&tar_gz, &tar_xz] { if Path::new(file).exists() { remove_file(file)?; @@ -46,14 +46,14 @@ impl Tarballer { .chain_err(|| "failed to collect file paths")?; files.sort_by(|a, b| a.bytes().rev().cmp(b.bytes().rev())); - // Prepare the .tar.gz file + // Prepare the `.tar.gz` file. let gz = GzEncoder::new(create_new_file(tar_gz)?, flate2::Compression::best()); - // Prepare the .tar.xz file + // Prepare the `.tar.xz` file. let xz = XzEncoder::new(create_new_file(tar_xz)?, 6); - // Write the tar into both encoded files. We write all directories - // first, so files may be directly created. (see rustup.rs#1092) + // Write the tar into both encoded files. We write all directories + // first, so files may be directly created. (See rust-lang/rustup.rs#1092.) let tee = RayonTee(xz, gz); let buf = BufWriter::with_capacity(1024 * 1024, tee); let mut builder = Builder::new(buf); @@ -74,7 +74,7 @@ impl Tarballer { .chain_err(|| "failed to finish writing .tar stream")? .into_inner().ok().unwrap(); - // Finish both encoded files + // Finish both encoded files. let (rxz, rgz) = rayon::join( || xz.finish().chain_err(|| "failed to finish .tar.xz file"), || gz.finish().chain_err(|| "failed to finish .tar.gz file"), @@ -110,7 +110,7 @@ fn append_path(builder: &mut Builder, src: &Path, path: &String) -> Ok(()) } -/// Returns all `(directories, files)` under the source path +/// Returns all `(directories, files)` under the source path. fn get_recursive_paths(root: P, name: Q) -> Result<(Vec, Vec)> where P: AsRef, Q: AsRef { diff --git a/src/util.rs b/src/util.rs index c7f8436..f499b89 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,7 +6,7 @@ use walkdir::WalkDir; // Needed to set the script mode to executable. #[cfg(unix)] use std::os::unix::fs::OpenOptionsExt; -// FIXME: what about Windows? Are default ACLs executable? +// FIXME: what about Windows? Are default ACLs executable? #[cfg(unix)] use std::os::unix::fs::symlink as symlink_file; @@ -15,14 +15,14 @@ use std::os::windows::fs::symlink_file; use crate::errors::*; -/// Convert a `&Path` to a UTF-8 `&str` +/// Converts a `&Path` to a UTF-8 `&str`. pub fn path_to_str(path: &Path) -> Result<&str> { path.to_str().ok_or_else(|| { ErrorKind::Msg(format!("path is not valid UTF-8 '{}'", path.display())).into() }) } -/// Wrap `fs::copy` with a nicer error message +/// Wraps `fs::copy` with a nicer error message. pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { if fs::symlink_metadata(&from)?.file_type().is_symlink() { let link = fs::read_link(&from)?; @@ -35,19 +35,19 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { } } -/// Wrap `fs::create_dir` with a nicer error message +/// Wraps `fs::create_dir` with a nicer error message. pub fn create_dir>(path: P) -> Result<()> { fs::create_dir(&path) .chain_err(|| format!("failed to create dir '{}'", path.as_ref().display())) } -/// Wrap `fs::create_dir_all` with a nicer error message +/// Wraps `fs::create_dir_all` with a nicer error message. pub fn create_dir_all>(path: P) -> Result<()> { fs::create_dir_all(&path) .chain_err(|| format!("failed to create dir '{}'", path.as_ref().display())) } -/// Wrap `fs::OpenOptions::create_new().open()` as executable, with a nicer error message +/// Wraps `fs::OpenOptions::create_new().open()` as executable, with a nicer error message. pub fn create_new_executable>(path: P) -> Result { let mut options = fs::OpenOptions::new(); options.write(true).create_new(true); @@ -56,19 +56,19 @@ pub fn create_new_executable>(path: P) -> Result { .chain_err(|| format!("failed to create file '{}'", path.as_ref().display())) } -/// Wrap `fs::OpenOptions::create_new().open()`, with a nicer error message +/// Wraps `fs::OpenOptions::create_new().open()`, with a nicer error message. pub fn create_new_file>(path: P) -> Result { fs::OpenOptions::new().write(true).create_new(true).open(&path) .chain_err(|| format!("failed to create file '{}'", path.as_ref().display())) } -/// Wrap `fs::File::open()` with a nicer error message +/// Wraps `fs::File::open()` with a nicer error message. pub fn open_file>(path: P) -> Result { fs::File::open(&path) .chain_err(|| format!("failed to open file '{}'", path.as_ref().display())) } -/// Wrap `remove_dir_all` with a nicer error message +/// Wraps `remove_dir_all` with a nicer error message. pub fn remove_dir_all>(path: P) -> Result<()> { crate::remove_dir_all::remove_dir_all(path.as_ref()) .chain_err(|| format!("failed to remove dir '{}'", path.as_ref().display())) @@ -87,7 +87,7 @@ pub fn copy_recursive(src: &Path, dst: &Path) -> Result<()> { } /// Copies the `src` directory recursively to `dst`. Both are assumed to exist -/// when this function is called. Invokes a callback for each path visited. +/// when this function is called. Invokes a callback for each path visited. pub fn copy_with_callback(src: &Path, dst: &Path, mut callback: F) -> Result<()> where F: FnMut(&Path, fs::FileType) -> Result<()> { @@ -108,7 +108,7 @@ pub fn copy_with_callback(src: &Path, dst: &Path, mut callback: F) -> Result< } -/// Create an "actor" with default values and setters for all fields. +/// Creates an "actor" with default values and setters for all fields. macro_rules! actor { ($( #[ $attr:meta ] )+ pub struct $name:ident { $( $( #[ $field_attr:meta ] )+ $field:ident : $type:ty = $default:expr, )*