From 81e6deba75509c2390802be072802bbc4ccee966 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 17 Jul 2021 18:04:26 +0200 Subject: [PATCH 1/5] Provide more context for errors while generating an installer --- src/generator.rs | 3 ++- src/util.rs | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/generator.rs b/src/generator.rs index 2601eb5..4c247be 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -78,7 +78,8 @@ impl Generator { // Copy the overlay if !self.non_installed_overlay.is_empty() { - copy_recursive(self.non_installed_overlay.as_ref(), &package_dir)?; + copy_recursive(self.non_installed_overlay.as_ref(), &package_dir) + .context("failed to copy overlay")?; } // Generate the install script diff --git a/src/util.rs b/src/util.rs index 078ceb3..e600727 100644 --- a/src/util.rs +++ b/src/util.rs @@ -28,9 +28,16 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { } else { let amt = fs::copy(&from, &to).with_context(|| { format!( - "failed to copy '{}' to '{}'", + "failed to copy '{}' ({}) to '{}' ({}, parent {})", from.as_ref().display(), - to.as_ref().display() + if from.as_ref().exists() { "exists" } else { "doesn't exist" }, + to.as_ref().display(), + if to.as_ref().exists() { "exists" } else { "doesn't exist" }, + if to.as_ref().parent().unwrap_or_else(|| Path::new("")).exists() { + "exists" + } else { + "doesn't exist" + }, ) })?; Ok(amt) @@ -97,7 +104,20 @@ pub fn remove_file>(path: P) -> Result<()> { /// Copies the `src` directory recursively to `dst`. Both are assumed to exist /// when this function is called. pub fn copy_recursive(src: &Path, dst: &Path) -> Result<()> { - copy_with_callback(src, dst, |_, _| Ok(())) + copy_with_callback(src, dst, |_, _| Ok(())).with_context(|| { + format!( + "failed to recursively copy '{}' ({}) to '{}' ({}, parent {})", + src.display(), + if src.exists() { "exists" } else { "doesn't exist" }, + dst.display(), + if dst.exists() { "exists" } else { "doesn't exist" }, + if dst.parent().unwrap_or_else(|| Path::new("")).exists() { + "exists" + } else { + "doesn't exist" + }, + ) + }) } /// Copies the `src` directory recursively to `dst`. Both are assumed to exist From 40635547304289a62db730e3f0a32d5d270ac2fb Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 17 Jul 2021 19:23:51 +0200 Subject: [PATCH 2/5] Canonicalize paths --- src/tarballer.rs | 6 ++++++ src/util.rs | 16 +++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/tarballer.rs b/src/tarballer.rs index 4ac8cf7..a839847 100644 --- a/src/tarballer.rs +++ b/src/tarballer.rs @@ -63,6 +63,12 @@ impl Tarballer { } for path in files { let src = Path::new(&self.work_dir).join(&path); + let src = src + .parent() + .unwrap() + .canonicalize() + .map(|path| path.join(src.file_name().unwrap())) + .unwrap_or_else(|_| src.to_owned()); append_path(&mut builder, &src, &path) .with_context(|| format!("failed to tar file '{}'", src.display()))?; } diff --git a/src/util.rs b/src/util.rs index e600727..2a6b9b4 100644 --- a/src/util.rs +++ b/src/util.rs @@ -21,6 +21,12 @@ pub fn path_to_str(path: &Path) -> Result<&str> { /// Wraps `fs::copy` with a nicer error message. pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { + // Canonicalize the paths. On windows this results in `\\?\` paths for which the `MAX_PATH` + // limit doesn't apply. Fallback to the original path if canonicalization fails. This can happen + // on Windows when using a network drive. + let from = from.as_ref().canonicalize().unwrap_or_else(|_| from.as_ref().to_owned()); + let to = to.as_ref().parent().unwrap().canonicalize().map(|path| path.join(to.as_ref().file_name().unwrap())).unwrap_or_else(|_| to.as_ref().to_owned()); + if fs::symlink_metadata(&from)?.file_type().is_symlink() { let link = fs::read_link(&from)?; symlink_file(link, &to)?; @@ -29,11 +35,11 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { let amt = fs::copy(&from, &to).with_context(|| { format!( "failed to copy '{}' ({}) to '{}' ({}, parent {})", - from.as_ref().display(), - if from.as_ref().exists() { "exists" } else { "doesn't exist" }, - to.as_ref().display(), - if to.as_ref().exists() { "exists" } else { "doesn't exist" }, - if to.as_ref().parent().unwrap_or_else(|| Path::new("")).exists() { + from.display(), + if from.exists() { "exists" } else { "doesn't exist" }, + to.display(), + if to.exists() { "exists" } else { "doesn't exist" }, + if to.parent().unwrap_or_else(|| Path::new("")).exists() { "exists" } else { "doesn't exist" From 9cbbe75750755559fad38b6394821a8592dbd161 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 25 Oct 2021 17:16:55 +0200 Subject: [PATCH 3/5] Revert canonicalization and use PathBuf instead of String --- src/combiner.rs | 13 +++++++------ src/generator.rs | 11 ++++++----- src/tarballer.rs | 32 +++++++------------------------- src/util.rs | 16 +++++----------- 4 files changed, 25 insertions(+), 47 deletions(-) diff --git a/src/combiner.rs b/src/combiner.rs index 006a40c..d814597 100644 --- a/src/combiner.rs +++ b/src/combiner.rs @@ -7,6 +7,7 @@ use crate::{ use anyhow::{bail, Context, Result}; use std::io::{Read, Write}; use std::path::Path; +use std::path::PathBuf; use tar::Archive; actor! { @@ -34,10 +35,10 @@ actor! { non_installed_overlay: String = "", /// The directory to do temporary work. - work_dir: String = "./workdir", + work_dir: PathBuf = "./workdir", /// The location to put the final image and tarball. - output_dir: String = "./dist", + output_dir: PathBuf = "./dist", /// The formats used to compress the tarball compression_formats: CompressionFormats = CompressionFormats::default(), @@ -49,7 +50,7 @@ impl Combiner { pub fn run(self) -> Result<()> { create_dir_all(&self.work_dir)?; - let package_dir = Path::new(&self.work_dir).join(&self.package_name); + let package_dir = self.work_dir.join(&self.package_name); if package_dir.exists() { remove_dir_all(&package_dir)?; } @@ -73,14 +74,14 @@ impl Combiner { .with_context(|| { format!( "unable to extract '{}' into '{}'", - &input_tarball, self.work_dir + &input_tarball, self.work_dir.display() ) })?; let pkg_name = input_tarball.trim_end_matches(&format!(".tar.{}", compression.extension())); let pkg_name = Path::new(pkg_name).file_name().unwrap(); - let pkg_dir = Path::new(&self.work_dir).join(&pkg_name); + let pkg_dir = self.work_dir.join(&pkg_name); // Verify the version number. let mut version = String::new(); @@ -137,7 +138,7 @@ impl Combiner { // Make the tarballs. create_dir_all(&self.output_dir)?; - let output = Path::new(&self.output_dir).join(&self.package_name); + let output = self.output_dir.join(&self.package_name); let mut tarballer = Tarballer::default(); tarballer .work_dir(self.work_dir) diff --git a/src/generator.rs b/src/generator.rs index 4c247be..8605527 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -5,6 +5,7 @@ use crate::util::*; use anyhow::{bail, format_err, Context, Result}; use std::io::Write; use std::path::Path; +use std::path::PathBuf; actor! { #[derive(Debug)] @@ -34,13 +35,13 @@ actor! { bulk_dirs: String = "", /// The directory containing the installation medium - image_dir: String = "./install_image", + image_dir: PathBuf = "./install_image", /// The directory to do temporary work - work_dir: String = "./workdir", + work_dir: PathBuf = "./workdir", /// The location to put the final image and tarball - output_dir: String = "./dist", + output_dir: PathBuf = "./dist", /// The formats used to compress the tarball compression_formats: CompressionFormats = CompressionFormats::default(), @@ -52,7 +53,7 @@ impl Generator { pub fn run(self) -> Result<()> { create_dir_all(&self.work_dir)?; - let package_dir = Path::new(&self.work_dir).join(&self.package_name); + let package_dir = self.work_dir.join(&self.package_name); if package_dir.exists() { remove_dir_all(&package_dir)?; } @@ -95,7 +96,7 @@ impl Generator { // Make the tarballs create_dir_all(&self.output_dir)?; - let output = Path::new(&self.output_dir).join(&self.package_name); + let output = self.output_dir.join(&self.package_name); let mut tarballer = Tarballer::default(); tarballer .work_dir(self.work_dir) diff --git a/src/tarballer.rs b/src/tarballer.rs index a839847..786a5f3 100644 --- a/src/tarballer.rs +++ b/src/tarballer.rs @@ -1,7 +1,7 @@ use anyhow::{bail, Context, Result}; use std::fs::{read_link, symlink_metadata}; use std::io::{empty, BufWriter, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use tar::{Builder, Header}; use walkdir::WalkDir; @@ -20,7 +20,7 @@ actor! { output: String = "./dist", /// The folder in which the input is to be found. - work_dir: String = "./workdir", + work_dir: PathBuf = "./workdir", /// The formats used to compress the tarball. compression_formats: CompressionFormats = CompressionFormats::default(), @@ -50,25 +50,16 @@ impl Tarballer { let buf = BufWriter::with_capacity(1024 * 1024, encoder); let mut builder = Builder::new(buf); - let pool = rayon::ThreadPoolBuilder::new() - .num_threads(2) - .build() - .unwrap(); + let pool = rayon::ThreadPoolBuilder::new().num_threads(2).build().unwrap(); pool.install(move || { for path in dirs { - let src = Path::new(&self.work_dir).join(&path); + let src = self.work_dir.join(&path); builder .append_dir(&path, &src) .with_context(|| format!("failed to tar dir '{}'", src.display()))?; } for path in files { - let src = Path::new(&self.work_dir).join(&path); - let src = src - .parent() - .unwrap() - .canonicalize() - .map(|path| path.join(src.file_name().unwrap())) - .unwrap_or_else(|_| src.to_owned()); + let src = self.work_dir.join(&path); append_path(&mut builder, &src, &path) .with_context(|| format!("failed to tar file '{}'", src.display()))?; } @@ -112,20 +103,11 @@ fn append_path(builder: &mut Builder, src: &Path, path: &String) -> } /// Returns all `(directories, files)` under the source path. -fn get_recursive_paths(root: P, name: Q) -> Result<(Vec, Vec)> -where - P: AsRef, - Q: AsRef, -{ - let root = root.as_ref(); +fn get_recursive_paths(root: &Path, name: impl AsRef) -> Result<(Vec, Vec)> { let name = name.as_ref(); if !name.is_relative() && !name.starts_with(root) { - bail!( - "input '{}' is not in work dir '{}'", - name.display(), - root.display() - ); + bail!("input '{}' is not in work dir '{}'", name.display(), root.display()); } let mut dirs = vec![]; diff --git a/src/util.rs b/src/util.rs index 2a6b9b4..e600727 100644 --- a/src/util.rs +++ b/src/util.rs @@ -21,12 +21,6 @@ pub fn path_to_str(path: &Path) -> Result<&str> { /// Wraps `fs::copy` with a nicer error message. pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { - // Canonicalize the paths. On windows this results in `\\?\` paths for which the `MAX_PATH` - // limit doesn't apply. Fallback to the original path if canonicalization fails. This can happen - // on Windows when using a network drive. - let from = from.as_ref().canonicalize().unwrap_or_else(|_| from.as_ref().to_owned()); - let to = to.as_ref().parent().unwrap().canonicalize().map(|path| path.join(to.as_ref().file_name().unwrap())).unwrap_or_else(|_| to.as_ref().to_owned()); - if fs::symlink_metadata(&from)?.file_type().is_symlink() { let link = fs::read_link(&from)?; symlink_file(link, &to)?; @@ -35,11 +29,11 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { let amt = fs::copy(&from, &to).with_context(|| { format!( "failed to copy '{}' ({}) to '{}' ({}, parent {})", - from.display(), - if from.exists() { "exists" } else { "doesn't exist" }, - to.display(), - if to.exists() { "exists" } else { "doesn't exist" }, - if to.parent().unwrap_or_else(|| Path::new("")).exists() { + from.as_ref().display(), + if from.as_ref().exists() { "exists" } else { "doesn't exist" }, + to.as_ref().display(), + if to.as_ref().exists() { "exists" } else { "doesn't exist" }, + if to.as_ref().parent().unwrap_or_else(|| Path::new("")).exists() { "exists" } else { "doesn't exist" From 278e3fbccc496e321f35129fd9e6ae72be6d1152 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 25 Oct 2021 17:52:50 +0200 Subject: [PATCH 4/5] Support long paths on windows --- src/combiner.rs | 2 +- src/generator.rs | 11 +++---- src/tarballer.rs | 4 +-- src/util.rs | 80 ++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/src/combiner.rs b/src/combiner.rs index d814597..4c47024 100644 --- a/src/combiner.rs +++ b/src/combiner.rs @@ -141,7 +141,7 @@ impl Combiner { let output = self.output_dir.join(&self.package_name); let mut tarballer = Tarballer::default(); tarballer - .work_dir(self.work_dir) + .work_dir(LongPath::new(self.work_dir)) .input(self.package_name) .output(path_to_str(&output)?.into()) .compression_formats(self.compression_formats.clone()); diff --git a/src/generator.rs b/src/generator.rs index 8605527..c198f50 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -5,7 +5,6 @@ use crate::util::*; use anyhow::{bail, format_err, Context, Result}; use std::io::Write; use std::path::Path; -use std::path::PathBuf; actor! { #[derive(Debug)] @@ -35,13 +34,13 @@ actor! { bulk_dirs: String = "", /// The directory containing the installation medium - image_dir: PathBuf = "./install_image", + image_dir: LongPath = "./install_image", /// The directory to do temporary work - work_dir: PathBuf = "./workdir", + work_dir: LongPath = "./workdir", /// The location to put the final image and tarball - output_dir: PathBuf = "./dist", + output_dir: LongPath = "./dist", /// The formats used to compress the tarball compression_formats: CompressionFormats = CompressionFormats::default(), @@ -51,7 +50,7 @@ actor! { impl Generator { /// Generates the actual installer tarball pub fn run(self) -> Result<()> { - create_dir_all(&self.work_dir)?; + create_dir_all(&*self.work_dir)?; let package_dir = self.work_dir.join(&self.package_name); if package_dir.exists() { @@ -95,7 +94,7 @@ impl Generator { scripter.run()?; // Make the tarballs - create_dir_all(&self.output_dir)?; + create_dir_all(&*self.output_dir)?; let output = self.output_dir.join(&self.package_name); let mut tarballer = Tarballer::default(); tarballer diff --git a/src/tarballer.rs b/src/tarballer.rs index 786a5f3..4b4891e 100644 --- a/src/tarballer.rs +++ b/src/tarballer.rs @@ -1,7 +1,7 @@ use anyhow::{bail, Context, Result}; use std::fs::{read_link, symlink_metadata}; use std::io::{empty, BufWriter, Write}; -use std::path::{Path, PathBuf}; +use std::path::Path; use tar::{Builder, Header}; use walkdir::WalkDir; @@ -20,7 +20,7 @@ actor! { output: String = "./dist", /// The folder in which the input is to be found. - work_dir: PathBuf = "./workdir", + work_dir: LongPath = "./workdir", /// The formats used to compress the tarball. compression_formats: CompressionFormats = CompressionFormats::default(), diff --git a/src/util.rs b/src/util.rs index e600727..19c99c2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,6 +1,7 @@ use anyhow::{format_err, Context, Result}; use std::fs; -use std::path::Path; +use std::ops::Deref; +use std::path::{Component, Path, PathBuf, Prefix}; use walkdir::WalkDir; // Needed to set the script mode to executable. @@ -15,8 +16,7 @@ use std::os::windows::fs::symlink_file; /// Converts a `&Path` to a UTF-8 `&str`. pub fn path_to_str(path: &Path) -> Result<&str> { - path.to_str() - .ok_or_else(|| format_err!("path is not valid UTF-8 '{}'", path.display())) + path.to_str().ok_or_else(|| format_err!("path is not valid UTF-8 '{}'", path.display())) } /// Wraps `fs::copy` with a nicer error message. @@ -142,6 +142,80 @@ where Ok(()) } +fn normalize_rest(path: PathBuf) -> PathBuf { + let mut new_components = vec![]; + for component in path.components().skip(1) { + match component { + Component::Prefix(_) => unreachable!(), + Component::RootDir => new_components.clear(), + Component::CurDir => {} + Component::ParentDir => { + new_components.pop(); + } + Component::Normal(component) => new_components.push(component), + } + } + new_components.into_iter().collect() +} + +#[derive(Debug)] +pub struct LongPath(PathBuf); + +impl LongPath { + pub fn new(path: PathBuf) -> Self { + let path = if cfg!(windows) { + match path.components().next().unwrap() { + Component::Prefix(prefix_component) => { + match prefix_component.kind() { + Prefix::Verbatim(_) + | Prefix::VerbatimUNC(_, _) + | Prefix::VerbatimDisk(_) => { + // Already a verbatim path. + path + } + + Prefix::DeviceNS(dev) => { + Path::new("\\\\?\\").join(dev).join(normalize_rest(path)) + } + Prefix::UNC(host, share) => { + Path::new("\\\\?\\").join(host).join(share).join(normalize_rest(path)) + } + Prefix::Disk(_disk) => { + Path::new("\\\\?\\").join(prefix_component.as_os_str()).join(normalize_rest(path)) + } + } + } + + Component::RootDir + | Component::CurDir + | Component::ParentDir + | Component::Normal(_) => { + return LongPath::new( + std::env::current_dir().expect("failed to get current dir").join(&path), + ); + } + } + } else { + path + }; + LongPath(path) + } +} + +impl Into for &str { + fn into(self) -> LongPath { + LongPath::new(self.into()) + } +} + +impl Deref for LongPath { + type Target = Path; + + fn deref(&self) -> &Path { + &self.0 + } +} + /// Creates an "actor" with default values and setters for all fields. macro_rules! actor { ($( #[ $attr:meta ] )+ pub struct $name:ident { From 6888351ae5a4aae7466717bbeb2927f50867b750 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 25 Oct 2021 18:59:05 +0200 Subject: [PATCH 5/5] Fix it --- src/util.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/util.rs b/src/util.rs index 19c99c2..c8ee161 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,4 +1,5 @@ use anyhow::{format_err, Context, Result}; +use std::ffi::OsString; use std::fs; use std::ops::Deref; use std::path::{Component, Path, PathBuf, Prefix}; @@ -164,7 +165,8 @@ pub struct LongPath(PathBuf); impl LongPath { pub fn new(path: PathBuf) -> Self { let path = if cfg!(windows) { - match path.components().next().unwrap() { + // Convert paths to verbatim paths to ensure that paths longer than 255 characters work + match dbg!(path.components().next().unwrap()) { Component::Prefix(prefix_component) => { match prefix_component.kind() { Prefix::Verbatim(_) @@ -175,13 +177,21 @@ impl LongPath { } Prefix::DeviceNS(dev) => { - Path::new("\\\\?\\").join(dev).join(normalize_rest(path)) + let mut base = OsString::from("\\\\?\\"); + base.push(dev); + Path::new(&base).join(normalize_rest(path)) } Prefix::UNC(host, share) => { - Path::new("\\\\?\\").join(host).join(share).join(normalize_rest(path)) + let mut base = OsString::from("\\\\?\\UNC\\"); + base.push(host); + base.push("\\"); + base.push(share); + Path::new(&base).join(normalize_rest(path)) } Prefix::Disk(_disk) => { - Path::new("\\\\?\\").join(prefix_component.as_os_str()).join(normalize_rest(path)) + let mut base = OsString::from("\\\\?\\"); + base.push(prefix_component.as_os_str()); + Path::new(&base).join(normalize_rest(path)) } } } @@ -190,15 +200,15 @@ impl LongPath { | Component::CurDir | Component::ParentDir | Component::Normal(_) => { - return LongPath::new( - std::env::current_dir().expect("failed to get current dir").join(&path), - ); + return LongPath::new(dbg!( + std::env::current_dir().expect("failed to get current dir").join(&path) + )); } } } else { path }; - LongPath(path) + LongPath(dbg!(path)) } }