From ec90a43d322715dccc4f0b04ddac15373364b3ef Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 18 Aug 2023 10:44:16 +0200 Subject: [PATCH] Move packaged buildpack directory out of `target/` (#583) --- .github/workflows/ci.yml | 2 +- .gitignore | 2 ++ CHANGELOG.md | 6 +++++- libcnb-cargo/src/cli.rs | 4 ++++ libcnb-cargo/src/package/command.rs | 27 ++++++++++++++++++--------- libcnb-cargo/src/package/error.rs | 3 +++ libcnb-cargo/tests/test.rs | 17 ++++++++--------- libcnb-package/src/lib.rs | 23 +++++++++-------------- 8 files changed, 50 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 999261db..0093d899 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,4 +73,4 @@ jobs: # Uses a non-libc image to validate the static musl cross-compilation. # TODO: Switch this back to using the `alpine` tag once the stable Pack CLI release supports # image extensions (currently newer sample alpine images fail to build with stable Pack). - run: pack build example-basics --builder cnbs/sample-builder@sha256:da5ff69191919f1ff30d5e28859affff8e39f23038137c7751e24a42e919c1ab --trust-builder --buildpack target/buildpack/x86_64-unknown-linux-musl/debug/libcnb-examples_basics --path examples/ + run: pack build example-basics --builder cnbs/sample-builder@sha256:da5ff69191919f1ff30d5e28859affff8e39f23038137c7751e24a42e919c1ab --trust-builder --buildpack packaged/x86_64-unknown-linux-musl/debug/libcnb-examples_basics --path examples/ diff --git a/.gitignore b/.gitignore index c0f3e140..6addfe54 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,7 @@ /target/ +/packaged/ .DS_Store .idea Cargo.lock **/fixtures/*/target/ +**/fixtures/*/packaged/ diff --git a/CHANGELOG.md b/CHANGELOG.md index b3392f50..7c7f7346 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ 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-cargo`: Add `--package-dir` command line option to control where packaged buildpacks are written to. ([#583](https://github.com/heroku/libcnb.rs/pull/583)) - `libcnb-test`: - `LogOutput` now implements `std::fmt::Display`. ([#635](https://github.com/heroku/libcnb.rs/pull/635)) - `ContainerConfig` now implements `Clone`. ([#636](https://github.com/heroku/libcnb.rs/pull/636)) @@ -22,9 +23,12 @@ 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)) + - `get_buildpack_target_dir` was renamed to `get_buildpack_package_dir` ([#583](https://github.com/heroku/libcnb.rs/pull/583)) - `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)) +- `libcnb-cargo`: Default location for packaged buildpacks moved from Cargo's `target` directory to `packaged` in the Cargo workspace root. This simplifies the path and stops modification of the `target` directory which previously might have caching implications when other tools didn't expect non-Cargo output in that directory. Users that implicitly rely on the output directory need to adapt. The output of `cargo libcnb package` will refer to the new locations. ([#583](https://github.com/heroku/libcnb.rs/pull/583)) ### Fixed diff --git a/libcnb-cargo/src/cli.rs b/libcnb-cargo/src/cli.rs index c6f7e64c..86290a0c 100644 --- a/libcnb-cargo/src/cli.rs +++ b/libcnb-cargo/src/cli.rs @@ -1,4 +1,5 @@ use clap::{Parser, Subcommand}; +use std::path::PathBuf; #[derive(Parser)] #[command(bin_name = "cargo")] @@ -25,6 +26,9 @@ pub(crate) struct PackageArgs { /// Build for the target triple #[arg(long, default_value = "x86_64-unknown-linux-musl")] pub target: String, + /// Directory for packaged buildpacks, defaults to 'packaged' in Cargo workspace root + #[arg(long)] + pub package_dir: Option, } #[cfg(test)] diff --git a/libcnb-cargo/src/package/command.rs b/libcnb-cargo/src/package/command.rs index 8cfc8523..d271861e 100644 --- a/libcnb-cargo/src/package/command.rs +++ b/libcnb-cargo/src/package/command.rs @@ -12,7 +12,7 @@ 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, + assemble_buildpack_directory, find_buildpack_dirs, get_buildpack_package_dir, CargoProfile, }; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -25,21 +25,30 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { let current_dir = std::env::current_dir().map_err(Error::GetCurrentDir)?; - let workspace = get_cargo_workspace_root(¤t_dir)?; + let workspace_root_path = get_cargo_workspace_root(¤t_dir)?; - let workspace_target_dir = MetadataCommand::new() - .manifest_path(&workspace.join("Cargo.toml")) + let cargo_metadata = MetadataCommand::new() + .manifest_path(&workspace_root_path.join("Cargo.toml")) .exec() - .map(|metadata| metadata.target_directory.into_std_path_buf()) .map_err(|e| Error::ReadCargoMetadata { - path: workspace.clone(), + path: workspace_root_path.clone(), source: e, })?; + let package_dir = args.package_dir.clone().unwrap_or_else(|| { + cargo_metadata + .workspace_root + .into_std_path_buf() + .join("packaged") + }); + + std::fs::create_dir_all(&package_dir) + .map_err(|e| Error::CreatePackageDirectory(package_dir.clone(), e))?; + let buildpack_packages = create_dependency_graph( - find_buildpack_dirs(&workspace, &[workspace_target_dir.clone()]) + find_buildpack_dirs(&workspace_root_path, &[package_dir.clone()]) .map_err(|e| Error::FindBuildpackDirs { - path: workspace_target_dir.clone(), + path: workspace_root_path, source: e, })? .into_iter() @@ -54,7 +63,7 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> { 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) + get_buildpack_package_dir(id, &package_dir, args.release, &args.target) }; (id, target_dir) }) diff --git a/libcnb-cargo/src/package/error.rs b/libcnb-cargo/src/package/error.rs index 07939f8e..2013c823 100644 --- a/libcnb-cargo/src/package/error.rs +++ b/libcnb-cargo/src/package/error.rs @@ -27,6 +27,9 @@ pub(crate) enum Error { source: cargo_metadata::Error, }, + #[error("Could not create package directory: {0}\nError: {1}")] + CreatePackageDirectory(PathBuf, std::io::Error), + #[error("Could not determine a target directory for buildpack with id `{buildpack_id}`")] TargetDirectoryLookup { buildpack_id: BuildpackId }, diff --git a/libcnb-cargo/tests/test.rs b/libcnb-cargo/tests/test.rs index 10c9c9db..362531ea 100644 --- a/libcnb-cargo/tests/test.rs +++ b/libcnb-cargo/tests/test.rs @@ -25,7 +25,7 @@ fn package_buildpack_in_single_buildpack_project() { .unwrap(); let packaged_buildpack_dir = create_packaged_buildpack_dir_resolver( - &fixture_dir.path().join(TARGET_DIR_NAME), + &fixture_dir.path().join(DEFAULT_PACKAGE_DIR_NAME), true, X86_64_UNKNOWN_LINUX_MUSL, )(&buildpack_id); @@ -50,7 +50,7 @@ fn package_single_meta_buildpack_in_monorepo_buildpack_project() { .unwrap(); let packaged_buildpack_dir_resolver = create_packaged_buildpack_dir_resolver( - &fixture_dir.path().join(TARGET_DIR_NAME), + &fixture_dir.path().join(DEFAULT_PACKAGE_DIR_NAME), true, X86_64_UNKNOWN_LINUX_MUSL, ); @@ -110,7 +110,7 @@ fn package_single_buildpack_in_monorepo_buildpack_project() { .unwrap(); let packaged_buildpack_dir = create_packaged_buildpack_dir_resolver( - &fixture_dir.path().join(TARGET_DIR_NAME), + &fixture_dir.path().join(DEFAULT_PACKAGE_DIR_NAME), true, X86_64_UNKNOWN_LINUX_MUSL, )(&buildpack_id); @@ -140,7 +140,7 @@ fn package_all_buildpacks_in_monorepo_buildpack_project() { .unwrap(); let packaged_buildpack_dir_resolver = create_packaged_buildpack_dir_resolver( - &fixture_dir.path().join(TARGET_DIR_NAME), + &fixture_dir.path().join(DEFAULT_PACKAGE_DIR_NAME), true, X86_64_UNKNOWN_LINUX_MUSL, ); @@ -275,16 +275,15 @@ fn validate_packaged_meta_buildpack( } fn create_packaged_buildpack_dir_resolver( - cargo_target_dir: &Path, + package_dir: &Path, release: bool, target_triple: &str, ) -> impl Fn(&BuildpackId) -> PathBuf { - let cargo_target_dir = PathBuf::from(cargo_target_dir); + let package_dir = PathBuf::from(package_dir); let target_triple = target_triple.to_string(); move |buildpack_id| { - cargo_target_dir - .join("buildpack") + package_dir .join(&target_triple) .join(if release { "release" } else { "debug" }) .join(buildpack_id.as_str().replace('/', "_")) @@ -330,5 +329,5 @@ fn copy_dir_recursively(source: &Path, destination: &Path) -> std::io::Result<() } const X86_64_UNKNOWN_LINUX_MUSL: &str = "x86_64-unknown-linux-musl"; -const TARGET_DIR_NAME: &str = "target"; const CARGO_LIBCNB_BINARY_UNDER_TEST: &str = env!("CARGO_BIN_EXE_cargo-libcnb"); +const DEFAULT_PACKAGE_DIR_NAME: &str = "packaged"; diff --git a/libcnb-package/src/lib.rs b/libcnb-package/src/lib.rs index aafbe88b..07043606 100644 --- a/libcnb-package/src/lib.rs +++ b/libcnb-package/src/lib.rs @@ -255,14 +255,13 @@ pub fn find_buildpack_dirs(start_dir: &Path, ignore: &[PathBuf]) -> std::io::Res /// Provides a standard path to use for storing a compiled buildpack's artifacts. #[must_use] -pub fn get_buildpack_target_dir( +pub fn get_buildpack_package_dir( buildpack_id: &BuildpackId, - target_dir: &Path, + package_dir: &Path, is_release: bool, target_triple: &str, ) -> PathBuf { - target_dir - .join("buildpack") + package_dir .join(target_triple) .join(if is_release { "release" } else { "debug" }) .join(default_buildpack_directory_name(buildpack_id)) @@ -270,27 +269,23 @@ pub fn get_buildpack_target_dir( #[cfg(test)] mod tests { - use crate::get_buildpack_target_dir; + use crate::get_buildpack_package_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 package_dir = PathBuf::from("/package"); 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" - ) + get_buildpack_package_dir(&buildpack_id, &package_dir, false, target_triple), + PathBuf::from("/package/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" - ) + get_buildpack_package_dir(&buildpack_id, &package_dir, true, target_triple), + PathBuf::from("/package/x86_64-unknown-linux-musl/release/some-org_with-buildpack") ); } }