Skip to content

Commit

Permalink
Revert removing log and changes to on_error
Browse files Browse the repository at this point in the history
We still want the `log` feature to exist until all buildpacks have
transitioned over to `build_output`, since:
1. It will ease the transition for repositories where there are
   multiple buildpacks, since each buildpack can be migrated
   to `build_output` individually, rather than needing to do
   all of them at once.
2. It means any buildpacks that cannot switch to `build_output`
   right away, can still stay up to date with other libcnb changes.

The change to `on_error` has also been reverted, since:
1. It's a change that's standalone from introducing the
   `build_output` module (ie: lets first introduce a new thing,
   then afterwards handle migrating things to use it)
2. It's otherwise yet another thing we need to review in this
   already very-long lived PR, and it would be best to avoid
   further delays. (I can see at least one issue with the change
   that would need fixing, and I haven't even reviewed it fully
  yet.)
  • Loading branch information
edmorley committed Feb 13, 2024
1 parent 55868b3 commit 1da805e
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 16 deletions.
5 changes: 0 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- `libherokubuildpack`:
- Removed the `log` module in favor of `buildpack_output`. ([#721](https://github.com/heroku/libcnb.rs/pull/721))

### Added

- `libherokubuildpack`:
Expand Down
6 changes: 4 additions & 2 deletions libherokubuildpack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ all-features = true
workspace = true

[features]
default = ["command", "download", "digest", "tar", "toml", "fs", "write", "buildpack_output"]
default = ["command", "download", "digest", "error", "log", "tar", "toml", "fs", "write", "buildpack_output"]
download = ["dep:ureq", "dep:thiserror"]
digest = ["dep:sha2"]
error = ["buildpack_output", "dep:libcnb"]
error = ["log", "dep:libcnb"]
log = ["dep:termcolor"]
tar = ["dep:tar", "dep:flate2"]
toml = ["dep:toml"]
fs = ["dep:pathdiff"]
Expand All @@ -41,6 +42,7 @@ libcnb = { workspace = true, optional = true }
pathdiff = { version = "0.2.1", optional = true }
sha2 = { version = "0.10.8", optional = true }
tar = { version = "0.4.40", default-features = false, optional = true }
termcolor = { version = "1.4.1", optional = true }
thiserror = { version = "1.0.57", optional = true }
toml = { workspace = true, optional = true }
ureq = { version = "2.9.5", default-features = false, features = ["tls"], optional = true }
Expand Down
15 changes: 6 additions & 9 deletions libherokubuildpack/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::buildpack_output::BuildpackOutput;
use crate::log::log_error;
use std::fmt::Debug;

/// Handles a given [`libcnb::Error`] in a consistent style.
///
/// This function is intended to be used inside [`libcnb::Buildpack::on_error`].
///
/// It outputs generic libcnb errors in a consistent style using [`BuildpackOutput`] from this
/// It outputs generic libcnb errors in a consistent style using the [logging functions](log_error) from this
/// crate. Buildpack specific errors are handled by the passed custom handler.
///
/// # Example:
Expand All @@ -14,7 +14,7 @@ use std::fmt::Debug;
/// use libcnb::Buildpack;
/// use libcnb::detect::{DetectContext, DetectResult};
/// use libcnb::generic::{GenericMetadata, GenericPlatform};
/// use libherokubuildpack::buildpack_output::BuildpackOutput;
/// use libherokubuildpack::log::log_error;
/// use libherokubuildpack::error::on_error;
///
/// #[derive(Debug)]
Expand All @@ -24,13 +24,12 @@ use std::fmt::Debug;
/// }
///
/// fn on_foo_buildpack_error(e: FooBuildpackError) {
/// let output = BuildpackOutput::new(std::io::stdout()).start_silent();
/// match e {
/// FooBuildpackError::InvalidFooDescriptorToml => {
/// output.error("Invalid foo.toml\n\nYour app's foo.toml is invalid!");
/// log_error("Invalid foo.toml", "Your app's foo.toml is invalid!");
/// }
/// FooBuildpackError::CannotExecuteFooBuildTool(inner) => {
/// output.error(format!("Couldn't execute foo build tool\n\nYour app's foo.toml is invalid!\n\nCause: {}", &inner));
/// log_error("Couldn't execute foo build tool", format!("Cause: {}", &inner));
/// }
/// }
/// }
Expand Down Expand Up @@ -64,9 +63,7 @@ where
match error {
libcnb::Error::BuildpackError(buildpack_error) => f(buildpack_error),
libcnb_error => {
BuildpackOutput::new(std::io::stdout())
.start_silent()
.error(format!("Internal Buildpack Error\n\n{libcnb_error}"));
log_error("Internal Buildpack Error", libcnb_error.to_string());
}
}
}
2 changes: 2 additions & 0 deletions libherokubuildpack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ pub mod download;
pub mod error;
#[cfg(feature = "fs")]
pub mod fs;
#[cfg(feature = "log")]
pub mod log;
#[cfg(feature = "tar")]
pub mod tar;
#[cfg(feature = "toml")]
Expand Down
74 changes: 74 additions & 0 deletions libherokubuildpack/src/log.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use std::io::Write;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

/// # Panics
///
/// Will panic if there was a problem setting the color settings, or all bytes could
/// not be written due to either I/O errors or EOF being reached.
// TODO: Replace `.unwrap()` usages with `.expect()` to give a clearer error message:
// https://github.com/heroku/libcnb.rs/issues/712
#[allow(clippy::unwrap_used)]
pub fn log_error(header: impl AsRef<str>, body: impl AsRef<str>) {
let mut stream = StandardStream::stderr(ColorChoice::Always);
stream
.set_color(ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true))
.unwrap();
writeln!(&mut stream, "\n[Error: {}]", header.as_ref()).unwrap();
stream.reset().unwrap();

stream
.set_color(ColorSpec::new().set_fg(Some(Color::Red)))
.unwrap();
writeln!(&mut stream, "{}", body.as_ref()).unwrap();
stream.flush().unwrap();
}

