starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 11:15:54 +03:00

[GH-ISSUE #2212] `just clippy` does not catch warnings produced by `just dnssec-openssl` #925

New issue

Closed

opened 2026-03-16 00:58:38 +03:00 by kerem · 6 comments

kerem commented

2026-03-16 00:58:38 +03:00

Owner

Originally created by @japaric on GitHub (May 15, 2024).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2212

Describe the bug
what the title says

To Reproduce
Steps to reproduce the behavior:

$ # from the root of the repository
$ just dnssec-openssl
cargo ws exec --ignore=\{async-std-resolver,hickory-compatibility\} cargo  check --all-targets --benches --examples --bins --tests --features=dnssec-openssl
(..)
warning: unused import: `time::Duration`
 --> tests/integration-tests/tests/client_tests.rs:9:5
  |
9 | use time::Duration;
  |     ^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `time::Duration`
 --> tests/integration-tests/tests/client_future_tests.rs:9:5
  |
9 | use time::Duration;
  |     ^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `hickory_proto::rr::Record`
  --> tests/integration-tests/tests/client_tests.rs:27:5
   |
27 | use hickory_proto::rr::Record;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused imports: `Record`, `dnssec::SigSigner`
  --> tests/integration-tests/tests/client_future_tests.rs:32:25
   |
32 | use hickory_proto::rr::{dnssec::SigSigner, Record};
   |                         ^^^^^^^^^^^^^^^^^  ^^^^^^

warning: unused imports: `DnsExchangeBackground`, `DnsMultiplexer`
  --> tests/integration-tests/tests/client_future_tests.rs:34:27
   |
34 | use hickory_proto::xfer::{DnsExchangeBackground, DnsMultiplexer};
   |                           ^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^
(..)

$ just clippy && echo OK
(..)
OK

Expected behavior
just clippy should catch the warnings shown when one runs just dnssec-openssl and similar tasks.

System:

OS: Arch Linux
Architecture: x86_64
Version N/A
rustc version: 1.76.0

Version:
Crate: whole repository
Version: 97e1f434

Additional context

Seems like just clippy only test two combinations of Cargo features: all features disabled; and all features enabled.

To test the combinations of other Cargo features a tool like cargo-hack could be used. I know the rustls project uses it to test combinations of Cargo features but I have not used it myself.

Originally created by @japaric on GitHub (May 15, 2024). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2212 **Describe the bug** what the title says **To Reproduce** Steps to reproduce the behavior: ``` console $ # from the root of the repository $ just dnssec-openssl cargo ws exec --ignore=\{async-std-resolver,hickory-compatibility\} cargo check --all-targets --benches --examples --bins --tests --features=dnssec-openssl (..) warning: unused import: `time::Duration` --> tests/integration-tests/tests/client_tests.rs:9:5 | 9 | use time::Duration; | ^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default warning: unused import: `time::Duration` --> tests/integration-tests/tests/client_future_tests.rs:9:5 | 9 | use time::Duration; | ^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default warning: unused import: `hickory_proto::rr::Record` --> tests/integration-tests/tests/client_tests.rs:27:5 | 27 | use hickory_proto::rr::Record; | ^^^^^^^^^^^^^^^^^^^^^^^^^ warning: unused imports: `Record`, `dnssec::SigSigner` --> tests/integration-tests/tests/client_future_tests.rs:32:25 | 32 | use hickory_proto::rr::{dnssec::SigSigner, Record}; | ^^^^^^^^^^^^^^^^^ ^^^^^^ warning: unused imports: `DnsExchangeBackground`, `DnsMultiplexer` --> tests/integration-tests/tests/client_future_tests.rs:34:27 | 34 | use hickory_proto::xfer::{DnsExchangeBackground, DnsMultiplexer}; | ^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^ (..) ``` ``` console $ just clippy && echo OK (..) OK ``` **Expected behavior** `just clippy` should catch the warnings shown when one runs `just dnssec-openssl` and similar tasks. **System:** - OS: Arch Linux - Architecture: x86_64 - Version N/A - rustc version: 1.76.0 **Version:** Crate: whole repository Version: 97e1f434 **Additional context** Seems like `just clippy` only test two combinations of Cargo features: all features disabled; and all features enabled. To test the combinations of other Cargo features a tool like [`cargo-hack`](https://crates.io/crates/cargo-hack) could be used. I know the rustls project uses it to test combinations of Cargo features but I have not used it myself.

kerem

2026-03-16 00:58:38 +03:00

closed this issue
added the
ops

cleanup
labels

kerem commented

2026-03-16 00:58:54 +03:00

Author

Owner

@djc commented on GitHub (May 15, 2024):

I'd be open to a PR that introduces the use of cargo-hack similar to what we have in the rustls repo. Might want a separate job for it since it could be pretty slow?

(If you just want to fix the dnssec-openssl warnings, that might be a good start, too.)

@bluejekyll I think you've mentioned wanting to just get rid of OpenSSL-based stuff before, how are you feeling about that project now?

@djc commented on GitHub (May 15, 2024): I'd be open to a PR that introduces the use of cargo-hack similar to what we have in the rustls repo. Might want a separate job for it since it could be pretty slow? (If you just want to fix the `dnssec-openssl` warnings, that might be a good start, too.) @bluejekyll I think you've mentioned wanting to just get rid of OpenSSL-based stuff before, how are you feeling about that project now?

kerem commented

2026-03-16 00:58:59 +03:00

Author

Owner

@bluejekyll commented on GitHub (May 18, 2024):

I keep hesitating on the removal of OpenSSL because of some of the compliance it has that we're still waiting on for Rustle. I think we should just clean it up instead of fixing up testing related to it, but it wouldn't be hard to fix this.

Today we only test these feature flags directly with clippy:

clippy:
    find {{justfile_directory()}} -name '*.rs' -exec touch {} \;
    just clippy-inner --no-default-features
    just clippy-inner
    just clippy-inner --all-features

We could add this after --all-features, just clippy-inner --features=dnssec-openssl. OpenSSL generally doesn't get caught, because where they conflict, Rustls takes precedent.

@bluejekyll commented on GitHub (May 18, 2024): I keep hesitating on the removal of OpenSSL because of some of the compliance it has that we're still waiting on for Rustle. I think we should just clean it up instead of fixing up testing related to it, but it wouldn't be hard to fix this. Today we only test these feature flags directly with clippy: ``` clippy: find {{justfile_directory()}} -name '*.rs' -exec touch {} \; just clippy-inner --no-default-features just clippy-inner just clippy-inner --all-features ``` We could add this after `--all-features`, `just clippy-inner --features=dnssec-openssl`. OpenSSL generally doesn't get caught, because where they conflict, Rustls takes precedent.

kerem commented

2026-03-16 00:59:05 +03:00

Author

Owner

@djc commented on GitHub (May 18, 2024):

I keep hesitating on the removal of OpenSSL because of some of the compliance it has that we're still waiting on for Rustle. I think we should just clean it up instead of fixing up testing related to it, but it wouldn't be hard to fix this.

What kind of compliance are we waiting for? I think Rustls is in a pretty good place today.

@djc commented on GitHub (May 18, 2024): > I keep hesitating on the removal of OpenSSL because of some of the compliance it has that we're still waiting on for Rustle. I think we should just clean it up instead of fixing up testing related to it, but it wouldn't be hard to fix this. What kind of compliance are we waiting for? I think Rustls is in a pretty good place today.

kerem commented

2026-03-16 00:59:10 +03:00

Author

Owner

@bluejekyll commented on GitHub (May 18, 2024):

What kind of compliance are we waiting for?

Primarily, FIPS compliance, does this announcement mean that Rustls is FIPS compliant now? https://www.memorysafety.org/blog/rustls-with-aws-crypto-back-end-and-fips/

@bluejekyll commented on GitHub (May 18, 2024): > What kind of compliance are we waiting for? Primarily, FIPS compliance, does this announcement mean that Rustls is FIPS compliant now? https://www.memorysafety.org/blog/rustls-with-aws-crypto-back-end-and-fips/

kerem commented

2026-03-16 00:59:15 +03:00

Author

Owner

@djc commented on GitHub (May 18, 2024):

Yes, Rustls from 0.22 onwards supports the aws-lc-rs crypto provider which has a FIPS mode available (on Linux only).

@djc commented on GitHub (May 18, 2024): Yes, Rustls from 0.22 onwards supports the aws-lc-rs crypto provider which has a FIPS mode available (on Linux only).

kerem commented

2026-03-16 00:59:20 +03:00

Author

Owner

@djc commented on GitHub (Jan 10, 2025):

Support for OpenSSL was removed: