addressing PR review comments

Author: colincasey

libcnb-cargo/src/package/command.rs

@@ -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, find_cargo_workspace,
+    assemble_buildpack_directory, find_buildpack_dirs, find_cargo_workspace_root_dir,
     get_buildpack_target_dir, CargoProfile,
 };
 use std::collections::HashMap;
@@ -25,7 +25,7 @@ pub(crate) fn execute(args: &PackageArgs) -> Result<()> {
 
     let current_dir = std::env::current_dir().map_err(Error::GetCurrentDir)?;
 
-    let workspace_dir = find_cargo_workspace(&current_dir)?;
+    let workspace_dir = find_cargo_workspace_root_dir(&current_dir)?;
 
     let output_dir = get_buildpack_output_dir(&workspace_dir)?;
 

libcnb-cargo/src/package/error.rs

@@ -50,8 +50,8 @@ pub(crate) enum Error {
         source: std::io::Error,
     },
 
-    #[error("Unexpected Cargo exit status for target {target}\nExit Status: {code}\nExamine Cargo output for details and potential compilation errors.")]
-    BinaryBuildExitStatus { target: String, code: String },
+    #[error("Unexpected Cargo exit status for target {0}\nExit Status: {}\nExamine Cargo output for details and potential compilation errors.", .exit_code_or_unknown(.1))]
+    BinaryBuildExitStatus(String, ExitStatus),
 
     #[error("Configured buildpack target name {target} could not be found!")]
     BinaryBuildMissingTarget { target: String },
@@ -126,8 +126,8 @@ pub(crate) enum Error {
     #[error("Failed to spawn Cargo command\nError: {0}")]
     SpawnCargoCommand(std::io::Error),
 
-    #[error("Unexpected Cargo exit status while attempting to read workspace root\nExit Status: {0}\nExamine Cargo output for details and potential compilation errors.")]
-    CargoCommandFailure(String),
+    #[error("Unexpected Cargo exit status while attempting to read workspace root\nExit Status: {}\nExamine Cargo output for details and potential compilation errors.", exit_code_or_unknown(.0))]
+    CargoCommandFailure(ExitStatus),
 
     #[error("Could not read Cargo.toml metadata from workspace\nPath: {0}\nError: {1}")]
     GetBuildpackOutputDir(PathBuf, cargo_metadata::Error),
@@ -145,10 +145,7 @@ impl From<BuildBinariesError> for Error {
             BuildBinariesError::BuildError(
                 target,
                 BuildError::UnexpectedCargoExitStatus(exit_status),
-            ) => Error::BinaryBuildExitStatus {
-                target,
-                code: exit_status_or_unknown(exit_status),
-            },
+            ) => Error::BinaryBuildExitStatus(target, exit_status),
 
             BuildBinariesError::MissingBuildpackTarget(target) => {
                 Error::BinaryBuildMissingTarget { target }
@@ -235,14 +232,15 @@ impl From<FindCargoWorkspaceError> for Error {
             FindCargoWorkspaceError::GetCargoEnv(error) => Error::GetCargoBin(error),
             FindCargoWorkspaceError::SpawnCommand(error) => Error::SpawnCargoCommand(error),
             FindCargoWorkspaceError::CommandFailure(exit_status) => {
-                Error::CargoCommandFailure(exit_status_or_unknown(exit_status))
+                Error::CargoCommandFailure(exit_status)
             }
             FindCargoWorkspaceError::GetParentDirectory(path) => Error::GetWorkspaceDirectory(path),
         }
     }
 }
 
-fn exit_status_or_unknown(exit_status: ExitStatus) -> String {
+#[allow(clippy::trivially_copy_pass_by_ref)]
+fn exit_code_or_unknown(exit_status: &ExitStatus) -> String {
     exit_status
         .code()
         .map_or_else(|| String::from("<unknown>"), |code| code.to_string())

libcnb-package/src/lib.rs

@@ -278,7 +278,9 @@ pub fn get_buildpack_target_dir(
 /// - no `CARGO` environment variable with the path to the `cargo` binary
 /// - executing this function with a directory that is not within a Cargo project
 /// - any other file or system error that might occur
-pub fn find_cargo_workspace(dir_in_workspace: &Path) -> Result<PathBuf, FindCargoWorkspaceError> {
+pub fn find_cargo_workspace_root_dir(
+    dir_in_workspace: &Path,
+) -> Result<PathBuf, FindCargoWorkspaceError> {
     let cargo_bin = std::env::var("CARGO")
         .map(PathBuf::from)
         .map_err(FindCargoWorkspaceError::GetCargoEnv)?;
@@ -297,6 +299,7 @@ pub fn find_cargo_workspace(dir_in_workspace: &Path) -> Result<PathBuf, FindCarg
         .then_some(output)
         .ok_or(FindCargoWorkspaceError::CommandFailure(status))
         .and_then(|output| {
+            // Cargo outputs a newline after the actual path, so we have to trim.
             let root_cargo_toml = PathBuf::from(String::from_utf8_lossy(&output.stdout).trim());
             root_cargo_toml
                 .parent()