From 0e69b3e6aceef7d1fc2488aede4431581c442d0c Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Wed, 9 Oct 2024 22:59:48 -0700 Subject: [PATCH 1/2] build: finalize feature gating on component Follow-up to #4892. During review of that PR, I noticed that `cargo run -p pcli` was emitting a lot of warnings about unused imports. Here I try to address those warnings, by adding more conditional imports based on the `component` feature. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/core/app/Cargo.toml | 2 +- crates/core/component/dex/Cargo.toml | 1 + crates/core/component/dex/src/lib.rs | 12 ++++++++---- crates/core/component/ibc/src/lib.rs | 1 + crates/core/component/shielded-pool/src/fmd.rs | 1 + .../component/shielded-pool/src/ics20_withdrawal.rs | 6 +++++- crates/core/component/shielded-pool/src/lib.rs | 2 ++ crates/core/component/stake/src/lib.rs | 2 ++ 10 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 367c0504aa..ff36c93b74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4856,6 +4856,7 @@ dependencies = [ "base64 0.21.7", "bincode", "blake2b_simd 1.0.2", + "cfg-if", "cnidarium", "cnidarium-component", "decaf377", diff --git a/Cargo.toml b/Cargo.toml index 7ae2d4d471..705b0796df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -130,6 +130,7 @@ bitvec = { version = "1" } blake2b_simd = { version = "1" } bytes = { version = "1.2" } camino = { version = "1" } +cfg-if = { version = "1" } chacha20poly1305 = { version = "0.9.0" } chrono = { default-features = false, version = "0.4" } clap = { version = "3.2" } diff --git a/crates/core/app/Cargo.toml b/crates/core/app/Cargo.toml index 9013371d07..883ae6998a 100644 --- a/crates/core/app/Cargo.toml +++ b/crates/core/app/Cargo.toml @@ -42,7 +42,7 @@ bech32 = { workspace = true } bincode = { workspace = true } bitvec = { workspace = true } blake2b_simd = { workspace = true } -cfg-if = "1.0" +cfg-if = { workspace = true } cnidarium = { workspace = true, optional = true, features = ["migration", "rpc"], default-features = true } cnidarium-component = { workspace = true, optional = true, default-features = true } decaf377 = { workspace = true, default-features = true } diff --git a/crates/core/component/dex/Cargo.toml b/crates/core/component/dex/Cargo.toml index d4eb923904..ffd7e3ea7a 100644 --- a/crates/core/component/dex/Cargo.toml +++ b/crates/core/component/dex/Cargo.toml @@ -40,6 +40,7 @@ async-trait = {workspace = true} base64 = {workspace = true} bincode = {workspace = true} blake2b_simd = {workspace = true} +cfg-if = {workspace = true} cnidarium = {workspace = true, optional = true, default-features = true} cnidarium-component = {workspace = true, optional = true, default-features = true} decaf377 = {workspace = true, features = ["r1cs"], default-features = true} diff --git a/crates/core/component/dex/src/lib.rs b/crates/core/component/dex/src/lib.rs index d20ee807b4..8e953506e7 100644 --- a/crates/core/component/dex/src/lib.rs +++ b/crates/core/component/dex/src/lib.rs @@ -1,11 +1,15 @@ #![deny(clippy::unwrap_used)] // Requires nightly. #![cfg_attr(docsrs, feature(doc_auto_cfg))] -#[cfg(feature = "component")] -pub mod component; -pub mod event; + +cfg_if::cfg_if! { + if #[cfg(feature="component")] { + pub mod component; + pub mod event; + pub mod state_key; + } +} pub mod genesis; -pub mod state_key; mod batch_swap_output_data; mod candlestick; diff --git a/crates/core/component/ibc/src/lib.rs b/crates/core/component/ibc/src/lib.rs index 172d74d2e2..8b3e281db9 100644 --- a/crates/core/component/ibc/src/lib.rs +++ b/crates/core/component/ibc/src/lib.rs @@ -15,6 +15,7 @@ pub mod genesis; mod ibc_action; mod ibc_token; pub mod params; +#[cfg(feature = "component")] mod version; mod prefix; diff --git a/crates/core/component/shielded-pool/src/fmd.rs b/crates/core/component/shielded-pool/src/fmd.rs index eb2fa46bdd..c822d5e457 100644 --- a/crates/core/component/shielded-pool/src/fmd.rs +++ b/crates/core/component/shielded-pool/src/fmd.rs @@ -6,6 +6,7 @@ use penumbra_proto::{ }; use serde::{Deserialize, Serialize}; +#[cfg(feature = "component")] pub mod state_key; /// How long users have to switch to updated parameters. diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index c452d5d703..c05682328e 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -1,4 +1,8 @@ -use ibc_types::core::{channel::ChannelId, channel::PortId, client::Height as IbcHeight}; +use ibc_types::core::{channel::ChannelId, client::Height as IbcHeight}; + +#[cfg(feature = "component")] +use ibc_types::core::channel::PortId; + use penumbra_asset::{ asset::{self, Metadata}, Balance, Value, diff --git a/crates/core/component/shielded-pool/src/lib.rs b/crates/core/component/shielded-pool/src/lib.rs index 84d112ed52..dca250dfb4 100644 --- a/crates/core/component/shielded-pool/src/lib.rs +++ b/crates/core/component/shielded-pool/src/lib.rs @@ -6,10 +6,12 @@ pub mod component; pub mod ics20_withdrawal; pub use ics20_withdrawal::Ics20Withdrawal; +#[cfg(feature = "component")] pub mod event; pub mod fmd; pub mod genesis; pub mod params; +#[cfg(feature = "component")] pub mod state_key; pub mod note; diff --git a/crates/core/component/stake/src/lib.rs b/crates/core/component/stake/src/lib.rs index eb574260df..6592dab38d 100644 --- a/crates/core/component/stake/src/lib.rs +++ b/crates/core/component/stake/src/lib.rs @@ -6,6 +6,7 @@ mod changes; mod current_consensus_keys; mod delegation_token; +#[cfg(feature = "component")] mod event; mod governance_key; mod identity_key; @@ -18,6 +19,7 @@ pub mod funding_stream; pub mod genesis; pub mod params; pub mod rate; +#[cfg(feature = "component")] pub mod state_key; pub mod undelegate; pub mod undelegate_claim; From e77549e7cfa064bcc0095317cd10e547d73878e6 Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Thu, 10 Oct 2024 09:43:18 -0700 Subject: [PATCH 2/2] ci: custom rust-check script Uses the usual `cargo check`, failing on warnings, and adds a custom target-dir for caching the check results alongside the usual debug/release targets. Doing so enables fast rechecks from local cache. Lots of disk space required: ~50GB for the `./target/check/` directory. This is why I'm not bolting it to CI right now. When cache is warm locally, the whole `just check` flow takes ~6s. --- deployments/scripts/rust-check | 53 ++++++++++++++++++++++++++++++++++ justfile | 4 +-- 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100755 deployments/scripts/rust-check diff --git a/deployments/scripts/rust-check b/deployments/scripts/rust-check new file mode 100755 index 0000000000..555fbe6daa --- /dev/null +++ b/deployments/scripts/rust-check @@ -0,0 +1,53 @@ +#!/bin/bash +# CI script to run `cargo check` in various forms, to ensure +# the Rust source code in the project has no warnings. +# +# We use a custom `--target-dir` flag for the cargo commands, +# to cache the check results: because the fail-on-warnings behavior +# is specified via RUSTFLAGS, cargo will not reuse the build cache +# for invocations with different (or missing) RUSTFLAGS. +set -euo pipefail + +# Ensure all warnings are treated as errors. +export RUSTFLAGS="-D warnings" + +# Emit all crate package names that contain `bin` targets. +# for building via `cargo build --package `. +function get_crates_with_binaries() { + cargo metadata --format-version=1 | jq -r ' + .workspace_members[] as $member | + .packages[] | + select(.id == $member) | + select(.targets[] | .kind[] | contains("bin")) | + .name + ' | sort -u +} + +# Emit all binary names for crates defined within the workspace, +# for building via `cargo build --bin `. +function get_workspace_binary_names() { + cargo metadata --format-version=1 | jq ' + .workspace_members[] as $member | + .packages[] | + select(.id == $member) | + .targets[] | + select(.kind[] | contains("bin")) | + .name' | sort -u +} + + +# Primary script entrypoint +function main() { + >&2 echo "Checking all rust source code in workspace for warnings..." + cargo check --target-dir ./target/check --release --all-targets + >&2 echo "Checking warnings for binary crates via '--package'..." + get_crates_with_binaries \ + | xargs -I{} \ + cargo check -q --release --target-dir ./target/check --package {} + >&2 echo "Checking warnings for binary crates via '--bin'..." + get_workspace_binary_names \ + | xargs -I{} \ + cargo check -q --release --target-dir ./target/check --bin {} +} + +main diff --git a/justfile b/justfile index e7c72fe9c6..087090cb1c 100644 --- a/justfile +++ b/justfile @@ -15,9 +15,9 @@ dev: fmt: cargo fmt --all -# Runs 'cargo check' on all rust files in the project. +# Runs 'cargo check' on all rust files in the project, failing on warnings check: - RUSTFLAGS="-D warnings" cargo check --release --all-targets + ./deployments/scripts/rust-check # Render livereload environment for editing the Protocol documentation. protocol-docs: