chore: Use the workspace.package table for keys which apply to all of our crates

[DataTriny]

• The authors key has been sort of deprecated for a while now, but I kept this change in a separate commit so we can easily revert it if we don't agree.
• I have everything ready to enforce our MSRV, but 1.68.2 isn't the correct value at this time:
  • We read the CARGO_BIN_NAME environment variables in some examples, which is only supported since 1.69.
  • We allow the unnecessary_literal_unwrap in platforms/windows/src/subclass.rs yet this clippy lint was introduced in 1.72.

So we need to decide whether to replace the offending lines, or to bump our MSRV to 1.72.

[mwcampbell]

I'm curious how you decided on Rust 1.68.2 as an MSRV, but I don't see any problem with it. I also have no problem with dropping the authors field.

[DataTriny]

Rust 1.68.2 is what cargo-msrv reports when ran:

cargo msrv

But if I change the command to:

cargo msrv -- cargo clippy

Then the result is 1.72.1.

I will check but I think this conflicts with some of our downstream users. So the question is: should we remove stuff so that we can compile all targets with 1.68.2 or should we mark our MSRV as 1.72.1?

I will add appropriate checks in our CI once this PR is merged so that we never have to deal with this again.

[DataTriny]

Here is a list of all the AccessKit users I am aware of with their claimed MSRV:

• bevy: 1.74
• egui: 1.72
• fltk: 1.56
• freya: unknown
• glazier: unknown
• rui: unknown
• slint: 1.70
• vizia: 1.60

Actually we could run cargo clippy and cargo test separately in the CI, using the nightly toolchain for the former and our MSRV for the later. I've seen other projects do this. Obviously the downside is longer CI times because of more jobs to spawn.

[mwcampbell]

I'm not sure I understand the point of running cargo msrv -- cargo clippy. That may catch clippy lints that were fixed or removed in later versions, but is it actually likely that downstream users will run clippy on our crates?

[DataTriny]

I only ran cargo msrv -- cargo clippy to determine the minimum Rust version able to execute clippy.

Here is what happens when trying to run clippy with Rust 1.68.2:

error: unknown lint: `clippy::unnecessary_literal_unwrap`
   --> platforms\windows\src\subclass.rs:101:17
    |
101 |         #[allow(clippy::unnecessary_literal_unwrap)]
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::unnecessary_filter_map`
    |
    = note: `-D unknown-lints` implied by `-D warnings`

I have tested modifying the CI to use latest stable for clippy but 1.68.2 for tests and it works fine. If you agree with this approach then there is nothing more to add here and I will submit a PR with the changes in our main workflow.

[mwcampbell]

I verified that cargo clippy does run on dependencies by default when running it on a downstream crate. But I don't think we should set our MSRV based on clippy, for two reasons. First, the "unknown lint" warning is not an error by default, though clearly it was on your system, unless you omitted -D warnings when telling me what command you ran. Second, clippy does have the --no-deps option, so if someone needs to suppress our clippy warning/error to make clippy work in their CI with an old toolchain version, they can use that.

But now, I wonder if we should specify the MSRV for the whole workspace, or per crate. The accesskit, accesskit_consumer, and accesskit_macos crates have an MSRV of 1.63.0. The accesskit_windows crate has an MSRV of 1.64.0. accesskit_unix has an MSRV of 1.68.2, which it inherited from one of the atspi crates. accesskit_winit has an MSRV of 1.68.2 on Unix, or 1.65.0 on other platforms; I guess we would set it to 1.68.2 to cover all platforms. Still, if we want to avoid breaking users who only care about Windows and/or macOS and are oooooon a toolchain older than 1.68.2, I guess we should set the MSRV per-crate.

[mwcampbell]

On second thought, if there's an easy way to avoid inconveniencing our users, including those stuck on old toolchain versions for any reason, we should do so. So I'll see if there's a way I can rewrite the code where we currently allow the unnecessary_literal_unwrap lint.

[mwcampbell]

Hmm, if we set an MSRV of at least 1.66.0 across all crates, then we can simplify the implementation of NodeClassSet::lock_global, and also declare NodeClassSet::new as const, which some users might find helpful. And 1.66.0 is just over a year old. I can do that fix in a separate PR.

[DataTriny]

Sorry for not being clear enough here:

The error I showed come from a CI run I did on my fork. I don't think we should determine our MSRV based on what is needed for clippy to run, hence my suggestion to modify our workflow.

I would prefer to have a MSRV policy that applies to the whole workspace, for the sake of simplicity and clarity, but I can change my mind if we have valid counterpoints. I always keep a fairly up-to-date Rust version on my system and I don't like seeing clippy warnings, so I think we should really run clippy separately in our CI.

[mwcampbell]

OK, a single MSRV for the whole workspace is fine. And since we're declaring an MSRV of 1.68.2, I can go ahead and open a PR for the simplification of NodeClassSet::new.