Skip to content

Commit

Permalink
Fix colour resetting for the log_* macros
Browse files Browse the repository at this point in the history
This fixes some long-standing ANSI colour bugs with the `log_header`,
`log_error` and `log_warning` macros. Whilst we soon want to move away
to more advanced logging libraries that use the new logging style, there
are still many buildpacks using these macros which will benefit short
term from these fixes (Procfile, Go, Node.js, JVM, Python, PHP,
buildpacks-release-phase, buildpacks-frontend-web).

The logging macros would previously emit output roughly like:

```
<colour>
[Error: Foo]
<reset><colour>Message line one.
Message line two.
```

This was not only missing the final `<reset>`, but also didn't wrap
each line individually with colour codes/resets.

This causes issues when lines end up prefixed - such as the Git
`remote:` prefix, or when using Pack CLI locally with an untrusted build
(which adds the colourised `[builder]` prefixes) or `--timestamps` mode.

For example in this:

```
remote: <colour>
remote: [Error: Foo]
remote: <reset><colour>Message line one.
remote: Message line two.
```

...several of the `remote:`s would inherit the colours.

Instead what we need is:

```
remote:
remote: <colour>[Error: Foo]<reset>
remote: <colour>Message line one.<reset>
remote: <colour>Message line two.<reset>
```

Fixes #555.
Closes #844.
  • Loading branch information
edmorley committed Dec 10, 2024
1 parent 711fc1b commit 4367854
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 25 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- `libherokubuildpack`:
- Fixed `log_header`, `log_error` and `log_warning` to correctly reset the ANSI colour styles at the end of each line. ([#890](https://github.com/heroku/libcnb.rs/pull/890))


## [0.26.0] - 2024-11-18

### Changed

- `libherokubuildpack`:
- Removed `buildpack_output` module. This functionality from ([#721](https://github.com/heroku/libcnb.rs/pull/721)) was experimental. The API was not stable and it is being removed. A similar API is available at [bullet_stream](https://crates.io/crates/bullet_stream). ([#852](https://github.com/heroku/libcnb.rs/pull/852)
- Removed `buildpack_output` module. This functionality from ([#721](https://github.com/heroku/libcnb.rs/pull/721)) was experimental. The API was not stable and it is being removed. A similar API is available at [bullet_stream](https://crates.io/crates/bullet_stream). ([#852](https://github.com/heroku/libcnb.rs/pull/852))

## [0.25.0] - 2024-10-23

Expand Down
69 changes: 45 additions & 24 deletions libherokubuildpack/src/log.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::io::Write;
use std::io::{self, Write};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

/// # Panics
Expand All @@ -10,16 +10,14 @@ use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
#[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();
write_styled_message(
&mut stream,
format!("\n[Error: {}]", header.as_ref()),
ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true),
)
.unwrap();

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

Expand All @@ -32,16 +30,19 @@ pub fn log_error(header: impl AsRef<str>, body: impl AsRef<str>) {
#[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();
write_styled_message(
&mut stream,
format!("\n[Warning: {}]", header.as_ref()),
ColorSpec::new().set_fg(Some(Color::Yellow)).set_bold(true),
)
.unwrap();

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

Expand All @@ -54,11 +55,12 @@ pub fn log_warning(header: impl AsRef<str>, body: impl AsRef<str>) {
#[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();
write_styled_message(
&mut stream,
format!("\n[{}]", title.as_ref()),
ColorSpec::new().set_fg(Some(Color::Magenta)).set_bold(true),
)
.unwrap();
stream.flush().unwrap();
}

Expand All @@ -72,3 +74,22 @@ pub fn log_info(message: impl AsRef<str>) {
println!("{}", message.as_ref());
std::io::stdout().flush().unwrap();
}

// Styles each line of text separately, so that when buildpack output is streamed to the
// user (and prefixes like `remote:` added) the line colour doesn't leak into the prefixes.
fn write_styled_message(
stream: &mut StandardStream,
message: impl AsRef<str>,
spec: &ColorSpec,
) -> io::Result<()> {
// Using `.split('\n')` rather than `.lines()` since the latter eats trailing newlines in
// the passed message, which would (a) prevent the caller from being able to add spacing at
// the end of their message and (b) differ from the `println!` / `writeln!` behaviour.
for line in message.as_ref().split('\n') {
stream.set_color(spec)?;
write!(stream, "{line}")?;
stream.reset()?;
writeln!(stream)?;
}
Ok(())
}

0 comments on commit 4367854

Please sign in to comment.