Skip to content

Commit 4db7e33

Browse files
authored
libherokubuildpack: Fix colour resetting for the log_* macros (#890)
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. GUS-W-17400078.
1 parent 425f2cf commit 4db7e33

File tree

2 files changed

+50
-25
lines changed

2 files changed

+50
-25
lines changed

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
## [Unreleased]
1111

12+
### Fixed
13+
14+
- `libherokubuildpack`:
15+
- 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))
1216

1317
## [0.26.0] - 2024-11-18
1418

1519
### Changed
1620

1721
- `libherokubuildpack`:
18-
- 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)
22+
- 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))
1923

2024
## [0.25.0] - 2024-10-23
2125

libherokubuildpack/src/log.rs

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::io::Write;
1+
use std::io::{self, Write};
22
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
33

44
/// # Panics
@@ -10,16 +10,14 @@ use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
1010
#[allow(clippy::unwrap_used)]
1111
pub fn log_error(header: impl AsRef<str>, body: impl AsRef<str>) {
1212
let mut stream = StandardStream::stderr(ColorChoice::Always);
13-
stream
14-
.set_color(ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true))
15-
.unwrap();
16-
writeln!(&mut stream, "\n[Error: {}]", header.as_ref()).unwrap();
17-
stream.reset().unwrap();
13+
write_styled_message(
14+
&mut stream,
15+
format!("\n[Error: {}]", header.as_ref()),
16+
ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true),
17+
)
18+
.unwrap();
1819

19-
stream
20-
.set_color(ColorSpec::new().set_fg(Some(Color::Red)))
21-
.unwrap();
22-
writeln!(&mut stream, "{}", body.as_ref()).unwrap();
20+
write_styled_message(&mut stream, body, ColorSpec::new().set_fg(Some(Color::Red))).unwrap();
2321
stream.flush().unwrap();
2422
}
2523

@@ -32,16 +30,19 @@ pub fn log_error(header: impl AsRef<str>, body: impl AsRef<str>) {
3230
#[allow(clippy::unwrap_used)]
3331
pub fn log_warning(header: impl AsRef<str>, body: impl AsRef<str>) {
3432
let mut stream = StandardStream::stderr(ColorChoice::Always);
35-
stream
36-
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)).set_bold(true))
37-
.unwrap();
38-
writeln!(&mut stream, "\n[Warning: {}]", header.as_ref()).unwrap();
39-
stream.reset().unwrap();
33+
write_styled_message(
34+
&mut stream,
35+
format!("\n[Warning: {}]", header.as_ref()),
36+
ColorSpec::new().set_fg(Some(Color::Yellow)).set_bold(true),
37+
)
38+
.unwrap();
4039

41-
stream
42-
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
43-
.unwrap();
44-
writeln!(&mut stream, "{}", body.as_ref()).unwrap();
40+
write_styled_message(
41+
&mut stream,
42+
body,
43+
ColorSpec::new().set_fg(Some(Color::Yellow)),
44+
)
45+
.unwrap();
4546
stream.flush().unwrap();
4647
}
4748

@@ -54,11 +55,12 @@ pub fn log_warning(header: impl AsRef<str>, body: impl AsRef<str>) {
5455
#[allow(clippy::unwrap_used)]
5556
pub fn log_header(title: impl AsRef<str>) {
5657
let mut stream = StandardStream::stdout(ColorChoice::Always);
57-
stream
58-
.set_color(ColorSpec::new().set_fg(Some(Color::Magenta)).set_bold(true))
59-
.unwrap();
60-
writeln!(&mut stream, "\n[{}]", title.as_ref()).unwrap();
61-
stream.reset().unwrap();
58+
write_styled_message(
59+
&mut stream,
60+
format!("\n[{}]", title.as_ref()),
61+
ColorSpec::new().set_fg(Some(Color::Magenta)).set_bold(true),
62+
)
63+
.unwrap();
6264
stream.flush().unwrap();
6365
}
6466

@@ -72,3 +74,22 @@ pub fn log_info(message: impl AsRef<str>) {
7274
println!("{}", message.as_ref());
7375
std::io::stdout().flush().unwrap();
7476
}
77+
78+
// Styles each line of text separately, so that when buildpack output is streamed to the
79+
// user (and prefixes like `remote:` added) the line colour doesn't leak into the prefixes.
80+
fn write_styled_message(
81+
stream: &mut StandardStream,
82+
message: impl AsRef<str>,
83+
spec: &ColorSpec,
84+
) -> io::Result<()> {
85+
// Using `.split('\n')` rather than `.lines()` since the latter eats trailing newlines in
86+
// the passed message, which would (a) prevent the caller from being able to add spacing at
87+
// the end of their message and (b) differ from the `println!` / `writeln!` behaviour.
88+
for line in message.as_ref().split('\n') {
89+
stream.set_color(spec)?;
90+
write!(stream, "{line}")?;
91+
stream.reset()?;
92+
writeln!(stream)?;
93+
}
94+
Ok(())
95+
}

0 commit comments

Comments
 (0)