Skip to content

Commit

Permalink
Add support for composite buildpack targets during publication (#221)
Browse files Browse the repository at this point in the history
This PR reads `[[metadata.targets]]` from a composite buildpack's `buildpack.toml` to determine what targets the composite buildpack is intended to be compiled for.

### Why `[[metadata.targets]]`? 

We're not actually sure if `[[targets]]` is legal as a part of a composite buildpack descriptor, so libcnb.rs has not modeled it as noted here:

https://github.com/heroku/libcnb.rs/blob/a4e48fde0b502eec1ce1d0b811bb1ac61beed101/libcnb-data/src/buildpack/mod.rs#L182-L185

The multi-platform rfc seems to indicate that this will eventually be part of `package.toml`:

https://github.com/buildpacks/rfcs/blob/abd3ba28d7648e03a9c2074a88dae9fe3b069683/text/0000-multiarch-builders-and-package.md?plain=1#L754-L776

But `package.toml` is not a part of the spec, and this field isn't implemented in `libcnb.rs` or `pack` yet. Using it now would break `cargo libcnb package` workflows. Setting `[[targets]]` in `package.toml` also seems like something that is an output of `cargo libcnb package`, rather than an input to it.

I don't think `metadata.targets` will be permanent, but rather a stopgap until multi-arch support lands in `pack` and `libcnb.rs` at a future date.
  • Loading branch information
joshwlewis authored May 9, 2024
1 parent 424d7d6 commit acd9dd0
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ regex = "1"
semver = "1"
serde_json = "1"
thiserror = "1"
toml = "0.8"
toml_edit = "0.22"
uriparse = "0.6"
serde = { version = "1.0.198", features = ["derive"] }

[dev-dependencies]
toml = "0.8"
tempfile = "3.10"
73 changes: 72 additions & 1 deletion src/commands/generate_buildpack_matrix/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::commands::resolve_path;
use crate::github::actions;
use clap::Parser;
use libcnb_data::buildpack::{BuildpackDescriptor, BuildpackId, BuildpackTarget};
use libcnb_data::generic::GenericMetadata;
use libcnb_package::output::{
create_packaged_buildpack_dir_resolver, default_buildpack_directory_name,
};
Expand Down Expand Up @@ -189,7 +190,9 @@ pub(crate) fn read_buildpack_info(
fn read_buildpack_targets(buildpack_descriptor: &BuildpackDescriptor) -> Vec<BuildpackTarget> {
let mut targets = match buildpack_descriptor {
BuildpackDescriptor::Component(descriptor) => descriptor.targets.clone(),
BuildpackDescriptor::Composite(_) => vec![],
BuildpackDescriptor::Composite(descriptor) => {
read_metadata_targets(descriptor.metadata.clone()).unwrap_or_default()
}
};
if targets.is_empty() {
targets.push(BuildpackTarget {
Expand Down Expand Up @@ -289,6 +292,31 @@ fn has_bin_files(buildpack_dir: &Path) -> bool {
.all(|file| buildpack_dir.join("bin").join(file).exists())
}

// Project descriptors for composite buildpacks don't support `[[targets]]`,
// but this project needs a way to determine what targets to package composite
// buildpacks for. This function reads `[[targets]]` out of a project
// descriptor's metadata (which is unrestricted) instead.
fn read_metadata_targets(md: GenericMetadata) -> Option<Vec<BuildpackTarget>> {
let get_toml_string = |table: &toml::Table, key: &str| -> Option<String> {
Some(table.get(key)?.as_str()?.to_string())
};
Some(
md?.get("targets")?
.as_array()?
.iter()
.filter_map(|tgt_value| {
let tgt_table = tgt_value.as_table()?;
Some(BuildpackTarget {
os: get_toml_string(tgt_table, "os"),
arch: get_toml_string(tgt_table, "arch"),
variant: get_toml_string(tgt_table, "variant"),
distros: vec![],
})
})
.collect(),
)
}

#[cfg(test)]
mod tests {
use super::read_buildpack_info;
Expand Down Expand Up @@ -406,4 +434,47 @@ mod tests {
PathBuf::from("./packaged-fake/linux-amd64/release/heroku_fakeymcfakeface")
);
}

#[test]
fn read_composite_buildpack() {
let bp_descriptor: BuildpackDescriptor = toml::from_str(
r#"
api = "0.10"
[buildpack]
id = "heroku/fakeymcfakeface"
version = "3.2.1"
[[order]]
[[order.group]]
id = "heroku/nodejs-engine"
version = "3.0.5"
[[metadata.targets]]
os = "linux"
arch = "amd64"
[[metadata.targets]]
os = "linux"
arch = "arm64"
[metadata.release]
image = { repository = "docker.io/heroku/buildpack-fakey" }
"#,
)
.expect("expected buildpack descriptor to parse");
let package_dir = PathBuf::from("./packaged-fake");
let bp_dir = tempdir().expect("Error creating tempdir");

let bp_info = read_buildpack_info(&bp_descriptor, bp_dir.path(), &package_dir, "1928273")
.expect("Expected to read buildpack info");

assert_eq!(bp_info.buildpack_id, "heroku/fakeymcfakeface");
assert_eq!(bp_info.buildpack_type, BuildpackType::Composite);

assert_eq!(bp_info.targets[0].os, Some("linux".to_string()));
assert_eq!(bp_info.targets[0].arch, Some("amd64".to_string()));
assert_eq!(bp_info.targets[1].arch, Some("arm64".to_string()));
assert_eq!(
bp_info.targets[1].output_dir,
PathBuf::from(
"./packaged-fake/aarch64-unknown-linux-musl/release/heroku_fakeymcfakeface"
)
);
}
}

0 comments on commit acd9dd0

Please sign in to comment.