From c2531885d136707cbbc6f19df2b16504c3a0d22f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Sat, 4 Feb 2023 01:31:43 +0000 Subject: [PATCH] Write out manifest.in entries deterministically Currently, the order of the entries in `manifest.in` depends on the iteration order of `copy_with_callback`, which in turn depends on `WalkDir`, which explicitly says iteration order is unspecified. It's possible to call `WalkDir::sort_by` to give an iteration order for each directory's entries, but it felt better to accumulate the lines and then sort them to a) make it more evident that this is happening, and b) enable the copying to be parallelized in the future. --- src/generator.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/generator.rs b/src/generator.rs index 6a4cb9b..1e4d00b 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -3,6 +3,7 @@ use super::Tarballer; use crate::compression::CompressionFormats; use crate::util::*; use anyhow::{bail, format_err, Context, Result}; +use std::collections::BTreeSet; use std::io::Write; use std::path::Path; @@ -121,13 +122,14 @@ impl Generator { /// Copies the `src` directory recursively to `dst`, writing `manifest.in` too. fn copy_and_manifest(src: &Path, dst: &Path, bulk_dirs: &str) -> Result<()> { - let manifest = create_new_file(dst.join("manifest.in"))?; + let mut manifest = create_new_file(dst.join("manifest.in"))?; let bulk_dirs: Vec<_> = bulk_dirs .split(',') .filter(|s| !s.is_empty()) .map(Path::new) .collect(); + let mut paths = BTreeSet::new(); copy_with_callback(src, dst, |path, file_type| { // We need paths to be compatible with both Unix and Windows. if path @@ -157,14 +159,20 @@ fn copy_and_manifest(src: &Path, dst: &Path, bulk_dirs: &str) -> Result<()> { if file_type.is_dir() { // Only manifest directories that are explicitly bulk. if bulk_dirs.contains(&path) { - writeln!(&manifest, "dir:{}", string)?; + paths.insert(format!("dir:{}\n", string)); } } else { // Only manifest files that aren't under bulk directories. if !bulk_dirs.iter().any(|d| path.starts_with(d)) { - writeln!(&manifest, "file:{}", string)?; + paths.insert(format!("file:{}\n", string)); } } Ok(()) - }) + })?; + + for path in paths { + manifest.write_all(path.as_bytes())?; + } + + Ok(()) }