Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add new --level-prefix option #646

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Jul 24, 2024

  • test-run: Assert that repeating --chdir logs a warning

    This is the case since commit 0d369cd "main: Warn when
    non-repeatable options are repeated".

  • utils: Put nearly all diagnostic output through a common log function

    This takes a syslog-style severity level, allowing us to act based on
    the severity.

    Take the opportunity to make the __debug__ macro (which normally expands
    to nothing, but can be enabled by changing a #if 0 to #if 1) less
    weird and easier to use, by taking it out of the reserved-for-the-compiler
    namespace, adding a newline automatically, and not requiring nested
    parentheses.

  • Add new --level-prefix option

    This prepends a syslog-style priority level such as <3> to each line of
    diagnostic output, so that the diagnostic output can be parsed by
    tools like systemd-cat --level-prefix=1. A future version of Steam's
    pressure-vessel is likely to use this to make warnings and fatal errors
    from bubblewrap more visible.

cc @refi64 @RyuzakiKK

@smcv smcv marked this pull request as ready for review July 24, 2024 17:55
utils.h Outdated Show resolved Hide resolved
utils.h Show resolved Hide resolved
@smcv smcv force-pushed the wip/smcv/level-prefix branch from db40f0b to a58b8b6 Compare July 25, 2024 17:39
@smcv smcv requested a review from refi64 July 29, 2024 10:11
@smcv smcv force-pushed the wip/smcv/level-prefix branch 2 times, most recently from f71b028 to ff6baae Compare August 1, 2024 12:37
@smcv
Copy link
Collaborator Author

smcv commented Aug 1, 2024

Updated: while reviewing a similar change in libcapsule, @refi64 pointed out that I'd described the prefixes as "syslog-style" here, but not in libcapsule. In fact saying "syslog-style" is misleading, because https://datatracker.ietf.org/doc/html/rfc5424#section-6.2 describes a prefix that encodes both the severity and the facility, whereas here we're using only the severity, in order to be compatible with systemd-cat(1). Consistently say "severity" in preference to "priority" to reflect this.

(Documentation updates only; the only code change is to rename a level function parameter to severity.)

@smcv smcv requested a review from refi64 August 1, 2024 18:13
@smcv
Copy link
Collaborator Author

smcv commented Aug 5, 2024

Any thoughts on this from other maintainers? My colleague @refi64 already provided some useful feedback, which I've addressed.

@RyuzakiKK
Copy link
Contributor

I'm not a maintainer, but FWIW this looks good to me as well.

smcv added 3 commits August 15, 2024 15:00
This is the case since commit 0d369cd "main: Warn when
non-repeatable options are repeated".

Signed-off-by: Simon McVittie <smcv@collabora.com>
This takes a syslog-style severity level, allowing a larger program
that runs bwrap and reads a pipe from its stderr to filter or highlight
messages  based on the severity.

Take the opportunity to make the __debug__ macro (which normally expands
to nothing, but can be enabled by changing a `#if 0` to `#if 1`) less
weird and easier to use, by taking it out of the reserved-for-the-compiler
namespace, adding a newline automatically, and not requiring nested
parentheses.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This prepends a severity level such as <3> to each line of diagnostic
output, with numeric severity levels taken from matching syslog(3)
(such as LOG_ERR = 3), so that the diagnostic output can be parsed by
tools like `logger --prio-prefix` and `systemd-cat --level-prefix=1`
that support that encoding.

The facility (LOG_USER, etc.) is not included, since it makes little
sense to vary on a per-message basis. logger(1) supports prefixes
with or without a facility, and systemd-cat(1) only supports prefixes
without a facility, so this is compatible with both.

A future version of Steam's pressure-vessel is likely to use this to
make warnings and fatal errors from bubblewrap more visible.

Signed-off-by: Simon McVittie <smcv@collabora.com>
@smcv smcv force-pushed the wip/smcv/level-prefix branch from ff6baae to 89ae6b1 Compare August 15, 2024 14:03
@smcv smcv merged commit 2cca54f into containers:main Sep 30, 2024
5 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants