Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
* updated error message
* using iterator instead of loop for finding buildpack dirs logic
* added a quick test to ensure that ignores work
* updated changelog entry
  • Loading branch information
colincasey committed Sep 14, 2023
1 parent d149b15 commit 83f8f49
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 15 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +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))
- `libcnb-package`
- dropped `ignore` argument from `find_buildpack_dirs` that allowed for paths to be filtered out of the directory traversal. This function will now respect standard ignore files (like `.gitignore`) while traversing.
- When performing buildpack detection, standard ignore files (`.ignore` and `.gitignore`) will be respected

## [0.14.0] - 2023-08-18

Expand Down
41 changes: 41 additions & 0 deletions libcnb-cargo/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,47 @@ 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();

// when a git folder is not present, .ignore should be used
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();

// when a git folder is not present, .gitignore can be used
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());
Expand Down
2 changes: 1 addition & 1 deletion libcnb-package/src/buildpack_dependency_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn build_libcnb_buildpack_dependency_graph_node(

#[derive(thiserror::Error, Debug)]
pub enum BuildBuildpackDependencyGraphError {
#[error("IO error while finding buildpack directories: {0}")]
#[error("Error while finding buildpack directories: {0}")]
FindBuildpackDirectories(ignore::Error),
#[error("Cannot read buildpack descriptor: {0}")]
ReadBuildpackDescriptorError(TomlFileError),
Expand Down
26 changes: 14 additions & 12 deletions libcnb-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,20 @@ fn create_file_symlink<P: AsRef<Path>, Q: AsRef<Path>>(
/// 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<Vec<PathBuf>, ignore::Error> {
let mut buildpack_dirs = vec![];
for result in ignore::Walk::new(start_dir) {
match result {
Ok(entry) => {
if entry.path().is_dir() && entry.path().join("buildpack.toml").exists() {
buildpack_dirs.push(entry.path().to_path_buf());
}
}
Err(error) => return Err(error),
}
}
Ok(buildpack_dirs)
ignore::Walk::new(start_dir)
.collect::<Result<Vec<_>, _>>()
.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
}
})
.collect()
})
}

/// Returns the path of the root workspace directory for a Rust Cargo project. This is often a useful
Expand Down

0 comments on commit 83f8f49

Please sign in to comment.