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.

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

sysusers: add parsing logic #94

Merged
merged 2 commits into from
Nov 30, 2021
Merged

sysusers: add parsing logic #94

merged 2 commits into from
Nov 30, 2021

Conversation

lucab
Copy link
Owner

@lucab lucab commented Nov 19, 2021

This adds support for parsing entries from sysusers.d lines.
Overall logic is organized around the 'nom' parser-combinator
framework.
It is now possible to roundtrip configuration entries, loading
content from files as well as writing back data.

@lucab lucab force-pushed the ups/sysusers-parse branch 4 times, most recently from eebb1b1 to 2c7a4ef Compare November 22, 2021 13:44
cgwalters
cgwalters previously approved these changes Nov 22, 2021
Copy link

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

(Skimmed) LGTM.

Though are the systemd developers aware of this parser? Its existence may (likely will) require pull requests here as they evolve the format in the future.

In particular if we end up in a situation where e.g. rpm-ostree starts crashing at build time because a newer systemd package uses a new type tag, I think that's going to be a long term problem.

We can mostly mitigate that by having any new sysusers changes done via a PR here and try to respin e.g. rpm-ostree before newer systemd, but longer term I think we're going to need this parser to gracefully handle unknown features.

@lucab
Copy link
Owner Author

lucab commented Nov 22, 2021

longer term I think we're going to need this parser to gracefully handle unknown features

I think you raised a fair point. Let's rework the parser here to log a warning and proceed on unrecognized line types.

This adds support for parsing entries from sysusers.d lines.
Overall logic is organized around the 'nom' parser-combinator
framework.
It is now possible to roundtrip configuration entries, loading
content from files as well writing back data.
This introduces a specific error kind to detect and recover from
sysusers parsing failures due to an invalid type signature.
It allows leaving some room for new entry types which could
possibly be introduced upstream in the future.
Thus, lines with unrecognized signatures are now skipped, and a
warning message is logged.
@lucab
Copy link
Owner Author

lucab commented Nov 23, 2021

Added one commit to detect&recover on unknown entry types, let me know if that meets your expectations now.

@lucab
Copy link
Owner Author

lucab commented Nov 30, 2021

@cgwalters unless further comments, I'm going ahead and merging this.

Copy link

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Sorry, lost this one in queue, I will try to watch this repo too.

@lucab lucab merged commit b025612 into master Nov 30, 2021
@lucab lucab deleted the ups/sysusers-parse branch November 30, 2021 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants