From 7e274c50a49b473b9407302a44a7671219974187 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Fri, 11 Aug 2023 15:35:06 -0300 Subject: [PATCH 1/3] In preparation for adding support for testing meta-buildpacks the process of building output directories is being improved in libcnb-package so that they are easier to work with in libcnb-package and libcnb-test. Both the packaging and testing processes will use this new functionality to construct output locations to write compiled buildpacks into as well as rewriting dependency paths to resolved project-level dependencies declared with libcnb:{buildpack_id}. --- CHANGELOG.md | 14 ++- libcnb-cargo/src/package/command.rs | 133 +++++++++++---------- libcnb-cargo/src/package/error.rs | 13 +- libcnb-cargo/tests/test.rs | 21 ++-- libcnb-package/src/buildpack_dependency.rs | 20 ++-- libcnb-package/src/lib.rs | 54 +-------- libcnb-package/src/output.rs | 77 ++++++++++++ 7 files changed, 187 insertions(+), 145 deletions(-) create mode 100644 libcnb-package/src/output.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 63731b73..01b58b65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ separate changelogs for each crate were used. If you need to refer to these old ### Added -- `libcnb-package`: Add cross-compilation assistance for Linux `aarch64-unknown-linux-musl`. ([#577](https://github.com/heroku/libcnb.rs/pull/577)) +- `libcnb-package`: + - Add cross-compilation assistance for Linux `aarch64-unknown-linux-musl`. ([#577](https://github.com/heroku/libcnb.rs/pull/577)) + - Added the `output::BuildpackOutputDirectoryLocator` struct which contains information on how compiled buildpack directories are structured and provides a `.get(buildpack_id)` method which produces the output path for a buildpack. ([#632](https://github.com/heroku/libcnb.rs/pull/632)) ### Changed @@ -19,10 +21,18 @@ separate changelogs for each crate were used. If you need to refer to these old - `TestRunner::new` has been removed, since its only purpose was for advanced configuration that's no longer applicable. Use `TestRunner::default` instead. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) - `LogOutput` no longer exposes `stdout_raw` and `stderr_raw`. ([#607](https://github.com/heroku/libcnb.rs/pull/607)) - Improved wording of panic error messages. ([#619](https://github.com/heroku/libcnb.rs/pull/619) and [#620](https://github.com/heroku/libcnb.rs/pull/620)) -- `libcnb-package`: buildpack target directory now contains the target triple. Users that implicitly rely on the output directory need to adapt. The output of `cargo libcnb package` will refer to the new locations. ([#580](https://github.com/heroku/libcnb.rs/pull/580)) +- `libcnb-package`: + - buildpack target directory now contains the target triple. Users that implicitly rely on the output directory need to adapt. The output of `cargo libcnb package` will refer to the new locations. ([#580](https://github.com/heroku/libcnb.rs/pull/580)) + - Changed `buildpack_dependency::rewrite_buildpackage_local_dependencies` to accept a `&BuildpackOutputDirectoryLocator` instead of `&HashMap<&BuildpackId, PathBuf>`. ([#632](https://github.com/heroku/libcnb.rs/pull/632)) + - Moved `default_buildpack_directory_name` to `output::default_buildpack_directory_name`. ([#632](https://github.com/heroku/libcnb.rs/pull/632)) - `libherokubuildpack`: Switch the `flate2` decompression backend from `miniz_oxide` to `zlib`. ([#593](https://github.com/heroku/libcnb.rs/pull/593)) - Bump minimum external dependency versions. ([#587](https://github.com/heroku/libcnb.rs/pull/587)) +### Removed + +- `libcnb-package`: + - `get_buildpack_target_dir` has been removed in favor of `output::BuildpackOutputDirectoryLocator` for building output paths to compiled buildpacks. ([#632](https://github.com/heroku/libcnb.rs/pull/632)) + ### Fixed - `libcnb-test`: diff --git a/libcnb-cargo/src/package/command.rs b/libcnb-cargo/src/package/command.rs index 8cfc8523..dd671637 100644 --- a/libcnb-cargo/src/package/command.rs +++ b/libcnb-cargo/src/package/command.rs @@ -1,7 +1,7 @@ use crate::cli::PackageArgs; use crate::package::error::Error; use cargo_metadata::MetadataCommand; -use libcnb_data::buildpack::{BuildpackDescriptor, BuildpackId}; +use libcnb_data::buildpack::BuildpackDescriptor; use libcnb_data::buildpackage::Buildpackage; use libcnb_package::build::build_buildpack_binaries; use libcnb_package::buildpack_dependency::{ @@ -11,16 +11,23 @@ use libcnb_package::buildpack_dependency::{ use libcnb_package::buildpack_package::{read_buildpack_package, BuildpackPackage}; use libcnb_package::cross_compile::{cross_compile_assistance, CrossCompileAssistance}; use libcnb_package::dependency_graph::{create_dependency_graph, get_dependencies}; -use libcnb_package::{ - assemble_buildpack_directory, find_buildpack_dirs, get_buildpack_target_dir, CargoProfile, -}; -use std::collections::HashMap; +use libcnb_package::output::BuildpackOutputDirectoryLocator; +use libcnb_package::{assemble_buildpack_directory, find_buildpack_dirs, CargoProfile}; +use std::ffi::OsString; use std::path::{Path, PathBuf}; use std::process::Command; type Result = std::result::Result; pub(crate) fn execute(args: &PackageArgs) -> Result<()> { + let target_triple = args.target.clone(); + + let cargo_profile = if args.release { + CargoProfile::Release + } else { + CargoProfile::Dev + }; + eprintln!("🔍 Locating buildpacks..."); let current_dir = std::env::current_dir().map_err(Error::GetCurrentDir)?; @@ -47,19 +54,6 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { .collect::>>()?, )?; - let target_directories_index = buildpack_packages - .node_weights() - .map(|buildpack_package| { - let id = buildpack_package.buildpack_id(); - let target_dir = if contains_buildpack_binaries(&buildpack_package.path) { - buildpack_package.path.clone() - } else { - get_buildpack_target_dir(id, &workspace_target_dir, args.release, &args.target) - }; - (id, target_dir) - }) - .collect::>(); - let buildpack_packages_requested = buildpack_packages .node_weights() .filter(|buildpack_package| { @@ -79,13 +73,18 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { let build_order = get_dependencies(&buildpack_packages, &buildpack_packages_requested)?; + let buildpack_output_directory_locator = BuildpackOutputDirectoryLocator::new( + workspace_target_dir, + cargo_profile, + target_triple.clone(), + ); + let lookup_target_dir = |buildpack_package: &BuildpackPackage| { - target_directories_index - .get(&buildpack_package.buildpack_id()) - .ok_or(Error::TargetDirectoryLookup { - buildpack_id: buildpack_package.buildpack_id().clone(), - }) - .map(std::clone::Clone::clone) + if contains_buildpack_binaries(&buildpack_package.path) { + buildpack_package.path.clone() + } else { + buildpack_output_directory_locator.get(buildpack_package.buildpack_id()) + } }; let mut current_count = 1; @@ -95,17 +94,27 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { "📦 [{current_count}/{total_count}] Building {}", buildpack_package.buildpack_id() ); - let target_dir = lookup_target_dir(buildpack_package)?; + let target_dir = lookup_target_dir(buildpack_package); match buildpack_package.buildpack_data.buildpack_descriptor { BuildpackDescriptor::Single(_) => { if contains_buildpack_binaries(&buildpack_package.path) { eprintln!("Not a libcnb.rs buildpack, nothing to compile..."); } else { - package_single_buildpack(buildpack_package, &target_dir, args)?; + package_single_buildpack( + buildpack_package, + &target_dir, + cargo_profile, + &target_triple, + args.no_cross_compile_assistance, + )?; } } BuildpackDescriptor::Meta(_) => { - package_meta_buildpack(buildpack_package, &target_dir, &target_directories_index)?; + package_meta_buildpack( + buildpack_package, + &target_dir, + &buildpack_output_directory_locator, + )?; } } current_count += 1; @@ -115,14 +124,14 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { build_order .into_iter() .map(lookup_target_dir) - .collect::>>()?, + .collect::>(), ); print_requested_buildpack_output_dirs( buildpack_packages_requested .into_iter() .map(lookup_target_dir) - .collect::>>()?, + .collect::>(), ); Ok(()) @@ -131,16 +140,10 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { fn package_single_buildpack( buildpack_package: &BuildpackPackage, target_dir: &Path, - args: &PackageArgs, + cargo_profile: CargoProfile, + target_triple: &str, + no_cross_compile_assistance: bool, ) -> Result<()> { - let cargo_profile = if args.release { - CargoProfile::Release - } else { - CargoProfile::Dev - }; - - let target_triple = &args.target; - let cargo_metadata = MetadataCommand::new() .manifest_path(&buildpack_package.path.join("Cargo.toml")) .exec() @@ -149,25 +152,7 @@ fn package_single_buildpack( source: e, })?; - let cargo_build_env = if args.no_cross_compile_assistance { - vec![] - } else { - eprintln!("Determining automatic cross-compile settings..."); - match cross_compile_assistance(target_triple) { - CrossCompileAssistance::Configuration { cargo_env } => cargo_env, - - CrossCompileAssistance::NoAssistance => { - eprintln!("Could not determine automatic cross-compile settings for target triple {target_triple}."); - eprintln!("This is not an error, but without proper cross-compile settings in your Cargo manifest and locally installed toolchains, compilation might fail."); - eprintln!("To disable this warning, pass --no-cross-compile-assistance."); - vec![] - } - - CrossCompileAssistance::HelpText(help_text) => { - Err(Error::CrossCompilationHelp { message: help_text })? - } - } - }; + let cargo_build_env = get_cargo_build_env(target_triple, no_cross_compile_assistance)?; eprintln!("Building binaries ({target_triple})..."); @@ -202,7 +187,7 @@ fn package_single_buildpack( fn package_meta_buildpack( buildpack_package: &BuildpackPackage, target_dir: &Path, - target_dirs_by_buildpack_id: &HashMap<&BuildpackId, PathBuf>, + buildpack_output_directory_locator: &BuildpackOutputDirectoryLocator, ) -> Result<()> { eprintln!("Writing buildpack directory..."); @@ -223,8 +208,11 @@ fn package_meta_buildpack( .map(|buildpackage_data| &buildpackage_data.buildpackage_descriptor) .ok_or(Error::MissingBuildpackageData) .and_then(|buildpackage| { - rewrite_buildpackage_local_dependencies(buildpackage, target_dirs_by_buildpack_id) - .map_err(std::convert::Into::into) + rewrite_buildpackage_local_dependencies( + buildpackage, + buildpack_output_directory_locator, + ) + .map_err(std::convert::Into::into) }) .and_then(|buildpackage| { rewrite_buildpackage_relative_path_dependencies_to_absolute( @@ -343,3 +331,28 @@ fn contains_buildpack_binaries(dir: &Path) -> bool { .map(|path| dir.join(path)) .all(|path| path.is_file()) } + +fn get_cargo_build_env( + target_triple: &str, + no_cross_compile_assistance: bool, +) -> Result> { + if no_cross_compile_assistance { + Ok(vec![]) + } else { + eprintln!("Determining automatic cross-compile settings..."); + match cross_compile_assistance(target_triple) { + CrossCompileAssistance::Configuration { cargo_env } => Ok(cargo_env), + + CrossCompileAssistance::NoAssistance => { + eprintln!("Could not determine automatic cross-compile settings for target triple {target_triple}."); + eprintln!("This is not an error, but without proper cross-compile settings in your Cargo manifest and locally installed toolchains, compilation might fail."); + eprintln!("To disable this warning, pass --no-cross-compile-assistance."); + Ok(vec![]) + } + + CrossCompileAssistance::HelpText(help_text) => { + Err(Error::CrossCompilationHelp(help_text))? + } + } + } +} diff --git a/libcnb-cargo/src/package/error.rs b/libcnb-cargo/src/package/error.rs index 07939f8e..783332dd 100644 --- a/libcnb-cargo/src/package/error.rs +++ b/libcnb-cargo/src/package/error.rs @@ -27,11 +27,8 @@ pub(crate) enum Error { source: cargo_metadata::Error, }, - #[error("Could not determine a target directory for buildpack with id `{buildpack_id}`")] - TargetDirectoryLookup { buildpack_id: BuildpackId }, - - #[error("{message}")] - CrossCompilationHelp { message: String }, + #[error("{0}")] + CrossCompilationHelp(String), #[error("No environment variable named `CARGO` is set")] GetCargoBin(#[from] std::env::VarError), @@ -96,9 +93,6 @@ pub(crate) enum Error { #[error("Failed to locate buildpack with id `{0}`")] BuildpackPackagesLookup(BuildpackId), - #[error("Failed to lookup target directory for dependency with id `{0}`")] - RewriteLocalDependencyTargetDirectoryLookup(BuildpackId), - #[error("Could not convert path into buildpackage file uri: {0}")] InvalidPathDependency(PathBuf), @@ -207,9 +201,6 @@ impl From> for Error { impl From for Error { fn from(value: RewriteBuildpackageLocalDependenciesError) -> Self { match value { - RewriteBuildpackageLocalDependenciesError::TargetDirectoryLookup(id) => { - Error::RewriteLocalDependencyTargetDirectoryLookup(id) - } RewriteBuildpackageLocalDependenciesError::InvalidDependency(path) => { Error::InvalidPathDependency(path) } diff --git a/libcnb-cargo/tests/test.rs b/libcnb-cargo/tests/test.rs index 378f3ce6..19f43938 100644 --- a/libcnb-cargo/tests/test.rs +++ b/libcnb-cargo/tests/test.rs @@ -1,7 +1,8 @@ use fs_extra::dir::{copy, CopyOptions}; use libcnb_data::buildpack::{BuildpackDescriptor, BuildpackId}; use libcnb_data::buildpack_id; -use libcnb_package::{get_buildpack_target_dir, read_buildpack_data, read_buildpackage_data}; +use libcnb_package::output::BuildpackOutputDirectoryLocator; +use libcnb_package::{read_buildpack_data, read_buildpackage_data, CargoProfile}; use std::env; use std::io::Read; use std::path::PathBuf; @@ -237,12 +238,18 @@ impl BuildpackPackagingTest { } fn target_dir(&self, buildpack_id: BuildpackId) -> PathBuf { - get_buildpack_target_dir( - &buildpack_id, - &self.dir().join("target"), - self.release_build, - &self.target_triple, - ) + let root_dir = self.dir().join("target"); + let cargo_profile = if self.release_build { + CargoProfile::Release + } else { + CargoProfile::Dev + }; + let locator = BuildpackOutputDirectoryLocator::new( + root_dir, + cargo_profile, + self.target_triple.clone(), + ); + locator.get(&buildpack_id) } fn run_libcnb_package(&self) -> TestOutput { diff --git a/libcnb-package/src/buildpack_dependency.rs b/libcnb-package/src/buildpack_dependency.rs index bd85aa8c..268ea9b9 100644 --- a/libcnb-package/src/buildpack_dependency.rs +++ b/libcnb-package/src/buildpack_dependency.rs @@ -1,7 +1,6 @@ +use crate::output::BuildpackOutputDirectoryLocator; use libcnb_data::buildpack::{BuildpackId, BuildpackIdError}; use libcnb_data::buildpackage::{Buildpackage, BuildpackageDependency}; -use std::collections::HashMap; -use std::hash::BuildHasher; use std::path::{Path, PathBuf}; /// Buildpack dependency type @@ -80,9 +79,9 @@ pub fn get_local_buildpackage_dependencies( /// * the given `buildpackage` contains a local dependency with an invalid [`BuildpackId`] /// * there is no entry found in `buildpack_ids_to_target_dir` for a local dependency's [`BuildpackId`] /// * the target path for a local dependency is an invalid URI -pub fn rewrite_buildpackage_local_dependencies( +pub fn rewrite_buildpackage_local_dependencies( buildpackage: &Buildpackage, - buildpack_ids_to_target_dir: &HashMap<&BuildpackId, PathBuf, S>, + buildpack_output_directory_locator: &BuildpackOutputDirectoryLocator, ) -> Result { let local_dependency_to_target_dir = |target_dir: &PathBuf| { BuildpackageDependency::try_from(target_dir.clone()).map_err(|_| { @@ -99,14 +98,10 @@ pub fn rewrite_buildpackage_local_dependencies( BuildpackDependency::External(buildpackage_dependency) => { Ok(buildpackage_dependency) } - BuildpackDependency::Local(buildpack_id, _) => buildpack_ids_to_target_dir - .get(&buildpack_id) - .ok_or( - RewriteBuildpackageLocalDependenciesError::TargetDirectoryLookup( - buildpack_id, - ), - ) - .and_then(local_dependency_to_target_dir), + BuildpackDependency::Local(buildpack_id, _) => { + let output_dir = buildpack_output_directory_locator.get(&buildpack_id); + local_dependency_to_target_dir(&output_dir) + } }) .collect() }) @@ -120,7 +115,6 @@ pub fn rewrite_buildpackage_local_dependencies( /// An error for [`rewrite_buildpackage_local_dependencies`] #[derive(Debug)] pub enum RewriteBuildpackageLocalDependenciesError { - TargetDirectoryLookup(BuildpackId), InvalidDependency(PathBuf), GetBuildpackDependenciesError(BuildpackIdError), } diff --git a/libcnb-package/src/lib.rs b/libcnb-package/src/lib.rs index aafbe88b..f9323960 100644 --- a/libcnb-package/src/lib.rs +++ b/libcnb-package/src/lib.rs @@ -10,9 +10,10 @@ pub mod buildpack_package; pub mod config; pub mod cross_compile; pub mod dependency_graph; +pub mod output; use crate::build::BuildpackBinaries; -use libcnb_data::buildpack::{BuildpackDescriptor, BuildpackId}; +use libcnb_data::buildpack::BuildpackDescriptor; use libcnb_data::buildpackage::Buildpackage; use std::fs; use std::path::{Path, PathBuf}; @@ -204,15 +205,6 @@ fn create_file_symlink, Q: AsRef>( std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()) } -/// Construct a good default filename for a buildpack directory. -/// -/// This function ensures the resulting name is valid and does not contain problematic characters -/// such as `/`. -#[must_use] -pub fn default_buildpack_directory_name(buildpack_id: &BuildpackId) -> String { - buildpack_id.replace('/', "_") -} - /// Recursively walks the file system from the given `start_dir` to locate any folders containing a /// `buildpack.toml` file. /// @@ -252,45 +244,3 @@ pub fn find_buildpack_dirs(start_dir: &Path, ignore: &[PathBuf]) -> std::io::Res find_buildpack_dirs_recursive(start_dir, ignore, &mut buildpack_dirs)?; Ok(buildpack_dirs) } - -/// Provides a standard path to use for storing a compiled buildpack's artifacts. -#[must_use] -pub fn get_buildpack_target_dir( - buildpack_id: &BuildpackId, - target_dir: &Path, - is_release: bool, - target_triple: &str, -) -> PathBuf { - target_dir - .join("buildpack") - .join(target_triple) - .join(if is_release { "release" } else { "debug" }) - .join(default_buildpack_directory_name(buildpack_id)) -} - -#[cfg(test)] -mod tests { - use crate::get_buildpack_target_dir; - use libcnb_data::buildpack_id; - use std::path::PathBuf; - - #[test] - fn test_get_buildpack_target_dir() { - let buildpack_id = buildpack_id!("some-org/with-buildpack"); - let target_dir = PathBuf::from("/target"); - let target_triple = "x86_64-unknown-linux-musl"; - - assert_eq!( - get_buildpack_target_dir(&buildpack_id, &target_dir, false, target_triple), - PathBuf::from( - "/target/buildpack/x86_64-unknown-linux-musl/debug/some-org_with-buildpack" - ) - ); - assert_eq!( - get_buildpack_target_dir(&buildpack_id, &target_dir, true, target_triple), - PathBuf::from( - "/target/buildpack/x86_64-unknown-linux-musl/release/some-org_with-buildpack" - ) - ); - } -} diff --git a/libcnb-package/src/output.rs b/libcnb-package/src/output.rs new file mode 100644 index 00000000..4b4973f6 --- /dev/null +++ b/libcnb-package/src/output.rs @@ -0,0 +1,77 @@ +use crate::CargoProfile; +use libcnb_data::buildpack::BuildpackId; +use std::path::PathBuf; + +pub struct BuildpackOutputDirectoryLocator { + root_dir: PathBuf, + cargo_profile: CargoProfile, + target_triple: String, +} + +impl BuildpackOutputDirectoryLocator { + #[must_use] + pub fn new(root_dir: PathBuf, cargo_profile: CargoProfile, target_triple: String) -> Self { + Self { + root_dir, + cargo_profile, + target_triple, + } + } + + #[must_use] + pub fn get(&self, buildpack_id: &BuildpackId) -> PathBuf { + self.root_dir + .join("buildpack") + .join(&self.target_triple) + .join(match self.cargo_profile { + CargoProfile::Dev => "debug", + CargoProfile::Release => "release", + }) + .join(default_buildpack_directory_name(buildpack_id)) + } +} + +/// Construct a good default filename for a buildpack directory. +/// +/// This function ensures the resulting name is valid and does not contain problematic characters +/// such as `/`. +#[must_use] +pub fn default_buildpack_directory_name(buildpack_id: &BuildpackId) -> String { + buildpack_id.replace('/', "_") +} + +#[cfg(test)] +mod tests { + use crate::output::BuildpackOutputDirectoryLocator; + use crate::CargoProfile; + use libcnb_data::buildpack_id; + use std::path::PathBuf; + + #[test] + fn test_get_buildpack_output_directory_locator() { + let buildpack_id = buildpack_id!("some-org/with-buildpack"); + + assert_eq!( + BuildpackOutputDirectoryLocator { + cargo_profile: CargoProfile::Dev, + target_triple: "x86_64-unknown-linux-musl".to_string(), + root_dir: PathBuf::from("/target") + } + .get(&buildpack_id), + PathBuf::from( + "/target/buildpack/x86_64-unknown-linux-musl/debug/some-org_with-buildpack" + ) + ); + assert_eq!( + BuildpackOutputDirectoryLocator { + cargo_profile: CargoProfile::Release, + target_triple: "x86_64-unknown-linux-musl".to_string(), + root_dir: PathBuf::from("/target") + } + .get(&buildpack_id), + PathBuf::from( + "/target/buildpack/x86_64-unknown-linux-musl/release/some-org_with-buildpack" + ) + ); + } +} From 428122693ec4db70640d3ecb97c35a4a62bc2dd6 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Fri, 11 Aug 2023 16:12:23 -0300 Subject: [PATCH 2/3] Improved output directory helpers In preparation for adding support for testing meta-buildpacks the process of building output directories is being improved in libcnb-package so that they are easier to work with in libcnb-package and libcnb-test. Both the packaging and testing processes will use this new functionality to construct output locations to write compiled buildpacks into as well as rewriting dependency paths to resolved project-level dependencies declared with libcnb:{buildpack_id}. --- libcnb-package/src/buildpack_dependency.rs | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/libcnb-package/src/buildpack_dependency.rs b/libcnb-package/src/buildpack_dependency.rs index 268ea9b9..48c9539a 100644 --- a/libcnb-package/src/buildpack_dependency.rs +++ b/libcnb-package/src/buildpack_dependency.rs @@ -180,11 +180,12 @@ mod tests { get_local_buildpackage_dependencies, rewrite_buildpackage_local_dependencies, rewrite_buildpackage_relative_path_dependencies_to_absolute, }; + use crate::output::BuildpackOutputDirectoryLocator; + use crate::CargoProfile; use libcnb_data::buildpack_id; use libcnb_data::buildpackage::{ Buildpackage, BuildpackageBuildpackReference, BuildpackageDependency, Platform, }; - use std::collections::HashMap; use std::path::PathBuf; #[test] @@ -203,17 +204,19 @@ mod tests { #[test] fn test_rewrite_buildpackage_local_dependencies() { let buildpackage = create_buildpackage(); - let buildpack_id = buildpack_id!("buildpack-id"); - let buildpack_ids_to_target_dir = HashMap::from([( - &buildpack_id, - PathBuf::from("/path/to/target/buildpacks/buildpack-id"), - )]); - let new_buildpackage = - rewrite_buildpackage_local_dependencies(&buildpackage, &buildpack_ids_to_target_dir) - .unwrap(); + let buildpack_output_directory_locator = BuildpackOutputDirectoryLocator::new( + PathBuf::from("/path/to/target"), + CargoProfile::Dev, + "arch".to_string(), + ); + let new_buildpackage = rewrite_buildpackage_local_dependencies( + &buildpackage, + &buildpack_output_directory_locator, + ) + .unwrap(); assert_eq!( new_buildpackage.dependencies[0].uri.to_string(), - "/path/to/target/buildpacks/buildpack-id" + "/path/to/target/buildpack/arch/debug/buildpack-id" ); } From 50a6ee75939b01863c859b0b80f76906f7a3e2d3 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Fri, 18 Aug 2023 15:53:21 -0300 Subject: [PATCH 3/3] addressing PR review comments --- libcnb-cargo/src/package/command.rs | 1 + libcnb-package/src/buildpack_dependency.rs | 12 ++++++------ libcnb-package/src/output.rs | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/libcnb-cargo/src/package/command.rs b/libcnb-cargo/src/package/command.rs index 304ce616..873d5341 100644 --- a/libcnb-cargo/src/package/command.rs +++ b/libcnb-cargo/src/package/command.rs @@ -19,6 +19,7 @@ use std::process::Command; type Result = std::result::Result; +#[allow(clippy::too_many_lines)] pub(crate) fn execute(args: &PackageArgs) -> Result<()> { let target_triple = args.target.clone(); diff --git a/libcnb-package/src/buildpack_dependency.rs b/libcnb-package/src/buildpack_dependency.rs index b4900c69..49368aea 100644 --- a/libcnb-package/src/buildpack_dependency.rs +++ b/libcnb-package/src/buildpack_dependency.rs @@ -179,7 +179,7 @@ mod tests { get_local_buildpackage_dependencies, rewrite_buildpackage_local_dependencies, rewrite_buildpackage_relative_path_dependencies_to_absolute, }; - use crate::output::BuildpackOutputDirectoryLocator; + use crate::output::create_packaged_buildpack_dir_resolver; use crate::CargoProfile; use libcnb_data::buildpack_id; use libcnb_data::buildpackage::{ @@ -203,19 +203,19 @@ mod tests { #[test] fn test_rewrite_buildpackage_local_dependencies() { let buildpackage = create_buildpackage(); - let buildpack_output_directory_locator = BuildpackOutputDirectoryLocator::new( - PathBuf::from("/path/to/target"), + let packaged_buildpack_dir_resolver = create_packaged_buildpack_dir_resolver( + &PathBuf::from("/path/to/target"), CargoProfile::Dev, - "arch".to_string(), + "arch", ); let new_buildpackage = rewrite_buildpackage_local_dependencies( &buildpackage, - &buildpack_output_directory_locator, + &packaged_buildpack_dir_resolver, ) .unwrap(); assert_eq!( new_buildpackage.dependencies[0].uri.to_string(), - "/path/to/target/buildpack/arch/debug/buildpack-id" + "/path/to/target/arch/debug/buildpack-id" ); } diff --git a/libcnb-package/src/output.rs b/libcnb-package/src/output.rs index 162365c6..17d55272 100644 --- a/libcnb-package/src/output.rs +++ b/libcnb-package/src/output.rs @@ -18,7 +18,7 @@ pub fn create_packaged_buildpack_dir_resolver( CargoProfile::Dev => "debug", CargoProfile::Release => "release", }) - .join(default_buildpack_directory_name(&buildpack_id)) + .join(default_buildpack_directory_name(buildpack_id)) } }