Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move packaged buildpack directory out of target/ #583

Merged
merged 6 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 libcnb-packaged/x86_64-unknown-linux-musl/debug/libcnb-examples_basics --path examples/
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/target/
/libcnb-packaged/
.DS_Store
.idea
Cargo.lock
**/fixtures/*/target/
**/fixtures/*/libcnb-packaged/
edmorley marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ separate changelogs for each crate were used. If you need to refer to these old
- `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))
- `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 `libcnb-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))
edmorley marked this conversation as resolved.
Show resolved Hide resolved

### Fixed

Expand Down
4 changes: 4 additions & 0 deletions libcnb-cargo/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clap::{Parser, Subcommand};
use std::path::PathBuf;

#[derive(Parser)]
#[command(bin_name = "cargo")]
Expand All @@ -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 'libcnb-packaged' in Cargo workspace root
#[arg(long)]
pub package_dir: Option<PathBuf>,
Malax marked this conversation as resolved.
Show resolved Hide resolved
edmorley marked this conversation as resolved.
Show resolved Hide resolved
}

#[cfg(test)]
Expand Down
27 changes: 18 additions & 9 deletions libcnb-cargo/src/package/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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("libcnb-packaged")
Malax marked this conversation as resolved.
Show resolved Hide resolved
});

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,
edmorley marked this conversation as resolved.
Show resolved Hide resolved
source: e,
})?
.into_iter()
Expand All @@ -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)
})
Expand Down
3 changes: 3 additions & 0 deletions libcnb-cargo/src/package/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },

Expand Down
17 changes: 8 additions & 9 deletions libcnb-cargo/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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('/', "_"))
Expand Down Expand Up @@ -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 = "libcnb-packaged";
edmorley marked this conversation as resolved.
Show resolved Hide resolved
Malax marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 9 additions & 14 deletions libcnb-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
edmorley marked this conversation as resolved.
Show resolved Hide resolved
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")
);
}
}