From 020b329424b8e72971c0a085f09c8ea159f24e78 Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 12 Dec 2024 15:43:42 -0500 Subject: [PATCH] ci: use cargo-hack and build all valid feature combos (#1653) I've added [cargo-hack](https://github.com/taiki-e/cargo-hack) which seems extremely popular, e.g. futures-util uses it for their CI builds. It actually will go build each of your workspace projects individually. I've setup a build matrix that uses cargo-hack to run builds with all valid feature combos we care about: * default * all-features * wasm32 * tokio * compio * tokio+compio --- .github/workflows/ci.yml | 57 ++++++++++++++++--------------- Cargo.lock | 1 + Cargo.toml | 1 + vortex-io/Cargo.toml | 3 +- vortex-io/src/dispatcher/mod.rs | 38 +++++++++------------ vortex-io/src/dispatcher/tokio.rs | 18 ++++------ 6 files changed, 58 insertions(+), 60 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bcf1610cc5..4475b1e298 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,42 +87,45 @@ jobs: - name: Docs run: cargo doc --no-deps - build-default: - name: "Build (default)" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/cleanup - - uses: rui314/setup-mold@v1 - - uses: ./.github/actions/setup-rust - - name: Rust Build (Default features) - run: cargo build --all-targets - - build-wasm32: - name: "Build (wasm32-unknown-unknown)" + build-rust: + name: "Rust build (${{matrix.config.name}})" runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + config: + - name: "all-features" + command: "build" + args: "--all-features --all-targets" + - name: "default features" + command: "build" + args: "--all-targets" + - name: "with tokio dispatcher" + command: "build" + args: "--no-default-features --features tokio --all-targets --ignore-unknown-features" + - name: "with compio dispatcher" + command: "build" + args: "--no-default-features --features compio --all-targets --ignore-unknown-features" + - name: "with tokio+compio dispatcher" + command: "build" + args: "--no-default-features --features tokio,compio --all-targets --ignore-unknown-features" + - name: "wasm32 with default features" + command: "build" + target: wasm32-unknown-unknown + args: "--target wasm32-unknown-unknown --exclude vortex-roaring --exclude vortex-datafusion " steps: - uses: actions/checkout@v4 - uses: ./.github/actions/cleanup - uses: rui314/setup-mold@v1 - uses: ./.github/actions/setup-rust with: - targets: wasm32-unknown-unknown - - name: Rust Build vortex - run: cargo build -p vortex --target wasm32-unknown-unknown + targets: ${{matrix.config.target || ''}} + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - name: Rust Build (${{matrix.config.name}}) + run: cargo hack ${{matrix.config.command}} ${{matrix.config.args}} --ignore-private - build-all: - name: "Build (all-features)" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/cleanup - - uses: rui314/setup-mold@v1 - - uses: ./.github/actions/setup-rust - - name: Rust Build (All Features) - run: cargo build --all-features --all-targets - rust-test: name: "Rust (tests)" runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index 1835483a49..7725342ff7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5041,6 +5041,7 @@ name = "vortex-io" version = "0.21.0" dependencies = [ "bytes", + "cfg-if", "compio", "flume", "futures", diff --git a/Cargo.toml b/Cargo.toml index 2fa7b20a6d..aac69b7424 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ async-trait = "0.1" backtrace = "0.3.74" bytes = "1.6.0" bzip2 = "0.5.0" +cfg-if = "1" chrono = "0.4.38" clap = "4.5.13" compio = "0.13" diff --git a/vortex-io/Cargo.toml b/vortex-io/Cargo.toml index 6df109c811..4b29ad6f3e 100644 --- a/vortex-io/Cargo.toml +++ b/vortex-io/Cargo.toml @@ -15,9 +15,10 @@ categories.workspace = true [dependencies] bytes = { workspace = true } +cfg-if = { workspace = true } compio = { workspace = true, features = ["bytes", "macros"], optional = true } pin-project = { workspace = true } -# this is the maximum subset of fetaures that is safe for wasm32-wasip1 targets +# this is the maximum subset of fetaures that is safe for wasm32 targets tokio = { workspace = true, features = ["io-util", "rt", "sync"] } tracing = { workspace = true, optional = true } futures = { workspace = true, features = ["std"] } diff --git a/vortex-io/src/dispatcher/mod.rs b/vortex-io/src/dispatcher/mod.rs index c9ac8412e2..56f23fb316 100644 --- a/vortex-io/src/dispatcher/mod.rs +++ b/vortex-io/src/dispatcher/mod.rs @@ -1,6 +1,6 @@ #[cfg(feature = "compio")] mod compio; -#[cfg(feature = "tokio")] +#[cfg(not(target_arch = "wasm32"))] mod tokio; #[cfg(target_arch = "wasm32")] mod wasm; @@ -8,15 +8,14 @@ mod wasm; use std::future::Future; use std::task::Poll; +use cfg_if::cfg_if; use futures::channel::oneshot; use futures::FutureExt; -#[cfg(not(any(feature = "compio", feature = "tokio", target_arch = "wasm32")))] -use vortex_error::vortex_panic; use vortex_error::{vortex_err, VortexResult}; #[cfg(feature = "compio")] use self::compio::*; -#[cfg(feature = "tokio")] +#[cfg(not(target_arch = "wasm32"))] use self::tokio::*; #[cfg(target_arch = "wasm32")] use self::wasm::*; @@ -29,7 +28,7 @@ mod sealed { #[cfg(feature = "compio")] impl Sealed for super::CompioDispatcher {} - #[cfg(feature = "tokio")] + #[cfg(not(target_arch = "wasm32"))] impl Sealed for super::TokioDispatcher {} #[cfg(target_arch = "wasm32")] @@ -89,7 +88,7 @@ impl Future for JoinHandle { #[derive(Debug)] enum Inner { - #[cfg(feature = "tokio")] + #[cfg(not(target_arch = "wasm32"))] Tokio(TokioDispatcher), #[cfg(feature = "compio")] Compio(CompioDispatcher), @@ -98,19 +97,16 @@ enum Inner { } impl Default for IoDispatcher { - #[cfg(target_arch = "wasm32")] fn default() -> Self { - return Self(Inner::Wasm(WasmDispatcher::new())); - } - - #[cfg(not(target_arch = "wasm32"))] - fn default() -> Self { - #[cfg(feature = "tokio")] - return Self(Inner::Tokio(TokioDispatcher::new(1))); - #[cfg(all(feature = "compio", not(feature = "tokio")))] - return Self(Inner::Compio(CompioDispatcher::new(1))); - #[cfg(not(any(feature = "compio", feature = "tokio")))] - vortex_panic!("must enable one of compio or tokio to use IoDispatcher"); + cfg_if! { + if #[cfg(target_arch = "wasm32")] { + Self(Inner::Wasm(WasmDispatcher::new())) + } else if #[cfg(not(feature = "compio"))] { + Self(Inner::Tokio(TokioDispatcher::new(1))) + } else { + Self(Inner::Compio(CompioDispatcher::new(1))) + } + } } } @@ -123,7 +119,7 @@ impl Dispatch for IoDispatcher { R: Send + 'static, { match self.0 { - #[cfg(feature = "tokio")] + #[cfg(not(target_arch = "wasm32"))] Inner::Tokio(ref tokio_dispatch) => tokio_dispatch.dispatch(task), #[cfg(feature = "compio")] Inner::Compio(ref compio_dispatch) => compio_dispatch.dispatch(task), @@ -134,7 +130,7 @@ impl Dispatch for IoDispatcher { fn shutdown(self) -> VortexResult<()> { match self.0 { - #[cfg(feature = "tokio")] + #[cfg(not(target_arch = "wasm32"))] Inner::Tokio(tokio_dispatch) => tokio_dispatch.shutdown(), #[cfg(feature = "compio")] Inner::Compio(compio_dispatch) => compio_dispatch.shutdown(), @@ -150,7 +146,7 @@ impl IoDispatcher { /// /// A handle to the dispatcher can be passed freely among threads, allowing multiple parties to /// perform dispatching across different threads. - #[cfg(feature = "tokio")] + #[cfg(not(target_arch = "wasm32"))] pub fn new_tokio(num_thread: usize) -> Self { Self(Inner::Tokio(TokioDispatcher::new(num_thread))) } diff --git a/vortex-io/src/dispatcher/tokio.rs b/vortex-io/src/dispatcher/tokio.rs index d6c0bd8e66..06bc328574 100644 --- a/vortex-io/src/dispatcher/tokio.rs +++ b/vortex-io/src/dispatcher/tokio.rs @@ -118,28 +118,24 @@ impl Dispatch for TokioDispatcher { #[cfg(test)] mod tests { - use std::io::Write; - - use tempfile::NamedTempFile; + use std::sync::atomic::{AtomicU32, Ordering}; + use std::sync::Arc; use super::TokioDispatcher; use crate::dispatcher::Dispatch; - use crate::{TokioFile, VortexReadAt}; #[tokio::test] async fn test_tokio_dispatch_simple() { let dispatcher = TokioDispatcher::new(4); - let mut tmpfile = NamedTempFile::new().unwrap(); - write!(tmpfile, "5678").unwrap(); - + let atomic_number = Arc::new(AtomicU32::new(0)); + let atomic_number_clone = Arc::clone(&atomic_number); let rx = dispatcher .dispatch(|| async move { - let file = TokioFile::open(tmpfile.path()).unwrap(); - - file.read_byte_range(0, 4).await.unwrap() + atomic_number_clone.fetch_add(1, Ordering::SeqCst); }) .unwrap(); - assert_eq!(&rx.await.unwrap(), "5678".as_bytes()); + rx.await.unwrap(); + assert_eq!(atomic_number.load(Ordering::SeqCst), 1u32); } }