-
Notifications
You must be signed in to change notification settings - Fork 414
Add unmaintained unsafe-libyaml & serde_yaml #2459
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
base: main
Are you sure you want to change the base?
Conversation
|
pinging maintainers of crates listed as alternatives, so they can comment on the descriptions / appropriateness of these recommendations: |
|
|
||
| - [`yaml-spanned`](https://crates.io/crates/yaml-spanned) - Deserializer only. | ||
| - [`serde_yaml2`](https://crates.io/crates/serde_yaml2) - Incomplete implementation of serde framework. | ||
| - [`serde-saphyr`](https://crates.io/crates/serde-saphyr) - Not complete implementation of YAML 1.2 or serde framework. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to replace this by
[`serde-saphyr`](https://crates.io/crates/serde-saphyr) - Implementation of YAML for serde framework, replaces unsafe-libyaml by saphyr-parser.While the library is relatively young, "incomplete" implies that this is a stub that lacks all the necessary functionality. This is not the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bourumir-wyngs I am not happy with that alternative you provide, but I do understand what you are getting at.
The main point that must be conveyed is that serde-saphyr is not a complete implementation of YAML 1.2, because saphyr-parser is still incomplete wrt YAML 1.2 spec. Very incomplete. I have lots of types of data structures that worked with serde_yaml that fail inside saphyr-parser. It did work for a few simpler data structures, and I adopted it for those. But it is not nearly as good at yaml-spanned, and it is roughly on par with serde_yaml2 based on my usage.
To be honest, I am not sure how complete serde-saphyr is wrt to the serde framework. i.e. which issues are due to serde-saphyr immaturity vs problems it inherits from saphyr-parser and cant fix until saphyr-parser improves. It is a bit difficult for an outside observer to easily identify which is the cause of the problems. That said, from what I saw, there is a bit of both to blame for the problems I saw trying to migrate a very large commercial codebase to serde-saphyr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While saphyr-parser needs more work, I do not see where else would be the way to go. yaml-spanned you talk about uses internally libyaml-safer that is, again, the fork of the same unsafe-libyaml. Having cascades of unsafe there is fundamentally a problem, memory safety is on the main Rust's selling points.
Serde-saphyr currently has 618 passing tests, many of which were ported from the yaml-test-suite (those with a y_prefix), and also quite many from serde-yaml. If you would share at least some of these "lots" structures that do not work, I would be thankful. yaml-rust2 (from whom saphyr-parser is the fork with modern features) has 19,601,133 downloads to date so I think works for some.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libyaml-safer does not use unsafe. https://github.com/simonask/libyaml-safer/blob/c09b7a24554171e32fa958042304af0f4cf4e203/src/lib.rs#L22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/saphyr-rs/saphyr/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen lists the many fundamental issues with saphyr-parser wrt YAML. And I ran into many of those. It changes the data-type of incoming data. And that blows up your serde layer on top of it in weird ways.
|
I consider That's a very precarious state for it to be in, so I wouldn't consider it advisable to recommend it as an alternative at this stage. If anyone wants to pick it up where I left off, I'm more than happy to provide guidance! |
|
@simonask , there wont be any updates from archived repo |
Closes #2132