From 31ec5af0d76dc244f09d4b84ca12183477678059 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Tue, 19 Sep 2023 12:00:43 -0300 Subject: [PATCH] Ignore file support when finding buildpacks (#673) * Ignore file support when finding buildpacks Previously we could rely on knowing that packaged buildpacks would be written to a location within the `target` directory configured in Crate. This was easy to determine through program code and could be supplied to `find_buildpack_dirs` so that packaged buildpacks weren't accidentally included when searching for buildpacks. With the changes introduced in [#583](https://github.com/heroku/libcnb.rs/pull/583) it is no longer possible to easily determine the output location for packaged buildpacks. This PR adds the [ignore](https://crates.io/crates/ignore) crate to provide directory iteration that respects standard ignore files. This will give the user the ability to configure where we search within a project for buildpacks. --- CHANGELOG.md | 1 + libcnb-cargo/src/package/command.rs | 5 +- libcnb-cargo/tests/integration_test.rs | 47 ++++++++++++++++++ libcnb-package/Cargo.toml | 1 + .../src/buildpack_dependency_graph.rs | 7 ++- libcnb-package/src/lib.rs | 48 +++++++------------ 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 692dad2d..45c19f93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `libcnb-cargo`: - No longer outputs paths for non-libcnb.rs and non-meta buildpacks. ([#657](https://github.com/heroku/libcnb.rs/pull/657)) - Build output for humans changed slightly, output intended for machines/scripting didn't change. ([#657](https://github.com/heroku/libcnb.rs/pull/657)) + - When performing buildpack detection, standard ignore files (`.ignore` and `.gitignore`) will be respected. ([#673](https://github.com/heroku/libcnb.rs/pull/673)) ## [0.14.0] - 2023-08-18 diff --git a/libcnb-cargo/src/package/command.rs b/libcnb-cargo/src/package/command.rs index 668dbb4b..351e0854 100644 --- a/libcnb-cargo/src/package/command.rs +++ b/libcnb-cargo/src/package/command.rs @@ -61,9 +61,8 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<(), Error> { }; eprintln!("šŸ—ļø Building buildpack dependency graph..."); - let buildpack_dependency_graph = - build_libcnb_buildpacks_dependency_graph(&workspace_root_path, &[&package_dir]) - .map_err(Error::CannotBuildBuildpackDependencyGraph)?; + let buildpack_dependency_graph = build_libcnb_buildpacks_dependency_graph(&workspace_root_path) + .map_err(Error::CannotBuildBuildpackDependencyGraph)?; eprintln!("šŸ”€ Determining build order..."); let root_nodes = buildpack_dependency_graph diff --git a/libcnb-cargo/tests/integration_test.rs b/libcnb-cargo/tests/integration_test.rs index 6abb7cfe..093b07f4 100644 --- a/libcnb-cargo/tests/integration_test.rs +++ b/libcnb-cargo/tests/integration_test.rs @@ -219,6 +219,53 @@ fn package_command_error_when_run_in_project_with_no_buildpacks() { ); } +#[test] +#[ignore = "integration test"] +fn package_command_respects_ignore_files() { + let fixture_dir = copy_fixture_to_temp_dir("multiple_buildpacks").unwrap(); + + // The `ignore` crate supports `.ignore` files. So this first `cargo libcnb package` execution + // just sanity checks that our ignore rules will be respected. + let ignore_file = fixture_dir.path().join(".ignore"); + fs::write(&ignore_file, "meta-buildpacks\nbuildpacks\n").unwrap(); + + let output = Command::new(CARGO_LIBCNB_BINARY_UNDER_TEST) + .args(["libcnb", "package", "--release"]) + .current_dir(fixture_dir.path()) + .output() + .unwrap(); + + assert_ne!(output.status.code(), Some(0)); + assert_eq!( + String::from_utf8_lossy(&output.stderr), + "šŸšš Preparing package directory...\nšŸ–„\u{fe0f} Gathering Cargo configuration (for x86_64-unknown-linux-musl)\nšŸ—\u{fe0f} Building buildpack dependency graph...\nšŸ”€ Determining build order...\nāŒ No buildpacks found!\n" + ); + + fs::remove_file(ignore_file).unwrap(); + + // The `ignore` crate supports `.gitignore` files but only if the folder is within a git repository + // which is the default configuration used in our directory traversal. + // https://docs.rs/ignore/latest/ignore/struct.WalkBuilder.html#method.require_git + // + // So this second `cargo libcnb package` execution just sanity checks that our gitignore rules + // in a git repository will be respected. + fs::create_dir(fixture_dir.path().join(".git")).unwrap(); + let ignore_file = fixture_dir.path().join(".gitignore"); + fs::write(ignore_file, "meta-buildpacks\nbuildpacks\n").unwrap(); + + let output = Command::new(CARGO_LIBCNB_BINARY_UNDER_TEST) + .args(["libcnb", "package", "--release"]) + .current_dir(fixture_dir.path()) + .output() + .unwrap(); + + assert_ne!(output.status.code(), Some(0)); + assert_eq!( + String::from_utf8_lossy(&output.stderr), + "šŸšš Preparing package directory...\nšŸ–„\u{fe0f} Gathering Cargo configuration (for x86_64-unknown-linux-musl)\nšŸ—\u{fe0f} Building buildpack dependency graph...\nšŸ”€ Determining build order...\nāŒ No buildpacks found!\n" + ); +} + fn validate_packaged_buildpack(packaged_buildpack_dir: &Path, buildpack_id: &BuildpackId) { assert!(packaged_buildpack_dir.join("buildpack.toml").exists()); assert!(packaged_buildpack_dir.join("package.toml").exists()); diff --git a/libcnb-package/Cargo.toml b/libcnb-package/Cargo.toml index 2f136e9c..f60779e5 100644 --- a/libcnb-package/Cargo.toml +++ b/libcnb-package/Cargo.toml @@ -13,6 +13,7 @@ include = ["src/**/*", "LICENSE", "README.md"] [dependencies] cargo_metadata = "0.17.0" +ignore = "0.4" libcnb-common.workspace = true libcnb-data.workspace = true petgraph = { version = "0.6.3", default-features = false } diff --git a/libcnb-package/src/buildpack_dependency_graph.rs b/libcnb-package/src/buildpack_dependency_graph.rs index 5021cb4b..d99b35f0 100644 --- a/libcnb-package/src/buildpack_dependency_graph.rs +++ b/libcnb-package/src/buildpack_dependency_graph.rs @@ -27,9 +27,8 @@ use std::path::{Path, PathBuf}; /// package.toml or an IO error occurred while traversing the given directory. pub fn build_libcnb_buildpacks_dependency_graph( cargo_workspace_root: &Path, - ignore: &[&Path], ) -> Result, BuildBuildpackDependencyGraphError> { - find_buildpack_dirs(cargo_workspace_root, ignore) + find_buildpack_dirs(cargo_workspace_root) .map_err(BuildBuildpackDependencyGraphError::FindBuildpackDirectories) .and_then(|buildpack_directories| { buildpack_directories @@ -85,8 +84,8 @@ fn build_libcnb_buildpack_dependency_graph_node( #[derive(thiserror::Error, Debug)] pub enum BuildBuildpackDependencyGraphError { - #[error("IO error while finding buildpack directories: {0}")] - FindBuildpackDirectories(std::io::Error), + #[error("Error while finding buildpack directories: {0}")] + FindBuildpackDirectories(ignore::Error), #[error("Cannot read buildpack descriptor: {0}")] ReadBuildpackDescriptorError(TomlFileError), #[error("Cannot read package descriptor: {0}")] diff --git a/libcnb-package/src/lib.rs b/libcnb-package/src/lib.rs index 1fc77eb1..96aea209 100644 --- a/libcnb-package/src/lib.rs +++ b/libcnb-package/src/lib.rs @@ -106,39 +106,23 @@ fn create_file_symlink, Q: AsRef>( /// /// # Errors /// -/// Will return an `Err` if any I/O errors happen while walking the file system. -pub fn find_buildpack_dirs(start_dir: &Path, ignore: &[&Path]) -> std::io::Result> { - fn find_buildpack_dirs_recursive( - path: &Path, - ignore: &[&Path], - accumulator: &mut Vec, - ) -> std::io::Result<()> { - if ignore.contains(&path) { - return Ok(()); - } - - let metadata = path.metadata()?; - if metadata.is_dir() { - let entries = fs::read_dir(path)?; - for entry in entries { - let entry = entry?; - let metadata = entry.metadata()?; - if metadata.is_dir() { - find_buildpack_dirs_recursive(&entry.path(), ignore, accumulator)?; - } else if let Some(file_name) = entry.path().file_name() { - if file_name.to_string_lossy() == "buildpack.toml" { - accumulator.push(path.to_path_buf()); +/// Will return an `Err` if any I/O errors happen while walking the file system or any parsing errors +/// from reading a gitignore file. +pub fn find_buildpack_dirs(start_dir: &Path) -> Result, ignore::Error> { + ignore::Walk::new(start_dir) + .collect::, _>>() + .map(|entries| { + entries + .iter() + .filter_map(|entry| { + if entry.path().is_dir() && entry.path().join("buildpack.toml").exists() { + Some(entry.path().to_path_buf()) + } else { + None } - } - } - } - - Ok(()) - } - - let mut buildpack_dirs: Vec = vec![]; - find_buildpack_dirs_recursive(start_dir, ignore, &mut buildpack_dirs)?; - Ok(buildpack_dirs) + }) + .collect() + }) } /// Returns the path of the root workspace directory for a Rust Cargo project. This is often a useful