sysroot: Log locking to stderr, not stdout #1535
Conversation
Otherwise we just completely break machine readability
with `bootc status --format=json`.
(We probably need a full audit to stop using `{,e}println!`
globally)
Signed-off-by: Colin Walters <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly changes the logging of the sysroot lock wait message from stdout to stderr. This is an important fix to prevent breaking machine-readable output formats like JSON. My review includes a suggestion to use a proper logging framework for better maintainability and to update the related documentation to reflect this change.
| } | ||
| if !printed { | ||
| println!("Waiting for sysroot lock..."); | ||
| eprintln!("Waiting for sysroot lock..."); |
There was a problem hiding this comment.
While using eprintln! is a good step to avoid polluting stdout, it would be more idiomatic and maintainable to use a structured logging facade like the log crate (e.g., log::info!("Waiting for sysroot lock...")). This aligns with the goal of auditing println! usage mentioned in the PR description and allows for benefits like configurable log levels.
As a side note, the doc comment for new_from_sysroot on line 35 is now outdated and should be updated to state that the message is printed to standard error.
2 participants
Otherwise we just completely break machine readability with
bootc status --format=json.(We probably need a full audit to stop using
{,e}println!globally)