Add a gh-action and buildomat jobs to cargo check on no-default-features and feature-powerset

zeeshanlakhani · zeeshanlakhani · commit 4e23c525d495 · 2024-07-09T22:39:33.000Z
Includes:
  - fixes around feature-flagging, particularly in oxql, as well as uuid-kinds, sled-storage
diff --git a/.cargo/config.toml b/.cargo/config.toml
@@ -8,6 +8,7 @@
 # CI scripts:
 # - .github/buildomat/build-and-test.sh
 # - .github/buildomat/jobs/clippy.sh
+# - .github/buildomat/jobs/check-features.sh
 # - .github/workflows/rust.yml
 #
 [build]
diff --git a/.github/buildomat/jobs/check-features.sh b/.github/buildomat/jobs/check-features.sh
@@ -0,0 +1,36 @@
+#!/bin/bash
+#:
+#: name = "check-features (helios)"
+#: variety = "basic"
+#: target = "helios-2.0"
+#: rust_toolchain = true
+#: output_rules = []
+
+# Run cargo check on illumos with feature-specifics like `no-default-features`.
+
+set -o errexit
+set -o pipefail
+set -o xtrace
+
+cargo --version
+rustc --version
+
+#
+# Set up our PATH for use with this workspace.
+#
+source ./env.sh
+
+banner prerequisites
+ptime -m bash ./tools/install_builder_prerequisites.sh -y
+
+banner check
+export CARGO_INCREMENTAL=0
+ptime -m cargo check --workspace --bins --tests --no-default-features
+RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps --no-default-features
+
+#
+# `cargo-hack` check feature-powerset
+#
+banner hack
+cargo install cargo-hack --locked
+cargo hack check --workspace --feature-powerset --no-dev-deps --exclude-features image-trampoline,image-standard
diff --git a/.github/buildomat/jobs/clippy.sh b/.github/buildomat/jobs/clippy.sh
@@ -10,7 +10,7 @@
 # (that we want to check) is conditionally-compiled on illumos only.
 #
 # Note that `cargo clippy` includes `cargo check, so this ends up checking all
-# of our code.
+# of our (default) code.
 
 set -o errexit
 set -o pipefail
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -53,7 +53,7 @@ jobs:
       run: cargo run --bin omicron-package -- -t default check
 
   # Note that `cargo clippy` includes `cargo check, so this ends up checking all
-  # of our code.
+  # of our (default) code.
   clippy-lint:
     runs-on: ubuntu-22.04
     env:
@@ -82,6 +82,36 @@ jobs:
     - name: Run Clippy Lints
       run: cargo xtask clippy
 
+  check-features:
+    runs-on: ubuntu-22.04
+    env:
+      CARGO_INCREMENTAL: 0
+    steps:
+    # This repo is unstable and unnecessary: https://github.com/microsoft/linux-package-repositories/issues/34
+    - name: Disable packages.microsoft.com repo
+      run: sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list
+    - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+      with:
+        ref: ${{ github.event.pull_request.head.sha }} # see omicron#4461
+    - uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2.7.3
+      if: ${{ github.ref != 'refs/heads/main' }}
+    - name: Report cargo version
+      run: cargo --version
+    - name: Update PATH
+      run: source "./env.sh"; echo "PATH=$PATH" >> "$GITHUB_ENV"
+    - name: Print PATH
+      run: echo $PATH
+    - name: Print GITHUB_ENV
+      run: cat "$GITHUB_ENV"
+    - name: Install Pre-Requisites
+      run: ./tools/install_builder_prerequisites.sh -y
+    - name: Run Cargo Check (No Default Features)
+      run: cargo check --workspace --bins --tests --no-default-features
+    - name: Install cargo-hack
+      uses: taiki-e/install-action@cargo-hack
+    - name: Run Cargo Hack Check (Feature-Powerset, No-Dev-Deps)
+      run: cargo hack check --workspace --feature-powerset --no-dev-deps --exclude-features image-trampoline,image-standard
+
   # This is just a test build of docs.  Publicly available docs are built via
   # the separate "rustdocs" repo.
   build-docs:
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml
@@ -51,7 +51,7 @@ num-integer.workspace = true
 once_cell.workspace = true
 openssl.workspace = true
 oximeter-client.workspace = true
