Skip to content

Commit

Permalink
libcnb-package: Only expose functions/types publicly if they are actu…
Browse files Browse the repository at this point in the history
…ally used (#750)

In order to:
- make the implementation of `libcnb-package` easier to reason about/refactor
- reduce the number of APIs visible to consumers of `libcnb-package`, so it's
  easier for them to see which APIs they should be using

In the case of `calculate_dir_size`, I've moved it back to `libcnb-cargo`
(where it was originally), since it's only used there.

I've checked that none of these APIs are used by our release automation:
https://github.com/search?q=repo%3Aheroku%2Flanguages-github-actions+libcnb_package+language%3ARust&type=code&l=Rust
(And longer term we should switch to having that automation call
a new `libcnb-cargo` subcommand rather than use `libcnb-package`
directly.)

I've not added a changelog entry, since we document `libcnb-package` as being
for internal use only and having no stability guarantees:
https://github.com/heroku/libcnb.rs/blob/main/libcnb-package/README.md

GUS-W-14512388.
  • Loading branch information
edmorley authored Nov 17, 2023
1 parent 9db3aa6 commit d72f9f2
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 40 deletions.
23 changes: 22 additions & 1 deletion libcnb-cargo/src/package/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use libcnb_package::buildpack_dependency_graph::build_libcnb_buildpacks_dependen
use libcnb_package::cross_compile::{cross_compile_assistance, CrossCompileAssistance};
use libcnb_package::dependency_graph::get_dependencies;
use libcnb_package::output::create_packaged_buildpack_dir_resolver;
use libcnb_package::util::{absolutize_path, calculate_dir_size};
use libcnb_package::util::absolutize_path;
use libcnb_package::{find_cargo_workspace_root_dir, CargoProfile};
use std::collections::BTreeMap;
use std::fs;
Expand Down Expand Up @@ -170,3 +170,24 @@ fn eprint_compiled_buildpack_success(current_dir: &Path, target_dir: &Path) {
relative_output_path.to_string_lossy(),
);
}

