Changes:
- Add "derive" feature for "clap" in tester/Cargo.toml.
- Move about description from tester/src/cli.rs to "description" field in
tester/Cargo.toml. So, "clap" can access it via #[command(about)]
macro configuration.
- Update Cargo.lock.
- Replace in tester/src/cli.rs usage of clap::App from Clap 2.34.0
with clap::Parser derive macro based API, cli::Cli struct and cli::Command
enum.
- Remove "--config/-c <FILE>" option for "tester" as it wasn't used and
I believe it was a copy-paste from "hagridctl".
- Omit explicit specification of args name which resulted in upper
casing cmd option's parameters, e.g. "<cert count>" -> "<CERT_COUNT>".
- Replace in cli::dispatch_cmd() "if let" based logic with simple match
on cli::Command enum variants.
- Get rid of manual parsing of options' and commands' values in
cli::dispatch_cmd(). As it is hadled by derived
value_parser (https://docs.rs/clap/latest/clap/_derive/index.html#arg-attributes)
based on field types.
- Make "fprs_path" parameter of tester::generate::do_generate function
obligatory (previously it was Option<&Path>) as this argument has a
default value to "fingerprints.txt" which is populated by clap. So, it
never be None.
In addition, clean up logic related to optional "fprs_path" parameter
in tester::generate::do_generate function.
- Change signatures of the following functions to accept
"impl AsRef<Path>" instead of &Path or Option<&Path>:
- tester::generate::do_generate()
- tester::genreqs::do_genreqs()
That allows us to pass as function arguments anything from which Path
can be borrowed, e.g. PathBuf without the need to explicitly transform
the value at call site with calls like .as_path() or .as_deref().
- Replace in tester/src/main.rs cli::app().get_matches() (which is Clap
v2 API) with cli::Cli::parse() (Clap v4 API).
Changes:
- Move the following structs from main.rs to "config" module:
- HagridConfigs
- HagridConfig
- Introduce "config::load" function and move configuration loading code into it.
- Adjust imports accordingly.
- Share ArgMatches returned by App::get_matches between
config::load and cli::dispatch_cmd functions.
- Make HagridConfigs struct private to the config module as rest of the
system doesn't need to know about it and interacts only with
HagridConfig struct.
This allows for much more convenient cmd binary launching without the need
to remember distinction bitween binaries in main crate (hagrid) and
others (hagridctl, tester) and need to provide additional -- to cargo to
pass arguments to binaries.
Apart from convenience it also provide kinda "documentation" of binary
artifacts in the project as "run" group in just collect all the
artifacts together.
Changes:
- Add new named aliases for `just run` which runs "hagrid" binary with web
server:
- run-hagrid
- hagrid
- Add new recipe to run "hagrid-delete" binary "run-hagrid-delete".
- Alias "run-hagrid-delete" recipe as
- hagrid-delete
- delete
- Add new recipe to run "hagridctl" crate with default binary
"run-hagridctl".
- Alias "run-hagridctl" recipe as
- hagridctl
- Add new recipe to run "tester" crate with default binary "run-tester".
- Alias "run-tester" recipe as
- tester
To install `just` follow instructions in
documentation https://just.systems/man/en/packages.html .
Full list of recipes and their description can be viewed by running
`just` command in project's root directory.
But here is the list of added commands for documentation sake:
- init # Perform initial setup of developer's system.
- init-rocket-config # Copy Rocket's template configuration from Rocket.toml.disk to Rocket.toml. Rocket is Rust web framework. See https://rocket.rs/guide/v0.5/configuration/#configuration
- just-fmt # Format justfile
- cargo-fmt # Format Rust code in all packages (aka path based dependencies)
- fmt # Format all code [alias: f]
- just-lint-fmt # Check justfile formatting
- cargo-lint-fmt # Check Rust code formatting in all packages (aka path based dependencies)
- lint-fmt # Check formatting of all code [alias: lf]
- clippy-lint # Lint Rust code with Clippy [alias: cl]
- lint # Lint all code [alias: l]
- clippy-fix *args # Apply Clippy's lint suggestions, i.e. fix Clippy linting warnings or errors
- cargo-fix *args # Fix compilation warnings by applying compiler suggestions
- fix *args # Fix lint and compilation warnings and errors. Pass given arguments to all sub-recipes, i.e. `just fix --allow-dirty` calls `just cargo-fix --allow-dirty` and `just clippy-fix --allow-dirty`.
- check # Check Rust code errors [alias: c]
- build # Compile Rust code [alias: b]
- test args='--workspace' # Run all tests (i.e. --workspace), but when args given pass them to `cargo test`, e.g. `just test fs::tests::init` [alias: t]
- run # Run web server [alias: r]
- watch-check *args # Run continuous check of Rust code errors. Detect file changes and repeat check automatically. Ctrl+c to exit. You can pass additional arguments, e.g. --notify (-N). [alias: wc]
- watch-run *args # Run web server and automatically restart on changes. Ctrl+c to exit. You can pass additional arguments, e.g. --notify (-N). [alias: wr]
- watch-test *args # Run tests every time files changed. Ctrl+c to exit. You can pass additional arguments, e.g. --notify (-N). [alias: wt]
- clean # Clean compilartion artifacts (i.e. "target" directory)
- clean-translations # Clean changes to translation files
- translate-templates # Translate *.hbs templates of web pages
- db # Open database prompt
This is done for the same reason as for anyhow::Result and
core::result::Result types.
Error is whidely used name and IMO it usually good to keep it standard.
Apart from that I think assosicated type specification looks fore
readable when anyhow::Error is used in place. For example:
type Error = Error;
was replaced with
type Error = anyhow::Error;
Changes:
- Replace usage of imported unquolified Error type with anyhow::Error.
- Remove imports of anyhow::Error.
Result it widely used type and the name is highly overloaded as almost
every library tend to define they own Result type.
When default Result type deviates from default it is usually harder to
read code and requires a bit of investigation work to understand this
particular Result corresponds to.
In Hagrid code base in many places anyhow::Result was made a default
type. It usually was imported through indirection, i.e.
use anyhow::Result;
was imported in files like
- database/src/lib.rs
- hagridctl/src/import.rs
- hagridctl/src/main.rs
- src/main.rs
and in submodules that above import was reimported with something like
use crate::Result;
or
use super::Result;
So, I qualified such Result as anyhow::Result which IMO make code easier
to understand and "anyhow" is short enough name to use along with
Result.
All imports and reimports was cleaned up.
Changes:
- Full qualify anyhow's Result name as anyhow::Result in places where it
is used to make code easier to read and understand.
- Remove imports and reimports of anyhow::Result.
- Restore default Result type to core::result::Result (aka std::result::Result).
- Clean up unnecesary name path qualification for Result type
like "std::result" or "result" as it becomes a default one and doesn't
overriden any more.
Both "extern crate" and #[macro_use] are artifacts of earlier editions
of Rust language. Nowadays we can entirely rely on "use" instead.
Changes:
- Replace "extern crate" with "use" imports.
- Replace
#[macro_use]
extern crate ...;
declarations with "use" imports of used macros. For example,
#[macro_use]
extern crate anyhow;
was replaced with
use anyhow::anyhow;
in every file where anyhow! macro were used.
- Favor direct usage of import path instead of aliased one.
For example, in many places "sequoia_opengpg" were aliased as "openpgp",
during imports replacements I tried to avoid usage of "openpgp" or
introduced additional aliases (like "use sequoia_openpgp as openpgp")
and used "sequoia_opengpg".
I think this way it is easier to understand where name came from
instead of search and jumping to lib.rs or main.rs files trying to
find where name were aliased.
Another example of such favoring is usage of "hagrid_database" over
the "database" in imports.
NOTE: the usage is still inconsistent and requires further clean up.
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: this manual char comparison can be written more succinctly
--> src/web/vks_api.rs:117:37
|
117 | .flat_map(|lang| lang.split(|c| c == '-' || c == ';' || c == '_').next())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['-', ';', '_']`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
= note: `#[warn(clippy::manual_pattern_char_comparison)]` on by default
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: manual implementation of `split_once`
--> src/mail.rs:340:17
|
340 | let mut it = line.splitn(2, ": ");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
341 | let h = it.next().unwrap();
| --------------------------- first usage here
342 | let v = it.next().unwrap();
| --------------------------- second usage here
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
= note: `#[warn(clippy::manual_split_once)]` on by default
help: replace with `split_once`
|
340 ~ let (h, v) = line.split_once(": ").unwrap();
341 ~
342 ~
|
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: allocating a new `String` only to create a temporary `&str` from it
--> database/src/test.rs:109:37
|
109 | let email = Email::from_str(&String::from_utf8(uid.value().to_vec()).unwrap()).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
= note: `#[warn(clippy::unnecessary_to_owned)]` on by default
help: convert from `&[u8]` to `&str` directly
|
109 - let email = Email::from_str(&String::from_utf8(uid.value().to_vec()).unwrap()).unwrap();
109 + let email = Email::from_str(core::str::from_utf8(uid.value()).unwrap()).unwrap();
|
warning: allocating a new `String` only to create a temporary `&str` from it
--> database/src/test.rs:137:37
|
137 | let email = Email::from_str(&String::from_utf8(uid.value().to_vec()).unwrap()).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
help: convert from `&[u8]` to `&str` directly
|
137 - let email = Email::from_str(&String::from_utf8(uid.value().to_vec()).unwrap()).unwrap();
137 + let email = Email::from_str(core::str::from_utf8(uid.value()).unwrap()).unwrap();
|
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: the following explicit lifetimes could be elided: 'a
--> database/src/fs.rs:348:6
|
348 | impl<'a> FilesystemTransaction<'a> {
| ^^ ^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
= note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
|
348 - impl<'a> FilesystemTransaction<'a> {
348 + impl FilesystemTransaction<'_> {
|
warning: the following explicit lifetimes could be elided: 'a
--> database/src/sqlite.rs:141:6
|
141 | impl<'a> DatabaseTransaction<'a> for SqliteTransaction {
| ^^ ^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
|
141 - impl<'a> DatabaseTransaction<'a> for SqliteTransaction {
141 + impl DatabaseTransaction<'_> for SqliteTransaction {
|
warning: the following explicit lifetimes could be elided: 'a
--> src/dump.rs:40:6
|
40 | impl<'a> std::fmt::Display for SessionKeyDisplay<'a> {
| ^^ ^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
= note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
|
40 - impl<'a> std::fmt::Display for SessionKeyDisplay<'a> {
40 + impl std::fmt::Display for SessionKeyDisplay<'_> {
|
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: called `.as_ref().map(|f| f.as_path())` on an `Option` value
--> tester/src/main.rs:78:13
|
78 | output_fprs.as_ref().map(|f| f.as_path()),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using as_deref: `output_fprs.as_deref()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
= note: `#[warn(clippy::option_as_ref_deref)]` on by default
warning: deref which would be done by auto-deref
--> src/web/vks_web.rs:358:52
|
358 | for ValueField { name, value } in Form::values(&*String::from_utf8_lossy(&buf)) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&String::from_utf8_lossy(&buf)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
= note: `#[warn(clippy::explicit_auto_deref)]` on by default
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: the borrowed expression implements the required traits
--> database/src/fs.rs:258:40
|
258 | let typ = fs::symlink_metadata(&path).ok()?.file_type();
| ^^^^^ help: change this to: `path`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
= note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
warning: the borrowed expression implements the required traits
--> database/src/fs.rs:287:44
|
287 | let typ = fs::symlink_metadata(&path)?.file_type();
| ^^^^^ help: change this to: `path`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
warning: the borrowed expression implements the required traits
--> database/src/fs.rs:328:13
|
328 | symlink(&symlink_content, &symlink_name_tmp)?;
| ^^^^^^^^^^^^^^^^ help: change this to: `symlink_content`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
warning: the borrowed expression implements the required traits
--> database/src/fs.rs:329:31
|
329 | rename(&symlink_name_tmp, &symlink_name)?;
| ^^^^^^^^^^^^^ help: change this to: `symlink_name`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
warning: the borrowed expression implements the required traits
--> database/src/fs.rs:334:35
|
334 | if let Ok(target) = read_link(&link) {
| ^^^^^ help: change this to: `link`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> database/src/sqlite.rs:424:46
|
424 | .map(|email| Email::from_str(&email))
| ^^^^^^ help: change this to: `email`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
= note: `#[warn(clippy::needless_borrow)]` on by default
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> database/src/lib.rs:546:51
|
546 | self.set_email_unpublished_filter(&tx, ¤t_fpr, |uid| {
| ^^^ help: change this to: `tx`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> database/src/lib.rs:602:29
|
602 | self.regenerate_wkd(&tx, fpr_primary, &published_tpk_clean)?;
| ^^^ help: change this to: `tx`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> hagridctl/src/import.rs:161:86
|
161 | db.set_email_published(&key_fpr.clone().try_into().unwrap(), &email)
| ^^^^^^ help: change this to: `email`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
= note: `#[warn(clippy::needless_borrow)]` on by default
warning: this expression creates a reference which is immediately dereferenced by the compiler
--> src/sealed_state.rs:20:37
|
20 | let cipher = Aes256Gcm::new(&key);
| ^^^^ help: change this to: `key`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
= note: `#[warn(clippy::needless_borrow)]` on by default
warning: the borrowed expression implements the required traits
--> src/template_helpers.rs:46:77
|
46 | remove_extension(remove_extension(path.strip_prefix(&template_path)?));
| ^^^^^^^^^^^^^^ help: change this to: `template_path`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
= note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
warning: the borrowed expression implements the required traits
--> src/web/vks.rs:216:46
|
216 | Err(e) => return UploadResponse::err(&e.to_string()),
| ^^^^^^^^^^^^^^ help: change this to: `e.to_string()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
warning: the borrowed expression implements the required traits
--> src/web/vks.rs:248:40
|
248 | return UploadResponse::err(&format!("error sending email to {}", &email));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: change this to: `format!("error sending email to {}", &email)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: `flatten()` will run forever if the iterator repeatedly produces an `Err`
--> tester/src/genreqs.rs:12:70
|
12 | let fingerprints: Vec<String> = io::BufReader::new(file).lines().flatten().collect();
| ^^^^^^^^^ help: replace with: `map_while(Result::ok)`
|
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
--> tester/src/genreqs.rs:12:37
|
12 | let fingerprints: Vec<String> = io::BufReader::new(file).lines().flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
= note: `#[warn(clippy::lines_filter_map_ok)]` on by default
Commands:
cargo clippy --tests --no-deps --workspace
Warnings:
warning: this import is redundant
--> database/src/fs.rs:12:1
|
12 | use tempfile;
| ^^^^^^^^^^^^^ help: remove it entirely
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
= note: `#[warn(clippy::single_component_path_imports)]` on by default
It eliminates the following output of `cargo autoinherit` command:
`sequoia-openpgp` won't be auto-inherited because there are multiple sources for it:
- version: =1.17.0
- version: ^1.17.0
Changes:
- Allow "sequoia-openpgp" crate to use workspace dependencies by resolving
version differences constraints in Cargo.toml files.