-oximeter-db.workspace = true
+oximeter-db = { workspace = true, default-features = false, features = [ "oxql" ] }
 oxnet.workspace = true
 parse-display.workspace = true
 paste.workspace = true
diff --git a/oximeter/db/Cargo.toml b/oximeter/db/Cargo.toml
@@ -19,6 +19,7 @@ clap.workspace = true
 dropshot.workspace = true
 futures.workspace = true
 highway.workspace = true
+num.workspace = true
 omicron-common.workspace = true
 omicron-workspace-hack.workspace = true
 oximeter.workspace = true
@@ -45,10 +46,6 @@ optional = true
 workspace = true
 optional = true
 
-[dependencies.num]
-workspace = true
-optional = true
-
 [dependencies.peg]
 workspace = true
 optional = true
@@ -91,6 +88,7 @@ indexmap.workspace = true
 itertools.workspace = true
 omicron-test-utils.workspace = true
 slog-dtrace.workspace = true
+sqlformat.workspace = true
 sqlparser.workspace = true
 strum.workspace = true
 tempfile.workspace = true
@@ -105,9 +103,11 @@ sql = [
     "dep:sqlparser",
     "dep:tabled"
 ]
+oxdb = [
+    "dep:tabled"
+]
 oxql = [
     "dep:crossterm",
-    "dep:num",
     "dep:peg",
     "dep:reedline",
     "dep:tabled",
@@ -116,3 +116,4 @@ oxql = [
 [[bin]]
 name = "oxdb"
 doc = false
+required-features = ["oxdb"]
diff --git a/oximeter/db/src/bin/oxdb/main.rs b/oximeter/db/src/bin/oxdb/main.rs
@@ -148,6 +148,7 @@ enum Subcommand {
     },
 
     /// Enter the Oximeter Query Language shell for interactive querying.
+    #[cfg(feature = "oxql")]
     Oxql {
         #[clap(flatten)]
         opts: oximeter_db::shells::oxql::ShellOptions,
@@ -350,6 +351,7 @@ async fn main() -> anyhow::Result<()> {
             oximeter_db::shells::sql::shell(args.address, args.port, log, opts)
                 .await?
         }
+        #[cfg(feature = "oxql")]
         Subcommand::Oxql { opts } => {
             oximeter_db::shells::oxql::shell(args.address, args.port, log, opts)
                 .await?
diff --git a/oximeter/db/src/client/mod.rs b/oximeter/db/src/client/mod.rs
@@ -7,6 +7,7 @@
 // Copyright 2024 Oxide Computer Company
 
 pub(crate) mod dbwrite;
+#[cfg(any(feature = "oxql", test))]
 pub(crate) mod oxql;
 pub(crate) mod query_summary;
 #[cfg(any(feature = "sql", test))]
diff --git a/oximeter/db/src/lib.rs b/oximeter/db/src/lib.rs
@@ -35,11 +35,12 @@ pub mod model;
 #[cfg(any(feature = "oxql", test))]
 pub mod oxql;
 pub mod query;
-#[cfg(any(feature = "oxql", feature = "sql", test))]
+#[cfg(any(feature = "oxql", feature = "sql", feature = "oxdb", test))]
 pub mod shells;
 #[cfg(any(feature = "sql", test))]
 pub mod sql;
 
+#[cfg(any(feature = "oxql", test))]
 pub use client::oxql::OxqlResult;
 pub use client::query_summary::QuerySummary;
 pub use client::Client;
@@ -142,10 +143,12 @@ pub enum Error {
     #[error("SQL error")]
     Sql(#[from] sql::Error),
 
+    #[cfg(any(feature = "oxql", test))]
     #[error(transparent)]
     Oxql(oxql::Error),
 }
 
+#[cfg(any(feature = "oxql", test))]
 impl From<crate::oxql::Error> for Error {
     fn from(e: crate::oxql::Error) -> Self {
         Error::Oxql(e)
diff --git a/oximeter/db/src/oxql/mod.rs b/oximeter/db/src/oxql/mod.rs
@@ -19,7 +19,7 @@ pub use self::table::Table;
 pub use self::table::Timeseries;
 pub use anyhow::Error;
 
-// Format a PEG parsing error into a nice anyhow error.
+/// Format a PEG parsing error into a nice anyhow error.
 fn fmt_parse_error(source: &str, err: PegError<LineCol>) -> Error {
     use std::fmt::Write;
     let mut out =
diff --git a/oximeter/db/src/query.rs b/oximeter/db/src/query.rs
@@ -371,10 +371,12 @@ impl FieldSelector {
     }
 }
 
-/// A stringly-typed selector for finding fields by name and comparsion with a given value.
+/// A stringly-typed selector for finding fields by name and comparsion with a
+/// given value.
 ///
-/// This is used internally to parse comparisons written as strings, such as from the `oxdb`
-/// command-line tool or from another external source (Nexus API, for example).
+/// This is used internally to parse comparisons written as strings, such as
+/// from the `oxdb` command-line tool or from another external
+/// source (Nexus API, for example).
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
 pub struct StringFieldSelector {
     name: String,
diff --git a/oximeter/instruments/Cargo.toml b/oximeter/instruments/Cargo.toml
@@ -36,6 +36,7 @@ http-instruments = [
     "dep:oximeter",
     "dep:schemars",
     "dep:serde",
+    "dep:slog",
     "dep:uuid"
 ]
 kstat = [
diff --git a/sled-storage/src/manager_test_harness.rs b/sled-storage/src/manager_test_harness.rs
@@ -123,6 +123,7 @@ impl Drop for StorageManagerTestHarness {
 impl StorageManagerTestHarness {
     /// Creates a new StorageManagerTestHarness with no associated disks.
     pub async fn new(log: &Logger) -> Self {
+        #[cfg(all(test, feature = "testing"))]
         illumos_utils::USE_MOCKS.store(false, Ordering::SeqCst);
         let tmp = camino_tempfile::tempdir_in("/var/tmp")
             .expect("Failed to make temporary directory");
diff --git a/sled-storage/src/pool.rs b/sled-storage/src/pool.rs
@@ -27,7 +27,7 @@ impl Pool {
     }
 
     /// Return a Pool consisting of fake info
-    #[cfg(feature = "testing")]
+    #[cfg(all(test, feature = "testing"))]
     pub fn new_with_fake_info(name: ZpoolName, parent: DiskIdentity) -> Pool {
         let info = ZpoolInfo::new_hardcoded(name.to_string());
         Pool { name, info, parent }
diff --git a/uuid-kinds/src/lib.rs b/uuid-kinds/src/lib.rs
@@ -2,26 +2,26 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-#![cfg_attr(not(feature = "std"), no_std)]
-
 //! A registry for UUID kinds used in Omicron and related projects.
 //!
 //! See this crate's `README.adoc` for more information.
 
+#![cfg_attr(not(feature = "std"), no_std)]
+
 // Export these types so that other users don't have to pull in newtype-uuid.
 #[doc(no_inline)]
 pub use newtype_uuid::{
     GenericUuid, ParseError, TagError, TypedUuid, TypedUuidKind, TypedUuidTag,
 };
 
-#[cfg(feature = "schemars08")]
+#[cfg(all(feature = "schemars08", feature = "std"))]
 use schemars::JsonSchema;
 
 macro_rules! impl_typed_uuid_kind {
     ($($kind:ident => $tag:literal),* $(,)?) => {
         $(
             paste::paste! {
-                #[cfg_attr(feature = "schemars08", derive(JsonSchema))]
+                #[cfg_attr(all(feature = "schemars08", feature = "std"), derive(JsonSchema))]
                 pub enum [< $kind Kind>] {}
 
                 impl TypedUuidKind for [< $kind Kind >] {

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`	`# CI scripts:`
`9`	`9`	`# - .github/buildomat/build-and-test.sh`
`10`	`10`	`# - .github/buildomat/jobs/clippy.sh`
	`11`	`+# - .github/buildomat/jobs/check-features.sh`
`11`	`12`	`# - .github/workflows/rust.yml`
`12`	`13`	`#`
`13`	`14`	`[build]`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`	`# (that we want to check) is conditionally-compiled on illumos only.`
`11`	`11`	`#`
`12`	`12`	# Note that `cargo clippy` includes `cargo check, so this ends up checking all
`13`		`-# of our code.`
	`13`	`+# of our (default) code.`
`14`	`14`
`15`	`15`	`set -o errexit`
`16`	`16`	`set -o pipefail`