Migrated from yaml-rust to its well-maintaned fork

[nikarh]

This PR changes the yaml-rust dependency to yaml-rust2.

yaml-rust crate hasn't been updated in 3 years, and since nobody was able to reach the author it was forked.

Related issues:

• Maintainers chyh1990/yaml-rust#192
• Add more maintainers chyh1990/yaml-rust#160
• Is this crate maintained? chyh1990/yaml-rust#197

Why migrate now? Due to https://rustsec.org/advisories/RUSTSEC-2024-0320, now this dependency causes cargo-deny to fail builds on projects that use it.

[torokati44]

Came here to post this PR, but didn't have to! :) Thanks!
Looking forward to this being merged and released. ^^

[torokati44]

BTW, if the code changes are undesired, there is a way to "alias" it back to the old package name:

chyh1990/yaml-rust#160 (comment)

[danielhjacobs]

Note that Ethiraric/yaml-rust2#9 is needed to avoid https://rustsec.org/advisories/RUSTSEC-2021-0153. That's already been merged into master but there is no release with that change yet.

[torokati44]

but there is no release with that change yet.

There is now! :)

[mitsuhiko]

Unfortunately this completely bumps the MSRV up significantly. This means I need to balance people that care about an unmaintained crate vs need support for older rust versions :-/

[torokati44]

Do you think it would be feasible to make the yaml feature (and thereby this dependency) completely optional then?

[mitsuhiko]

No. Because the YAML parser is also used internally to read snapshot headers. Maybe the right solution for now is to vendor a stripped down version of the old yaml crate until a new major.

See also #450

[torokati44]

Unfortunately this completely bumps the MSRV up significantly.

And this is with 0.7.0, not even with 0.8.0, which actually fixes the advisory. Not sure if this will make an additional difference though.

There's also this sort of thing:
https://github.com/paolobarbolini/bzip2-rs/blob/82e3cbb4ea6cb4249a43f90b06ae1a5e94f06a9f/Cargo.toml#L32-L37

I'm not saying it's pretty, but would you be open to something like this?
Maybe not explicit msrv_ features, but a feature to toggle between the original yaml-rust, and this fork?
Perhaps it can be done with Cargo.toml tweaking only due to the fork having kept the original package name (see the link in my second comment).

[mitsuhiko]

I'm not sure what the right course of action is. yaml-rust2 seems to bump up the MSRV to ridiculous levels. I'm not sure what it targets but even 0.7.0 seems to require rustc 1.70.0.

[torokati44]

I really think it would be worth giving a try to the dependency toggling feature, making use of the complete API compatibility of the fork.

[mitsuhiko]

Not sure what the point of this is. YAML is necessary for insta to even read any snapshot file. There is no MSRV compatible version of insta that does not have YAML support and if I need YAML support within insta for it's own uses, then i might as well only use that instead of entertaining two versions.

[torokati44]

I meant that the feature would switch between the original and the fork for both internal uses and the exposed yaml feature.

But sure, if you are okay with the big MSRV bump for #466, unconditionally switching to the fork, that also works.

[mitsuhiko]

Unfortunately it does not look like yaml-rust2 is an option. Let's move that discussion to #467