Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: finalize feature gating on component #4895

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion crates/core/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/dex/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
12 changes: 8 additions & 4 deletions crates/core/component/dex/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want to ever feature gate the events!! Pindexer wants to read these without pulling in cnidarium and rocksdb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, we should align imports and not the exports. But I'm confused about why the proposed breakage isn't detected at build time: I tried cherry-picking this commit onto the dex PRs you reference (#4894) and I was able to build pindexer just fine. That's not the same as pindexer actually working, of course, but I'd expect mucking around with the event interface would break things early.

pub mod state_key;
}
}
pub mod genesis;
pub mod state_key;

mod batch_swap_output_data;
mod candlestick;
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/ibc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub mod genesis;
mod ibc_action;
mod ibc_token;
pub mod params;
#[cfg(feature = "component")]
mod version;

mod prefix;
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/shielded-pool/src/fmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion crates/core/component/shielded-pool/src/ics20_withdrawal.rs
Original file line number Diff line number Diff line change
@@ -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;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes me suspect I'm optimizing for the disabling warnings, rather than meaningfully structuring the imports.

use penumbra_asset::{
asset::{self, Metadata},
Balance, Value,
Expand Down
2 changes: 2 additions & 0 deletions crates/core/component/shielded-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions crates/core/component/stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
mod changes;
mod current_consensus_keys;
mod delegation_token;
#[cfg(feature = "component")]
mod event;
mod governance_key;
mod identity_key;
Expand All @@ -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;
Expand Down
53 changes: 53 additions & 0 deletions deployments/scripts/rust-check
Original file line number Diff line number Diff line change
@@ -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 <foo>`.
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 <foo>`.
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
4 changes: 2 additions & 2 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading