Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

journalctl: introduce --list-invocations, -I, --invocation= options #32448

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

journalctl: introduce --list-invocations, -I, --invocation= options #32448

wants to merge 6 commits into from

Conversation

yuwata
Copy link
Member

@yuwata yuwata commented Apr 24, 2024 •
edited by github-actions bot

No description provided.

@github-actions github-actions bot added documentation util-lib tests run please-review PR is ready for (re-)review by a maintainer labels Apr 24, 2024
@yuwata yuwata force-pushed the journalctl-current-invocation branch from f722900 to dece432 Compare April 24, 2024 08:50
YHNdnzj
YHNdnzj reviewed Apr 24, 2024
View reviewed changes
src/shared/logs-show.c Show resolved Hide resolved
src/shared/logs-show.c Outdated Show resolved Hide resolved
src/journal/journalctl.c Outdated Show resolved Hide resolved
src/journal/journalctl-filter.c Outdated Show resolved Hide resolved
@yuwata yuwata force-pushed the journalctl-current-invocation branch 2 times, most recently from e4d9cfc to 7396b0e Compare April 24, 2024 09:03
@yuwata
Copy link
Member Author

yuwata commented Apr 24, 2024

@YHNdnzj Thank you! All comments are addressed. PTAL.

poettering
poettering requested changes Apr 24, 2024
View reviewed changes
Copy link
Member

@poettering poettering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, some comments though

src/shared/logs-show.c Outdated Show resolved Hide resolved
src/shared/logs-show.c Outdated Show resolved Hide resolved
src/shared/logs-show.c Outdated Show resolved Hide resolved
man/journalctl.xml Outdated Show resolved Hide resolved
man/journalctl.xml Outdated Show resolved Hide resolved
@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Apr 24, 2024
@keszybz
Copy link
Member

keszybz commented Apr 24, 2024

Hmm, this tailors the feature very narrowly. I thought that instead we could handle this similarly to --list-boots and --boot, and allow the user to list invocations (with start-end timestamps) and select an invocation range by number.

@yuwata yuwata mentioned this pull request Apr 25, 2024
Merged
@yuwata yuwata force-pushed the journalctl-current-invocation branch from 7396b0e to 42a4b45 Compare April 25, 2024 10:31
@yuwata yuwata changed the title journalctl: introduce --current-invocation option journalctl: introduce --list-invocations, -I, --invocation= options Apr 25, 2024
@github-actions github-actions bot added login systemctl please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Apr 25, 2024
@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels Apr 25, 2024
@yuwata yuwata force-pushed the journalctl-current-invocation branch from 42a4b45 to 21018a5 Compare April 26, 2024 06:18
@yuwata yuwata marked this pull request as draft April 26, 2024 06:18
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Apr 26, 2024
@yuwata yuwata removed the please-review PR is ready for (re-)review by a maintainer label Apr 26, 2024
@github-actions GitHub Actions
Copy link

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

@yuwata yuwata mentioned this pull request Apr 26, 2024
Merged
@yuwata yuwata force-pushed the journalctl-current-invocation branch 3 times, most recently from e7ba8d1 to c1c1a03 Compare April 30, 2024 02:27
@YHNdnzj YHNdnzj added this to the v257 milestone May 1, 2024
@yuwata yuwata force-pushed the journalctl-current-invocation branch 2 times, most recently from 91de243 to f0f48ef Compare May 11, 2024 12:32
@yuwata
Copy link
Member Author

yuwata commented May 11, 2024

This is mostly ready for review, except for missing test cases for the new feature.

@yuwata yuwata force-pushed the journalctl-current-invocation branch from f0f48ef to d28c6ee Compare May 12, 2024 07:16
github-advanced-security[bot]
github-advanced-security bot found potential problems May 12, 2024
View reviewed changes
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
test/units/testsuite-04.invocation.sh Fixed Show fixed Hide fixed
@yuwata yuwata force-pushed the journalctl-current-invocation branch 2 times, most recently from d3cd7f7 to bef1b80 Compare May 12, 2024 08:52
@YHNdnzj
Copy link
Member

YHNdnzj commented May 12, 2024

Hmm, is this still targeting v256?

@yuwata
Copy link
Member Author

yuwata commented May 12, 2024

Hmm, is this still targeting v256?

No, unless someone strongly wants to push this to v256.

@yuwata
Copy link
Member Author

yuwata commented May 12, 2024

Ah, I forgot to update the tag in the man page.

@yuwata yuwata mentioned this pull request May 13, 2024
Merged
@yuwata yuwata force-pushed the journalctl-current-invocation branch from bef1b80 to e2b0a66 Compare May 13, 2024 10:32
@yuwata
logs-show: rename BootId -> JournalId
e82e130 
The struct itself is generic, and can be used for other ID.
Let's rename it to more generic one.
No functional change, just refactoring and preparation for later
commits.
@yuwata
journalctl: split out show_ids() from action_list_boots()
7d2d6a4 
No functional change, just refactoring and prepraration for later change.
@yuwata
logs-show: introduce several helper functions
ef7538a 
Currently these are not used, but will be used later.
@yuwata
logs-show: extend journal_get_boots() and friends to find invocation IDs
c3322d5 
Currently the extended features are not used, but will be used later.
@yuwata
journalctl: add --list-invocations command and -I/--invocation options
80c4a19 
The --list-invocations command is similar to --list-boots, but shows
invocation IDs of specified unit. This should be useful when showing
a specific invocation of a unit.

The --invocation option is similar to --boot, but takes a invocation ID
or an offset. The -I option is equivalent to --invocation=0.
@yuwata
test: add test for journalctl --list-invocations and --invocation=
0f7e3dd
@yuwata yuwata force-pushed the journalctl-current-invocation branch from e2b0a66 to 0f7e3dd Compare May 14, 2024 21:45
@yuwata yuwata marked this pull request as ready for review May 14, 2024 21:45
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YHNdnzj YHNdnzj YHNdnzj left review comments

@poettering poettering poettering requested changes

Assignees
No one assigned
Labels
documentation journal new-feature please-review PR is ready for (re-)review by a maintainer tests util-lib
Milestone
v257
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yuwata @keszybz @YHNdnzj @poettering @bluca