Move packaged buildpack directory out of target/ (#583)

.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/

.gitignore

@@ -1,5 +1,7 @@
 /target/
+/packaged/
 .DS_Store
 .idea
 Cargo.lock
 **/fixtures/*/target/
+**/fixtures/*/packaged/

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
 

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<PathBuf>,
 }
 
 #[cfg(test)]

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(&current_dir)?;
+    let workspace_root_path = get_cargo_workspace_root(&current_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)
         })

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 },
 

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";

libcnb-package/src/lib.rs

@@ -255,42 +255,37 @@ 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))
 }
 
 #[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")
         );
     }
 }