fix: Change YAML crate to serde_yaml

[polarathene]

• yaml-rust is unmaintained for over 2 years.
• Like with yaml-rust, requires special handling for table keys (ensuring numbers are converted to strings).

Resolves: #473

---

Just some insights below that might be worth noting considering the switch. Presumably this should be delayed until a breaking release is acceptable?

Since config-rs is only interested in deserialization, some concerns users may have regarding serializing valid YAML files may be a non-issue, but changing from one YAML crate to another may behave differently with deserialization still:

• Prior to the 0.9 release (July 2022) of serde_yaml, it used the yaml-rust crate internally. Now it uses a transpiled libyaml crate.
• The yaml-rust format had parsing logic by config-rs to ensure a single document or create an empty Map. You should get similar with serde_yaml for an empty Map or ValueKind::Nil
• Numbers can be keys, but have always been converted to strings for the Map type in config-rs.
  • There is an existing test-case YAML config for this with mixed number and string keys. No difference in behaviour here than what we already had.
  • There is a tracking issue at serde_yaml, ideally that would be resolved upstream to properly handle number to string conversion (only seems to be an issue for untagged enums).
• serde_yaml changed tagged enums with 0.9 release, that is causing some issues from users upgrading from 0.8. config-rs test suite is passing, but the test coverage for config files input to Rust types may not cover some of these concerns? 🤷‍♂️
• Some YAML parsers have a wider range of values that are treated as booleans based on YAML 1.1 spec apparently, while serde_yaml only implements support for what the YAML 1.2 spec states should be treated as bools.
  • A request for wider support was rejected.
  • It may be encountered in some configs like Ansible, where it's considered a valid bool value type. I've not checked if yaml-rust is any better at this though.
• What may look like an invalid config, may be treated as a single String instead of throwing an error depending on the input, as it's technically valid.

[polarathene]

Note, I have implemented the equivalent for #472 which could include the serde_yaml support switch instead if preferred (or addressed as a separate follow-up PR).

Presently the Tagged variant of serde_yaml::Value is not handled. I don't think we can easily implement support for that conversion either? I'm not experienced enough at least to know how to integrate that.

The feature has !Variant as a value prefix in a .yaml file, and serde_yaml represents the type with two properties, tag (that !Variant value) and value actual data for the enum variant.

I'm inclined to ignore it since it's not a previously supported feature 🤷‍♂️

[matthiasbeyer]

First of all, sorry for the delay. I got sick and am writing this from my bed.

So what I see here is a lot of change in behavior when switching from one yaml implementation to another. I am not sure whether I like that, but on the other hand we are in a zero-dot version (0.x.y), so it wouldn't be an issue from a version perspective.

I'll leave it up to you, really!

[polarathene]

I got sick and am writing this from my bed.

Ah, no worries rest up 👍

I pinged due to your activity appearing active and wasn't sure if you were notified of the PR.

---

So what I see here is a lot of change in behavior when switching from one yaml implementation to another.

It's roughly at parity, at least the implementation is very similar on config-rs end and what is supported.

I've otherwise made note above of known issues. Some of these may not be relevant to config-rs as it deserializes from YAML input to Value as an intermediate / normalized storage before deserializing to a struct provided by the user? This adds another layer of deserialization where types may be converted to the appropriate type to satisfy the users config struct fields.

on the other hand we are in a zero-dot version (0.x.y), so it wouldn't be an issue from a version perspective.

Tests were helpful and ensured that the switch at least passes those, if there is something that wasn't covered, new tests could be added. It may warrant bumping to a 0.14 release? 🤷‍♂️ (or a part of that bump)

I'll leave it up to you, really!

My preference is to #472 which already bundles the equivalent feature change, but without the serde-yaml specific enum handling.

The yaml format file becomes very simple with one special case for the yaml maps (due to string keys for table being necessary).

There may be some differences between this PR and #472 untagged enum approach, but current tests pass all the same. In the event there is an issue, this PR provides an alternative to fallback to.

[matthiasbeyer]

Note to self:

My preference is to #472 which already bundles the equivalent feature change, but without the serde-yaml specific enum handling.

[polarathene]

Now that the MSRV lock file issue is out of the way, I'll be revisiting my PRs this weekend. If you want to block on any let me know, otherwise I'll self-review again and merge after addressing any existing concerns I raised.

[matthiasbeyer]

I'll self-review again and merge after addressing any existing concerns I raised.

Please do that! Thank you so much for your contributions!