Edition and MSRV for this crate?

[TonalidadeHidrica]

What is the (intended) minimum supported version and the Rust edition for this crate? The README specifies the MSRV when async is enabled, but it is not specified when otherwise. As for the edition, the Cargo.toml does not include the information, but it seems that you are using try! macro, which is deprecated in Rust 2018. Do you still want to stick to Rust 2015 to obtain backtrace for the errors? In order to upgrade to Rust 2018, all we have to do I think is just rename try macro to another one and define it by ourselves, or use modern error handling library like thiserror or anyhow. I want you hear your policy before contributing.

[est31]

As for the edition, the Cargo.toml does not include the information

If the edition is not specified, cargo assumes it to be 2015 for backwards compatibility reasons. There were some versions of cargo that would feature gate the edition mechanism, where edition = "2015" in Cargo.toml would give you an error. 1.12.0 is not part of the problem, it's more the releases leading up to Rust 1.31.0. So I kept it unspecified while lewton can have edition = "2015" thanks to the higher MSRV.

use modern error handling library like thiserror or anyhow.

I use anyhow in my mimas project, I'm really a fan of it, and is especially way better than what came before it. Thiserror I'm less sure about, but that's probably mainly because I haven't encountered use cases for it yet. Ogg definitely isn't because it has so few errors. Also, Ogg currently doesn't have any significant dependencies and I don't want to introduce something as heavy as those two crates even if thiserror would be worth it in isolation. E.g., currently MSRV is up to me to decide instead of me having to follow what my dependencies dictate. My mimas project already has hundreds of dependencies and more aggressive MSRV rules.

As for the MSRV, yes 1.12.0 is the current MSRV if async is disabled. Since a few days, it's even tested on CI. I'm open to increasing it eventually, e.g. for #5 I'm eyeing the const if/match/loop stabilization that happened in 1.46.0, but for now it's a bit too recent. Maybe in February or something, close to the 1.50.0 release? Note that it would be a breaking change and increase the MSRV of lewton too, and I've kept the MSRV of lewton traditionally always quite old.

Maybe one can instead just refactor the async feature to use async/await? 1.39.0 is old enough to depend immediately on it, but currently the async library ecosystem has multiple standards for the same thing and I don't want to lock in the ogg crate into one of them. A pure dependency-free async/await refactor might be helpful though.

[TonalidadeHidrica]

Thanks for the detailed explanation. So for now, it seems that I should go with Edition 2015 / 1.12.0 . I didn't know that edition defaults to 2015 when unspecified. But the problem is that, when I compile the project on my default toolchain (1.47.0), it gives me a lot of deprecation warning for the try! macro use. So I created a rust-toolchain file written 1.12.0, but then cargo build gives me an error:

error: failed to parse lock file at: /path/to/ogg/Cargo.lock

Caused by:
  invalid serialized PackageId for the key `package.dependencies`

so I have no idea what to do. Sorry for beginner-ish question, but how can I properly develop in 1.12.0?

I think it'd be very good if the MSRV is 1.39.0, but I'm not confident in refactoring it to "dependency-free" one, since I've not really understood the async/await yet. Does "dependency-free" refers to completely removing the dependencies for tokio-io and futures, and using standard library instead?

As for error handling library, can't we provide it as a separate feature, so that we can obtain backtraces via those library when debugging, while using plain Error type when the feature is disabled?

[est31]

I recommend doing rustup override add 1.12.0 instead of a toolchain file because then it won't need to be checked into git. The try macro is deprecated since 1.39.0, so using newer rustcs like 1.31.0 is definitely possible.

You can fix the Cargo.lock errors by removing the file and then letting 1.12.0 re-create it.

Does "dependency-free" refers to completely removing the dependencies for tokio-io and futures, and using standard library instead?

Yes :).

[TonalidadeHidrica]

Thanks for your advice. This is an opinion of newbie, but isn't it a common practice to place rust-toolchain so that all developers uses the common version?

[est31]

@TonalidadeHidrica yes some projects do that but if the number of developers is very small there is less of a benefit in it because it's just a single person.

Also, just because some version is the MSRV for build doesn't automatically mean that tests have to work on it as well. E.g. serde builds on the MSRV, but tests don't work, so mainly it's the CI which test for the MSRV while devs contribute using whatever compiler they like. In general, newer compiler versions are faster (thanks to improvements in compiler speed) so for local development it's better to use newer rustcs :).

That being said, the try deprecation warnings are annoying. I'll try to find a way to fix them.

[TonalidadeHidrica]

Thanks, I understood.

As for the deprecation warning, we can allow entire deprecation by #[allow(deprecated)], but not only for try macro it seems...

[TonalidadeHidrica]

When do you think is a good time to bump the MSRV up to, for example, 1.39.0?

[est31]

@TonalidadeHidrica the question is, which feature of that release will we depend on unconditionally?

Conditionally, it might make sense to specify that if you enable the async feature, 1.39.0 is the MSRV. But is there any feature that improves ogg that is worth the MSRV bump? Maybe if async is disabled, an older version can be used as MSRV? Not sure which version would be helpful here... 1.27.0 adds dyn syntax for example... yeah let's increase it to 1.27.0 for now.

[TonalidadeHidrica]

What I want is language features, rather than library features. Modern versions enable us to write code more concisely, and I prefer them. Of course you may not like some of them, and it's not a good reason enough to lock out some users. But isn't 1.12.0 "too" old? (Of course I can go with those new features, just for convenience.)

[TonalidadeHidrica]

Thank you for your effort for the upgrade. Now that my question is resolved, I'm closing this issue.

[TonalidadeHidrica]

@est31 Hi, long time no see. Since the project Edition was upgraded to 2021, could you update the mention of MSRV in the README (and maybe add rust-version to Cargo.toml)?

[est31]

@TonalidadeHidrica good point about rust-version, README should already have been changed by de418cf1f3dd102363786a7ca9acb6cd3a20e771

[TonalidadeHidrica]

@est31 Oh thanks, I didn't realize. What about the Rust version when async is enabled?

[est31]

What about the Rust version when async is enabled?

That one wasn't specified, and I still can't specify it because it depends on the MSRV policies of too many crates.