/// Recursively calculate the size of a directory and its contents in bytes.
fn calculate_dir_size(path: impl AsRef<Path>) -> std::io::Result<u64> {
let mut size_in_bytes = 0;

// The size of the directory entry (ie: its metadata only, not the directory contents).
size_in_bytes += path.as_ref().metadata()?.len();

for entry in std::fs::read_dir(&path)? {
let entry = entry?;
let metadata = entry.metadata()?;

if metadata.is_dir() {
size_in_bytes += calculate_dir_size(entry.path())?;
} else {
size_in_bytes += metadata.len();
}
}

Ok(size_in_bytes)
}
10 changes: 5 additions & 5 deletions libcnb-package/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::process::{Command, ExitStatus};
///
/// Will return `Err` if any build did not finish successfully, the configuration can't be
/// read or the configured main buildpack binary does not exist.
pub fn build_buildpack_binaries(
pub(crate) fn build_buildpack_binaries(
project_path: impl AsRef<Path>,
cargo_metadata: &Metadata,
cargo_profile: CargoProfile,
Expand Down Expand Up @@ -95,7 +95,7 @@ pub fn build_buildpack_binaries(
/// # Errors
///
/// Will return `Err` if the build did not finish successfully.
pub fn build_binary(
fn build_binary(
project_path: impl AsRef<Path>,
cargo_metadata: &Metadata,
cargo_profile: CargoProfile,
Expand Down Expand Up @@ -158,11 +158,11 @@ pub fn build_binary(
}

#[derive(Debug)]
pub struct BuildpackBinaries {
pub(crate) struct BuildpackBinaries {
/// The path to the main buildpack binary
pub buildpack_target_binary_path: PathBuf,
pub(crate) buildpack_target_binary_path: PathBuf,
/// Paths to additional binaries from the buildpack
pub additional_target_binary_paths: HashMap<String, PathBuf>,
pub(crate) additional_target_binary_paths: HashMap<String, PathBuf>,
}

#[derive(thiserror::Error, Debug)]
Expand Down
4 changes: 2 additions & 2 deletions libcnb-package/src/buildpack_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use libcnb_data::buildpack::BuildpackDescriptor;
use std::path::Path;

#[must_use]
pub fn determine_buildpack_kind(buildpack_dir: &Path) -> Option<BuildpackKind> {
pub(crate) fn determine_buildpack_kind(buildpack_dir: &Path) -> Option<BuildpackKind> {
read_toml_file::<BuildpackDescriptor>(buildpack_dir.join("buildpack.toml"))
.ok()
.map(|buildpack_descriptor| match buildpack_descriptor {
Expand All @@ -18,7 +18,7 @@ pub fn determine_buildpack_kind(buildpack_dir: &Path) -> Option<BuildpackKind> {
})
}

pub enum BuildpackKind {
pub(crate) enum BuildpackKind {
Composite,
LibCnbRs,
Other,
Expand Down
6 changes: 2 additions & 4 deletions libcnb-package/src/dependency_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use petgraph::Graph;
use std::error::Error;

/// A node of a dependency graph.
///
/// See: [`create_dependency_graph`]
pub trait DependencyNode<T, E>
where
T: PartialEq,
Expand All @@ -25,7 +23,7 @@ where
///
/// Will return an `Err` if the graph contains references to missing dependencies or the
/// dependencies of a [`DependencyNode`] couldn't be gathered.
pub fn create_dependency_graph<T, I, E>(
pub(crate) fn create_dependency_graph<T, I, E>(
nodes: Vec<T>,
) -> Result<Graph<T, ()>, CreateDependencyGraphError<I, E>>
where
Expand Down Expand Up @@ -59,7 +57,7 @@ where
Ok(graph)
}

/// An error from [`create_dependency_graph`]
/// An error that occurred while creating the dependency graph.
#[derive(thiserror::Error, Debug)]
pub enum CreateDependencyGraphError<I, E: Error> {
#[error("Error while determining dependencies of a node: {0}")]
Expand Down
2 changes: 1 addition & 1 deletion libcnb-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub enum CargoProfile {
/// # Errors
///
/// Will return `Err` if the buildpack directory couldn't be assembled.
pub fn assemble_buildpack_directory(
fn assemble_buildpack_directory(
destination_path: impl AsRef<Path>,
buildpack_descriptor_path: impl AsRef<Path>,
buildpack_binaries: &BuildpackBinaries,
Expand Down
2 changes: 1 addition & 1 deletion libcnb-package/src/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum PackageBuildpackError {
/// # Errors
///
/// Returns `Err` if compilation or packaging failed.
pub fn package_libcnb_buildpack(
fn package_libcnb_buildpack(
buildpack_directory: &Path,
cargo_profile: CargoProfile,
target_triple: &str,
Expand Down
27 changes: 1 addition & 26 deletions libcnb-package/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,5 @@
use std::path::{Component, Path, PathBuf};

/// Recursively calculate the size of a directory and its contents in bytes.
///
/// # Errors
///
/// Returns `Err` if an I/O error occurred during the size calculation.
pub fn calculate_dir_size(path: impl AsRef<Path>) -> std::io::Result<u64> {
let mut size_in_bytes = 0;

// The size of the directory entry (ie: its metadata only, not the directory contents).
size_in_bytes += path.as_ref().metadata()?.len();

for entry in std::fs::read_dir(&path)? {
let entry = entry?;
let metadata = entry.metadata()?;

if metadata.is_dir() {
size_in_bytes += calculate_dir_size(entry.path())?;
} else {
size_in_bytes += metadata.len();
}
}

Ok(size_in_bytes)
}

#[must_use]
pub fn absolutize_path(path: &Path, parent: &Path) -> PathBuf {
if path.is_relative() {
Expand All @@ -40,7 +15,7 @@ pub fn absolutize_path(path: &Path, parent: &Path) -> PathBuf {
/// symbolic links will not be resolved. In return, it can be used before creating a path on the
/// file system.
#[must_use]
pub fn normalize_path(path: &Path) -> PathBuf {
fn normalize_path(path: &Path) -> PathBuf {
let mut components = path.components().peekable();

let mut result = if let Some(component @ Component::Prefix(..)) = components.peek().copied() {
Expand Down

0 comments on commit d72f9f2

Please sign in to comment.