Skip to content

Make Trin compilable/(not crash) on Windows + Add check CI #923

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

Merged
merged 3 commits into from
Sep 26, 2023
Merged
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
27 changes: 27 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ commands:
command: sudo apt install clang
orbs:
rust: circleci/rust@1.6.0
win: circleci/windows@2.2.0
executors:
docker-publisher:
environment:
Expand Down Expand Up @@ -159,6 +160,31 @@ jobs:
name: Build Trin workspace
command: cargo build --workspace
- save-sccache-cache
check-windows:
description: |
Check the crate on Windows (Check will tell us if we can build on Windows without the need to codegen (which is the time consuming part)).
executor:
name: win/default
size: xlarge
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
steps:
- checkout
- run:
name: Install rustup and clang
# We are installing them at the same time because it is faster
# todo: Remove --ignore-checksums flag
command: choco install rustup.install llvm -y --ignore-checksums
- run:
name: Add target
command: rustup target add x86_64-pc-windows-msvc
- run:
name: Install target toolchain
command: rustup toolchain install stable-x86_64-pc-windows-msvc
- run:
name: Check Trin workspace
command: cargo check --workspace
test:
description: |
Run tests.
Expand Down Expand Up @@ -259,4 +285,5 @@ workflows:
- lint
- build
- test
- check-windows
- utp-test
1 change: 1 addition & 0 deletions ethportal-peertest/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(unix)]
pub mod constants;
pub mod scenarios;
pub mod utils;
Expand Down
2 changes: 1 addition & 1 deletion portalnet/src/overlay_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ where
Request::PopulatedOffer(offer) => Ok(response_clone
.content_keys
.iter()
.zip(offer.content_items.into_iter())
.zip(offer.content_items)
.filter(|(is_accepted, _item)| *is_accepted)
.map(|(_is_accepted, (_key, val))| val)
.collect()),
Expand Down
2 changes: 2 additions & 0 deletions portalnet/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,8 @@ pub mod test {
// The restarted store should have the same radius as the original
assert_eq!(radius, new_storage.radius);

drop(storage);
drop(new_storage);
Comment on lines +1143 to +1144
Copy link
Contributor

Choose a reason for hiding this comment

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

Why were the explicit drops necessary?

Copy link
Member Author

@KolbyML KolbyML Sep 19, 2023

Choose a reason for hiding this comment

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

They are in the tests below and above. This change isn't related to the PR. Without these drops the test will panic on certain operating systems. If you read the doc for .close() on the line below there is a comment in the doc's example about this.

temp_dir.close()?;
Ok(())
}
Expand Down
9 changes: 9 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use tokio::sync::RwLock;
use tracing::info;
use utp_rs::socket::UtpSocket;

#[cfg(windows)]
use ethportal_api::types::cli::Web3TransportType;
use ethportal_api::types::cli::{TrinConfig, BEACON_NETWORK, HISTORY_NETWORK, STATE_NETWORK};
use portalnet::{
discovery::{Discovery, Discv5UdpSocket},
Expand All @@ -25,6 +27,13 @@ use trin_validation::{accumulator::MasterAccumulator, oracle::HeaderOracle};
pub async fn run_trin(
trin_config: TrinConfig,
) -> Result<RpcServerHandle, Box<dyn std::error::Error>> {
// Panic early on a windows build that is trying to use IPC, which is unsupported for now
// Make sure not to panic on non-windows configurations.
#[cfg(windows)]
if let Web3TransportType::IPC = trin_config.web3_transport {
panic!("Tokio doesn't support Windows Unix Domain Sockets IPC, use --web3-transport http");
}

let trin_version = get_trin_version();
info!("Launching Trin: v{trin_version}");
info!(config = %trin_config, "With:");
Expand Down
1 change: 1 addition & 0 deletions tests/rpc_server.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(unix)]
/// Test that a 3rd-party web3 client can understand our JSON-RPC API
use std::net::{IpAddr, Ipv4Addr};

Expand Down
1 change: 1 addition & 0 deletions tests/self_peertest.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(unix)]
use rpc::RpcServerHandle;
use std::env;
use std::net::{IpAddr, Ipv4Addr};
Expand Down