feat: Define MSRV

[gutenfries]

Changes

• Fixes (feat): clearly define MSRV #1310

• Clearly define the MSRV, as well as create necessary files to communicate that to development tools (cargo, clippy, rustup)

• Tests the MSRV in the CI locked version matrix testing

• Adds cargo-features = ["workspace-inheritance"] to the head of Cargo.toml, which allowed for a lower MSRV without changing any code.

• Adds the MSRV to README.md, and creates a page in the docs for the MSRV.

• (UNRELATED) Renamed mismatched .yml and .yaml files in .github/workflows/ to all be .yml

Checklist

• add MSRV to README.md, as well as the docs
• test MSRV in CI

[gutenfries]

(@fzyzcjy) I've been working on finding an efficient and helpful manner to use the CI to check the MSRV on any Push/PR.

I think we both can see the benefits of this; if the MSRV goes up drastically, perhaps that code should not be merged yet.

In my opinion, a good way to do this is to check the MSRV in GHA, and if the MSRV is higher, the action should create a comment on that PR stating the changes, which can then be dealt with.

This differs from a push event, however, and on a push event the MSRV would be checked, and if it has changed, be updated across repo files that reference the MSRV. (this also happens on a PR event)

The issue I am running into is that the ${{ secrets.GITHUB_TOKEN }} does not have the permissions needed to write comments or create commits (write contents).

(These struggles are the reason for the large amount of commits)

--

If you think that this is something we should implement, I think that setting up a workflow token for this GHA would be good.

If NOT, then I vote rolling the MSRV verification into just precommit, escaping the CI entirely.

Until we agree on how this should be done, and what's best for security, simplicity, etc, I am not going to mark this ready for review, though the POC has been implemented and optimized.

Apologies for the quite lengthy summary my friend, and please let me know your thoughts on this

[fzyzcjy]

Good job!

Firstly, I guess usually our MSRV should not change (since as a library, we should use the most basic features of Rust to have maximum compability, unless we really need some new feature, or the MSRV is already quite old). Therefore, I expect changing the MSRV is an intentional (explicit) action that the implementor is aware of. For example, a PR may be "let us implement frb feature A which uses Rust language feature B, and since B needs MSRV=1.2.3, our MSRV should be bump to 1.2.3".

Thus, I guess the simplest way is that, we do not check MSRV dynamically. Instead, we run the CI using multiple rust runtime versions (using GHA matrix). For example, say MSRV=1.64, we may test Rust = 1.64 + 1.71.0 (i.e. pinned stable) + 1.73.0 (i.e. pinned nightly). (I am not sure whether we should use something like "nightly" to get latest nightly, or "1.73.0" for pinned nightly, since I do not want to break the CI without any code change which can be annoying).

I can totally understand your feeling if the code you have written has to be changed! So next time maybe we can discuss the rough idea before implementing the details, e.g. if we discuss whether adding MSRV check in CI, I may suggest the ideas above.

So in short, maybe we can remove MSRV check in CI or precommit, but we can add it as a separate command in justfile. Then, we can determine the MSRV (possibly using the script?) and fill that version into CI and readme etc.

[gutenfries]

No worries at all my friend!

I had wanted to have an entirely functional POC to provide to you to show the features I had implemented, fully understanding that this is opensource and they may be denied, but I was unable anyway lol

with that in mind, all it took was the deletion of one file :)

The dynamic MSRV check may still be run by either just msrv, or manually running the scripts in the repo root.

My changes are now ready for review

[fzyzcjy]

Good job! Only some nits (though look like a dozen, but they are mostly quite trivial)

[fzyzcjy]

nit: we need to add a link to it from our book index

[fzyzcjy]

nit: It seems that we are repeating ourself for 3 times:

1. the one in ci (we test our code with MSRV version)
2. the one in readme.md (the badge)
3. the doc here

is it possible we somehow add a very quick sanity check for them, to ensure that they are always in sync? e.g. some quick python/shell/whatever script to extract MSRV from these 3 places, and check they are equal. Then we can add them to CI, so nobody will accidencially make MSRV not-in-sync

---

EDIT: Oh I see, they are already auto generated from shell. Then what about this:

1. refactor the shell script into "determine MSRV automatically" and "given MSRV, write them into these few places". because in the future someone may want to manually write MSRV.
2. in these few places, maybe we can add comments saying e.g. <!-- MSRV IS AUTOMATICALLY WRITTEN BY xxxxxxxx.sh, PLEASE DO NOT MODIFY BY HAND, BUT USE THAT SCRIPT-->

[fzyzcjy]

nit: it seems that stable_msrv.sh and nightly_msrv.sh has a lot of duplicates. in order to have a good maintainability, it would be great to avoid it :)

[fzyzcjy]

nit: also add latest stable

[fzyzcjy]

nit: it would be great if the old format could be kept, because someone else in the future may also automatically change the format via IDE automatically, then the code blame history will be a mess... but that's not a super big problem.

[gutenfries]

what toml formatter does the project use?

If it is Taplo, perhaps a configuration file may be helpful (https://taplo.tamasfe.dev/configuration/file.html)

[fzyzcjy]

You are right, there is no format yet. I am just following flutter's convention: when there is no (auto) formatter, try to not change the code due to format.

However, feel free to configure a formatter (e.g. Taplo)! Just ensure that

• it is checked in CI (along with other formatters, like Dart and Rust formatter)
• it is run automatically when just precommit (trivial to do if you put is as a sibling to the existing Dart format etc)
• it has nice support and is popular, e.g. has VSCode and Intellij IDEA bulit-in support or plugin support

[gutenfries]

a) foresterre/cargo-msrv does NOT support nightly (perhaps I'll make a PR there in the future)

Yes I think so, that's why I hope you to PR there - then everyone in Rust ecosystem will benefit from your code about MSRV.

I whole-heartedly agree.

To be completely honest, I think that this PR should be closed, and once I/the dev team @ https://github.com/foresterre/cargo-msrv implement and release nightly MSRV checking, another different PR should be reviewed.

there are too many irrelevant commits in this PR and simply put, there is no reason.

I'll probably open a draft PR to track progress, but I'll mark as ready for merge and send a ping when we're ready.

UPDATE: (foresterre/cargo-msrv#101 (comment)) this is actively being worked on.

[fzyzcjy]

To be completely honest, I think that this PR should be closed, and once I/the dev team @ foresterre/cargo-msrv implement and release nightly MSRV checking, another different PR should be reviewed.
there are too many irrelevant commits in this PR and simply put, there is no reason.

I agree. Borrowing Flutter's PR philosophy - one PR should only do one thing, it looks good to split this PR into multiple PRs.

I'll probably open a draft PR to track progress, but I'll mark as ready for merge and send a ping when we're ready.

Sure, take your time!

UPDATE: (foresterre/cargo-msrv#101 (comment)) this is actively being worked on.

That looks great :)