From afb6c6496ea611bfed10ac5773b7ce1cbc671209 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 16:03:15 +0200 Subject: [PATCH 1/2] dist: deduplicate decompression setup code --- src/dist/component/package.rs | 79 +++++++++++------------------------ src/dist/manifestation.rs | 42 ++++++++----------- 2 files changed, 43 insertions(+), 78 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 9a77efc1ae..004a064c4f 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -140,18 +140,13 @@ impl Package for DirectoryPackage { pub(crate) struct TarPackage<'a>(DirectoryPackage, temp::Dir<'a>); impl<'a> TarPackage<'a> { - pub(crate) fn new( - stream: R, - tmp_cx: &'a temp::Context, - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, - process: &Process, - ) -> Result { - let temp_dir = tmp_cx.new_directory()?; + pub(crate) fn new(stream: R, cx: &PackageContext<'a>) -> Result { + let temp_dir = cx.tmp_cx.new_directory()?; let mut archive = tar::Archive::new(stream); // The rust-installer packages unpack to a directory called // $pkgname-$version-$target. Skip that directory when // unpacking. - unpack_without_first_dir(&mut archive, &temp_dir, notify_handler, process) + unpack_without_first_dir(&mut archive, &temp_dir, cx) .context("failed to extract package")?; Ok(TarPackage( @@ -165,8 +160,7 @@ impl<'a> TarPackage<'a> { fn unpack_ram( io_chunk_size: usize, effective_max_ram: Option, - notify_handler: Option<&dyn Fn(Notification<'_>)>, - process: &Process, + cx: &PackageContext<'_>, ) -> usize { const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; let minimum_ram = io_chunk_size * 2; @@ -180,7 +174,8 @@ fn unpack_ram( // Rustup does not know how much RAM the machine has: use the minimum minimum_ram }; - let unpack_ram = match process + let unpack_ram = match cx + .process .var("RUSTUP_UNPACK_RAM") .ok() .and_then(|budget_str| budget_str.parse::().ok()) @@ -203,7 +198,7 @@ fn unpack_ram( } } None => { - if let Some(h) = notify_handler { + if let Some(h) = cx.notify_handler { h(Notification::SetDefaultBufferSize(default_max_unpack_ram)) } default_max_unpack_ram @@ -289,21 +284,21 @@ enum DirStatus { fn unpack_without_first_dir( archive: &mut tar::Archive, path: &Path, - notify_handler: Option<&dyn Fn(Notification<'_>)>, - process: &Process, + cx: &PackageContext<'_>, ) -> Result<()> { let entries = archive.entries()?; let effective_max_ram = match effective_limits::memory_limit() { Ok(ram) => Some(ram as usize), Err(e) => { - if let Some(h) = notify_handler { + if let Some(h) = cx.notify_handler { h(Notification::Error(e.to_string())) } None } }; - let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, notify_handler, process); - let mut io_executor: Box = get_executor(notify_handler, unpack_ram, process)?; + let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, cx); + let mut io_executor: Box = + get_executor(cx.notify_handler, unpack_ram, cx.process)?; let mut directories: HashMap = HashMap::new(); // Path is presumed to exist. Call it a precondition. @@ -463,7 +458,7 @@ fn unpack_without_first_dir( // Tar has item before containing directory // Complain about this so we can see if these exist. writeln!( - process.stderr().lock(), + cx.process.stderr().lock(), "Unexpected: missing parent '{}' for '{}'", parent.display(), entry.path()?.display() @@ -554,19 +549,9 @@ impl Package for TarPackage<'_> { pub(crate) struct TarGzPackage<'a>(TarPackage<'a>); impl<'a> TarGzPackage<'a> { - pub(crate) fn new( - stream: R, - tmp_cx: &'a temp::Context, - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, - process: &Process, - ) -> Result { + pub(crate) fn new(stream: R, cx: &PackageContext<'a>) -> Result { let stream = flate2::read::GzDecoder::new(stream); - Ok(TarGzPackage(TarPackage::new( - stream, - tmp_cx, - notify_handler, - process, - )?)) + Ok(TarGzPackage(TarPackage::new(stream, cx)?)) } } @@ -592,19 +577,9 @@ impl Package for TarGzPackage<'_> { pub(crate) struct TarXzPackage<'a>(TarPackage<'a>); impl<'a> TarXzPackage<'a> { - pub(crate) fn new( - stream: R, - tmp_cx: &'a temp::Context, - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, - process: &Process, - ) -> Result { + pub(crate) fn new(stream: R, cx: &PackageContext<'a>) -> Result { let stream = xz2::read::XzDecoder::new(stream); - Ok(TarXzPackage(TarPackage::new( - stream, - tmp_cx, - notify_handler, - process, - )?)) + Ok(TarXzPackage(TarPackage::new(stream, cx)?)) } } @@ -630,19 +605,9 @@ impl Package for TarXzPackage<'_> { pub(crate) struct TarZStdPackage<'a>(TarPackage<'a>); impl<'a> TarZStdPackage<'a> { - pub(crate) fn new( - stream: R, - tmp_cx: &'a temp::Context, - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, - process: &Process, - ) -> Result { + pub(crate) fn new(stream: R, cx: &PackageContext<'a>) -> Result { let stream = zstd::stream::read::Decoder::new(stream)?; - Ok(TarZStdPackage(TarPackage::new( - stream, - tmp_cx, - notify_handler, - process, - )?)) + Ok(TarZStdPackage(TarPackage::new(stream, cx)?)) } } @@ -663,3 +628,9 @@ impl Package for TarZStdPackage<'_> { self.0.components() } } + +pub(crate) struct PackageContext<'a> { + pub(crate) tmp_cx: &'a temp::Context, + pub(crate) notify_handler: Option<&'a dyn Fn(Notification<'_>)>, + pub(crate) process: &'a Process, +} diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 86650e2e31..d0e1b46595 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -13,7 +13,7 @@ use tokio::sync::Semaphore; use tracing::info; use crate::dist::component::{ - Components, Package, TarGzPackage, TarXzPackage, TarZStdPackage, Transaction, + Components, Package, PackageContext, TarGzPackage, TarXzPackage, TarZStdPackage, Transaction, }; use crate::dist::config::Config; use crate::dist::download::{DownloadCfg, File}; @@ -267,37 +267,27 @@ impl Manifestation { let notification_converter = |notification: crate::utils::Notification<'_>| { (download_cfg.notify_handler)(notification.into()); }; - let gz; - let xz; - let zst; + + let cx = PackageContext { + tmp_cx, + notify_handler: Some(¬ification_converter), + process: download_cfg.process, + }; + + let (gz, xz, zst); let reader = utils::FileReaderWithProgress::new_file(&installer_file, ¬ification_converter)?; let package: &dyn Package = match format { CompressionKind::GZip => { - gz = TarGzPackage::new( - reader, - tmp_cx, - Some(¬ification_converter), - download_cfg.process, - )?; + gz = TarGzPackage::new(reader, &cx)?; &gz } CompressionKind::XZ => { - xz = TarXzPackage::new( - reader, - tmp_cx, - Some(¬ification_converter), - download_cfg.process, - )?; + xz = TarXzPackage::new(reader, &cx)?; &xz } CompressionKind::ZStd => { - zst = TarZStdPackage::new( - reader, - tmp_cx, - Some(¬ification_converter), - download_cfg.process, - )?; + zst = TarZStdPackage::new(reader, &cx)?; &zst } }; @@ -511,9 +501,13 @@ impl Manifestation { }; let reader = utils::FileReaderWithProgress::new_file(&installer_file, ¬ification_converter)?; - let package: &dyn Package = - &TarGzPackage::new(reader, tmp_cx, Some(¬ification_converter), process)?; + let cx = PackageContext { + tmp_cx, + notify_handler: Some(¬ification_converter), + process, + }; + let package: &dyn Package = &TarGzPackage::new(reader, &cx)?; for component in package.components() { tx = package.install(&self.installation, &component, None, tx)?; } From 7a7bfc735b27c35e59ab3c3acdb0c9426978e456 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 16:03:51 +0200 Subject: [PATCH 2/2] dist: simpify casting to trait object --- src/dist/manifestation.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index d0e1b46595..fcd5953ef9 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -274,22 +274,12 @@ impl Manifestation { process: download_cfg.process, }; - let (gz, xz, zst); let reader = utils::FileReaderWithProgress::new_file(&installer_file, ¬ification_converter)?; - let package: &dyn Package = match format { - CompressionKind::GZip => { - gz = TarGzPackage::new(reader, &cx)?; - &gz - } - CompressionKind::XZ => { - xz = TarXzPackage::new(reader, &cx)?; - &xz - } - CompressionKind::ZStd => { - zst = TarZStdPackage::new(reader, &cx)?; - &zst - } + let package = match format { + CompressionKind::GZip => &TarGzPackage::new(reader, &cx)? as &dyn Package, + CompressionKind::XZ => &TarXzPackage::new(reader, &cx)?, + CompressionKind::ZStd => &TarZStdPackage::new(reader, &cx)?, }; // If the package doesn't contain the component that the