project CI refresh #50
Conversation
* Reformats to better support parameters. * Adds merge_group trigger for supporting queued merges. * Adds cron scheduled build.
The `actions-rs/clippy-check` action is archived and there isn't a clear alternative. Since clippy is already being run in `rust.yml` the right fix seems to be removing this extra workflow.
Of the form:
```
warning: the item `AsRef` is imported redundantly
--> src/tls.rs:5:21
|
5 | use core::convert::{AsRef, TryInto};
| ^^^^^
--> /rustc/a165f1f65015b1bd4afd2ec50700aaacf2e0c485/library/core/src/prelude/mod.rs:28:13
|
= note: the item `AsRef` is already defined here
|
= note: `#[warn(unused_imports)]` on by default
```
Fixes warning of the form:
```
error: unnecessary qualification
--> /home/daniel/Code/Rust/tls-parser/target/debug/build/tls-parser-f3ef275828190684/out/codegen.rs:2:53
|
2 | pub static CIPHERS: phf::Map<u16, TlsCipherSuite> = ::phf::Map {
| ^^^^^^^^^^
|
note: the lint level is defined here
--> src/lib.rs:132:35
|
132 | /*unused_import_braces,*/ unused_qualifications)]
| ^^^^^^^^^^^^^^^^^^^^^
help: remove the unnecessary path segments
|
2 - pub static CIPHERS: phf::Map<u16, TlsCipherSuite> = ::phf::Map {
2 + pub static CIPHERS: phf::Map<u16, TlsCipherSuite> = phf::Map {
|
error: could not compile `tls-parser` (lib) due to 1 previous error
```
Unfortunately the `::phf` prefix is coming from a dependency,
`phf_codegen`, and so isn't easily fixed:
https://github.com/rust-phf/rust-phf/blob/999e6a260f03d82aa9d159465113294e7ed019e7/phf_codegen/src/lib.rs#L182
Adopts a similar feature matrix approach as the asn1-rs CI.
This matches the other Rusticata crates and fixes a build error with 1.60 from `memchr` requiring 1.61+ Note: we have two transitive dependencies that have bumped their MSRV within otherwise semver compatible versions: * `winnow` 0.4.2+ requires an MSRV of 1.64.0[0] * `toml_edit` 0.19.9+ requires an MSRV of 1.64.0[1] Recently the Rust project guidance on commiting lockfiles for library projects[2] was changed. They say: "A lockfile is an appropriate way to pin versions for your project so you can validate your MSRV" As a result I've opted to commit a lockfile and use `--locked` in CI to maintain MSRV. I think this is a better approach than manually working around the issue with `cargo update --precise` and in line with current best practices. [0]: https://github.com/winnow-rs/winnow/blob/v0.6.6/CHANGELOG.md#042---2023-04-28 [1]: https://github.com/toml-rs/toml/blob/main/crates/toml_edit/CHANGELOG.md#0199---2023-05-18 [2]: https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
|
@chifflier I think this one is ready for review now. Adding CI coverage for the MSRV of 1.63 required some care. This crate has two transitive dependencies that have bumped their MSRV within otherwise semver compatible versions: Recently the Rust project guidance on commiting lockfiles for library projects was changed and they now leave it up to individual projects, saying: "A lockfile is an appropriate way to pin versions for your project so you can validate your MSRV" I generally like that approach and it works well in some other repos I contribute to. As a result I've opted to commit a lockfile and use |
|
Hi, If I understand correctly, this changed last year, mostly for reasons similar to the problems we are encountering. Maybe the best solution would be add yet another test in CI (something like weekly or daily?) to test build with latest dependencies, in addition to the |
I'm open to that, but I think it will immediately error for this crate based on the situation being avoided with the lockfile. Should I go ahead and add that task? |
I think I confused myself here and it would be fine. We just wouldn't use the MSRV rust version for this test, using stable instead (?) |
The non-locked version should probably be on stable. |
Correct, it would be this one, but not all the deps will be pinned to old versions. Just the deps that can't be updated past a specific version without changing the MSRV. In this case, just |
Ok, fair enough, thank you for the explanation. |
Continuing the CI standardization and refresh started in rusticata/rusticata-macros#8