Replace discontinued `actions-rs` from CI

[FantasyTeddy]

Due to actions-rs being currently unmaintained, this PR replaces it with dtolnay/rust-toolchain and explicit calls to the respective cargo/cross commands.

Things to consider

• Maybe it would make sense to execute the "Formatting" and "Lints" jobs only once, instead of for each target platform.
• The clippy command currently does not fail when warnings are emitted. This could be fixed by setting RUSTFLAGS: "-Dwarnings" (see https://doc.rust-lang.org/nightly/clippy/continuous_integration/github_actions.html)
• The "Doctor" job is never executed, because the test variable is never set for any platform.

[grtcdr]

I hadn't noticed the action's deprecation, thanks a lot for giving us a hand in keeping things up-to-date.

As for the other points; merging the formatting and linting stages seems reasonable to me, since they both achieve somewhat the same thing (preserving a healthy codebase and consistent style) and they both fail on error, so we might as well group them together.

I wasn't aware of clippy's fail-on-warnings feature, or the fact that the doctor stage never runs on any platform. I'm sure there's a reason why it's disabled although that doesn't justify keeping an unused variable around. We should remove that condition and see how the command behaves on different platforms.

If you're interested in implementing all the points you mentioned in a different pull request, I'd be more than happy to merge them in.

Thanks again!

[FantasyTeddy]

As for the other points; merging the formatting and linting stages seems reasonable to me, since they both achieve somewhat the same thing (preserving a healthy codebase and consistent style) and they both fail on error, so we might as well group them together.

I wasn't necessarily suggesting that we merge the two stages, but that we extract them into one (or two) different jobs.

I wasn't aware of clippy's fail-on-warnings feature, or the fact that the doctor stage never runs on any platform. I'm sure there's a reason why it's disabled although that doesn't justify keeping an unused variable around. We should remove that condition and see how the command behaves on different platforms.

It seems that the test variable was removed in 274b19b6a4be74893f8ea77af53607a280d21599.

If you're interested in implementing all the points you mentioned in a different pull request, I'd be more than happy to merge them in.

Sure, what do you think, should we merge this PR and I open another one for those changes, or should I append them to this PR?

Thanks again!

Happy to help 🙂

[FantasyTeddy]

Addendum: I updated the "Install cross" step to install the latest version from their GitHub repository. This fixes the NetBSD build (see cross-rs/cross#1348) until a new version is released.

[grtcdr]

I wasn't necessarily suggesting that we merge the two stages, but that we extract them into one (or two) different jobs.

How would that help? Could you show an example?

It seems that the test variable was removed in https://github.com/Macchina-CLI/macchina/commit/274b19b6a4be74893f8ea77af53607a280d21599.

What a horrible, horrible commit message. I literally failed to explain the "why" factor that prompted that change. I can't recall what went on in my head at that time. So... we should bring that test variable back.

Sure, what do you think, should we merge this PR and I open another one for those changes, or should I append them to this PR?

First proposal, definitely :)

[FantasyTeddy]

I wasn't necessarily suggesting that we merge the two stages, but that we extract them into one (or two) different jobs.

How would that help? Could you show an example?

Sure, but I would suggest that we do this in the upcoming PR that does the change and you can give your feedback there. What do you think?

It seems that the test variable was removed in 274b19b.

What a horrible, horrible commit message. I literally failed to explain the "why" factor that prompted that change. I can't recall what went on in my head at that time. So... we should bring that test variable back.

Will gladly do this as well in the follow-up PR.

[grtcdr]

Sure, but I would suggest that we do this in the upcoming PR that does the change and you can give your feedback there. What do you think?

Totally, even better.

Will gladly do this as well in the follow-up PR.

Thanks!

[grtcdr]

More oddities have come our way.

Running cross fmt on my machine shows the following message:

[cross] note: Falling back to cargo on the host.

And platforms for which cross-compilation is enabled fail with this error:

[cross] warning: specified cargo subcommand fmt is not supported by cross.

We should probaly replace the variable env.PROGRAM in the formatting stage with just cargo.

[FantasyTeddy]

Yes this seems to be the behavior in the CI as well. However, I don't quite understand why this is now an issue, because looking at previous runs using actions-rs the message about "Falling back to cargo on the host." is present for all platforms using cross...

We should probaly replace the variable env.PROGRAM in the formatting stage with just cargo.

Thank you for the suggestion, I changed it accordingly.

[grtcdr]

Yes this seems to be the behavior in the CI as well. However, I don't quite understand why this is now an issue, because looking at previous runs using actions-rs the message about "Falling back to cargo on the host." is present for all platforms using cross...

I wouldn't know how to check previous runs, but if that's the case then it's pretty weird that it would break all of a sudden.