/// # Panics
///
/// Will panic if there was a problem setting the color settings, or all bytes could
/// not be written due to either I/O errors or EOF being reached.
// TODO: Replace `.unwrap()` usages with `.expect()` to give a clearer error message:
// https://github.com/heroku/libcnb.rs/issues/712
#[allow(clippy::unwrap_used)]
pub fn log_warning(header: impl AsRef<str>, body: impl AsRef<str>) {
let mut stream = StandardStream::stderr(ColorChoice::Always);
stream
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)).set_bold(true))
.unwrap();
writeln!(&mut stream, "\n[Warning: {}]", header.as_ref()).unwrap();
stream.reset().unwrap();

stream
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
.unwrap();
writeln!(&mut stream, "{}", body.as_ref()).unwrap();
stream.flush().unwrap();
}

/// # Panics
///
/// Will panic if there was a problem setting the color settings, or all bytes could
/// not be written due to either I/O errors or EOF being reached.
// TODO: Replace `.unwrap()` usages with `.expect()` to give a clearer error message:
// https://github.com/heroku/libcnb.rs/issues/712
#[allow(clippy::unwrap_used)]
pub fn log_header(title: impl AsRef<str>) {
let mut stream = StandardStream::stdout(ColorChoice::Always);
stream
.set_color(ColorSpec::new().set_fg(Some(Color::Magenta)).set_bold(true))
.unwrap();
writeln!(&mut stream, "\n[{}]", title.as_ref()).unwrap();
stream.reset().unwrap();
stream.flush().unwrap();
}

/// # Panics
///
/// Will panic if all bytes could not be written due to I/O errors or EOF being reached.
// TODO: Replace `.unwrap()` usages with `.expect()` to give a clearer error message:
// https://github.com/heroku/libcnb.rs/issues/712
#[allow(clippy::unwrap_used)]
pub fn log_info(message: impl AsRef<str>) {
println!("{}", message.as_ref());
std::io::stdout().flush().unwrap();
}

0 comments on commit 1da805e

Please sign in to comment.