From 57a426ee85ab6954eca726e4f75391e2c4dad1c1 Mon Sep 17 00:00:00 2001 From: Kyle Lacy Date: Sun, 8 Sep 2024 17:24:39 -0700 Subject: [PATCH 1/3] Update autopacking to ensure dependencies get packed first --- crates/brioche-autopack/src/lib.rs | 106 +++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/crates/brioche-autopack/src/lib.rs b/crates/brioche-autopack/src/lib.rs index eb7c075..3552a35 100644 --- a/crates/brioche-autopack/src/lib.rs +++ b/crates/brioche-autopack/src/lib.rs @@ -1,5 +1,5 @@ use std::{ - collections::{HashMap, HashSet, VecDeque}, + collections::{BTreeMap, HashMap, HashSet, VecDeque}, io::{BufRead as _, Read as _, Write as _}, path::{Path, PathBuf}, }; @@ -151,18 +151,21 @@ fn relative_template( #[derive(Debug, Clone)] pub struct RepackConfig {} +struct AutopackPathConfig { + can_skip: bool, +} + pub fn autopack(config: &AutopackConfig) -> eyre::Result<()> { let ctx = autopack_context(config)?; + let mut pending_paths = BTreeMap::::new(); match &config.inputs { AutopackInputs::Paths(paths) => { - for path in paths { - let did_pack = try_autopack_path(&ctx, path, path)?; - eyre::ensure!(did_pack, "failed to autopack path: {path:?}"); - if !config.quiet { - println!("autopacked {}", path.display()); - } - } + pending_paths.extend( + paths + .iter() + .map(|path| (path.clone(), AutopackPathConfig { can_skip: true })), + ); } AutopackInputs::Globs { base_path, @@ -191,19 +194,19 @@ pub fn autopack(config: &AutopackConfig) -> eyre::Result<()> { })?; if globs.is_match(&relative_entry_path) { - let did_pack = try_autopack_path(&ctx, entry.path(), entry.path())?; - if !config.quiet { - if did_pack { - println!("autopacked {}", entry.path().display()); - } else { - println!("skipped {}", entry.path().display()); - } - } + pending_paths.insert( + entry.path().to_owned(), + AutopackPathConfig { can_skip: false }, + ); } } } } + while let Some((path, path_config)) = pending_paths.pop_first() { + autopack_path(&ctx, &path, &path_config, &mut pending_paths)?; + } + Ok(()) } @@ -294,20 +297,47 @@ fn autopack_context(config: &AutopackConfig) -> eyre::Result { }) } +fn autopack_path( + ctx: &AutopackContext, + path: &Path, + path_config: &AutopackPathConfig, + pending_paths: &mut BTreeMap, +) -> eyre::Result<()> { + let did_pack = try_autopack_path(ctx, path, path, pending_paths)?; + if did_pack { + if !ctx.config.quiet { + println!("autopacked {}", path.display()); + } + } else if !path_config.can_skip { + if !ctx.config.quiet { + println!("skipped {}", path.display()); + } + } else { + eyre::bail!("failed to autopack path: {path:?}"); + } + + Ok(()) +} + fn try_autopack_path( ctx: &AutopackContext, source_path: &Path, output_path: &Path, + pending_paths: &mut BTreeMap, ) -> eyre::Result { let Some(kind) = autopack_kind(source_path)? else { return Ok(false); }; match kind { - AutopackKind::DynamicBinary => autopack_dynamic_binary(ctx, source_path, output_path), - AutopackKind::SharedLibrary => autopack_shared_library(ctx, source_path, output_path), - AutopackKind::Script => autopack_script(ctx, source_path, output_path), - AutopackKind::Repack => autopack_repack(ctx, source_path, output_path), + AutopackKind::DynamicBinary => { + autopack_dynamic_binary(ctx, source_path, output_path, pending_paths) + } + AutopackKind::SharedLibrary => { + autopack_shared_library(ctx, source_path, output_path, pending_paths) + } + AutopackKind::Script => autopack_script(ctx, source_path, output_path, pending_paths), + AutopackKind::Repack => autopack_repack(ctx, source_path, output_path, pending_paths), } } @@ -350,6 +380,7 @@ fn autopack_dynamic_binary( ctx: &AutopackContext, source_path: &Path, output_path: &Path, + pending_paths: &mut BTreeMap, ) -> eyre::Result { let Some(dynamic_binary_config) = &ctx.config.dynamic_binary else { return Ok(false); @@ -391,6 +422,17 @@ fn autopack_dynamic_binary( let interpreter_path = interpreter_path.ok_or_else(|| { eyre::eyre!("could not find interpreter for dynamic binary: {source_path:?}") })?; + + // Autopack the interpreter if it's pending + if let Some(interpreter_path_config) = pending_paths.remove(&interpreter_path) { + autopack_path( + ctx, + &interpreter_path, + &interpreter_path_config, + pending_paths, + )?; + } + let interpreter_resource_path = add_named_blob_from(ctx, &interpreter_path, None) .with_context(|| format!("failed to add resource for interpreter {interpreter_path:?}"))?; let program_resource_path = add_named_blob_from(ctx, source_path, None) @@ -414,6 +456,7 @@ fn autopack_dynamic_binary( ctx, &dynamic_binary_config.dynamic_linking, needed_libraries, + pending_paths, )?; let program = >::from_path_buf(program_resource_path) @@ -461,6 +504,7 @@ fn autopack_shared_library( ctx: &AutopackContext, source_path: &Path, output_path: &Path, + pending_paths: &mut BTreeMap, ) -> eyre::Result { let Some(shared_library_config) = &ctx.config.shared_library else { return Ok(false); @@ -500,6 +544,7 @@ fn autopack_shared_library( ctx, &shared_library_config.dynamic_linking, needed_libraries, + pending_paths, )?; let library_dirs = library_dir_resource_paths @@ -527,6 +572,7 @@ fn autopack_script( ctx: &AutopackContext, source_path: &Path, output_path: &Path, + pending_paths: &mut BTreeMap, ) -> eyre::Result { let Some(script_config) = &ctx.config.script else { return Ok(false); @@ -571,6 +617,12 @@ fn autopack_script( } let command = command.ok_or_else(|| eyre::eyre!("could not find command {command_name:?}"))?; + + // Autopack the command if it's still pending + if let Some(command_path_options) = pending_paths.remove(&command) { + autopack_path(ctx, &command, &command_path_options, pending_paths)?; + } + let command_resource = add_named_blob_from(ctx, &command, None)?; let script_resource = add_named_blob_from(ctx, source_path, None)?; @@ -661,6 +713,7 @@ fn autopack_repack( ctx: &AutopackContext, source_path: &Path, output_path: &Path, + pending_paths: &mut BTreeMap, ) -> eyre::Result { let Some(_) = &ctx.config.repack else { return Ok(false); @@ -756,7 +809,12 @@ fn autopack_repack( } } - let result = try_autopack_path(ctx, &unpacked_source_path, &unpacked_output_path)?; + let result = try_autopack_path( + ctx, + &unpacked_source_path, + &unpacked_output_path, + pending_paths, + )?; Ok(result) } @@ -769,6 +827,7 @@ fn collect_all_library_dirs( ctx: &AutopackContext, dynamic_linking_config: &DynamicLinkingConfig, mut needed_libraries: VecDeque, + pending_paths: &mut BTreeMap, ) -> eyre::Result> { let mut library_search_paths = vec![]; let mut resource_library_dirs = vec![]; @@ -794,6 +853,11 @@ fn collect_all_library_dirs( } }; + // Autopack the library if it's pending + if let Some(library_path_config) = pending_paths.remove(&library_path) { + autopack_path(ctx, &library_path, &library_path_config, pending_paths)?; + } + found_libraries.insert(library_name.clone()); // Don't add the library if it's been skipped. We still do everything From 1c00a338107e2ffb50f5e538364f2c1f569f1960 Mon Sep 17 00:00:00 2001 From: Kyle Lacy Date: Sun, 8 Sep 2024 23:43:41 -0700 Subject: [PATCH 2/3] Update autopacking to canonicalize paths before packing dependencies --- crates/brioche-autopack/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/brioche-autopack/src/lib.rs b/crates/brioche-autopack/src/lib.rs index 3552a35..f85a618 100644 --- a/crates/brioche-autopack/src/lib.rs +++ b/crates/brioche-autopack/src/lib.rs @@ -424,7 +424,10 @@ fn autopack_dynamic_binary( })?; // Autopack the interpreter if it's pending - if let Some(interpreter_path_config) = pending_paths.remove(&interpreter_path) { + let canonical_interpreter_path = interpreter_path + .canonicalize() + .with_context(|| format!("failed to canonicalize interpreter path {interpreter_path:?}"))?; + if let Some(interpreter_path_config) = pending_paths.remove(&canonical_interpreter_path) { autopack_path( ctx, &interpreter_path, @@ -854,7 +857,10 @@ fn collect_all_library_dirs( }; // Autopack the library if it's pending - if let Some(library_path_config) = pending_paths.remove(&library_path) { + let canonical_library_path = library_path + .canonicalize() + .with_context(|| format!("failed to canonicalize library path {library_path:?}"))?; + if let Some(library_path_config) = pending_paths.remove(&canonical_library_path) { autopack_path(ctx, &library_path, &library_path_config, pending_paths)?; } From 39cf5933982893f94d3dd64def895b43a656ff80 Mon Sep 17 00:00:00 2001 From: Kyle Lacy Date: Sun, 8 Sep 2024 23:58:45 -0700 Subject: [PATCH 3/3] Clean up dependency autopacking --- crates/brioche-autopack/src/lib.rs | 43 +++++++++++++++--------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/crates/brioche-autopack/src/lib.rs b/crates/brioche-autopack/src/lib.rs index f85a618..cac58f1 100644 --- a/crates/brioche-autopack/src/lib.rs +++ b/crates/brioche-autopack/src/lib.rs @@ -424,17 +424,7 @@ fn autopack_dynamic_binary( })?; // Autopack the interpreter if it's pending - let canonical_interpreter_path = interpreter_path - .canonicalize() - .with_context(|| format!("failed to canonicalize interpreter path {interpreter_path:?}"))?; - if let Some(interpreter_path_config) = pending_paths.remove(&canonical_interpreter_path) { - autopack_path( - ctx, - &interpreter_path, - &interpreter_path_config, - pending_paths, - )?; - } + try_autopack_dependency(ctx, &interpreter_path, pending_paths)?; let interpreter_resource_path = add_named_blob_from(ctx, &interpreter_path, None) .with_context(|| format!("failed to add resource for interpreter {interpreter_path:?}"))?; @@ -621,10 +611,8 @@ fn autopack_script( let command = command.ok_or_else(|| eyre::eyre!("could not find command {command_name:?}"))?; - // Autopack the command if it's still pending - if let Some(command_path_options) = pending_paths.remove(&command) { - autopack_path(ctx, &command, &command_path_options, pending_paths)?; - } + // Autopack the command if it's pending + try_autopack_dependency(ctx, &command, pending_paths)?; let command_resource = add_named_blob_from(ctx, &command, None)?; let script_resource = add_named_blob_from(ctx, source_path, None)?; @@ -857,12 +845,7 @@ fn collect_all_library_dirs( }; // Autopack the library if it's pending - let canonical_library_path = library_path - .canonicalize() - .with_context(|| format!("failed to canonicalize library path {library_path:?}"))?; - if let Some(library_path_config) = pending_paths.remove(&canonical_library_path) { - autopack_path(ctx, &library_path, &library_path_config, pending_paths)?; - } + try_autopack_dependency(ctx, &library_path, pending_paths)?; found_libraries.insert(library_name.clone()); @@ -1025,3 +1008,21 @@ fn add_named_blob_from( )?; Ok(resource_path) } + +fn try_autopack_dependency( + ctx: &AutopackContext, + path: &Path, + pending_paths: &mut BTreeMap, +) -> eyre::Result<()> { + // Get the canonical path of the dependency + let canonical_path = path + .canonicalize() + .with_context(|| format!("failed to canonicalize path {path:?}"))?; + + // If the path is pending, then autopack it + if let Some(path_config) = pending_paths.remove(&canonical_path) { + autopack_path(ctx, path, &path_config, pending_paths)?; + } + + Ok(()) +}