Skip to content

Commit 0e9e790

Browse files
authored
Fixes usdt feature dependency resolution bug (#496)
- Adds the Dropshot `usdt-probes` feature to every crate depending on both Dropshot and `slog-dtrace`. Leaving this feature out causes build failures when compiling packages individually, as the selected `usdt` feature actually includes inline assembly, but Dropshot does not include the right feature-selection crate attribute. This works when building the whole workspace, since all features are unioned. - Adds a `check` subcommand to `omicron-package`, that runs `cargo check` instead of `cargo build`. This is also run during CI now, to catch regressions here, at least for the crates that we package via `omicron-package`. Removes the toolchain specifications from CI files, as they are duplicated in the rust-toolchain.toml file. update note about rust-toolchain duplication in CI files Update dropshot dependency to pull in fixes for feature-selection
1 parent 2ab51ca commit 0e9e790

File tree

9 files changed

+57
-21
lines changed

9 files changed

+57
-21
lines changed

.github/buildomat/jobs/build-and-test.sh

+6
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ export RUSTFLAGS="-D warnings"
3333
export RUSTDOCFLAGS="-D warnings"
3434
ptime -m cargo +'nightly-2021-11-24' build --locked --all-targets --verbose
3535

36+
#
37+
# Check that building individual packages as when deploying Omicron succeeds
38+
#
39+
banner deploy-check
40+
ptime -m cargo run --bin omicron-package -- check
41+
3642
banner clickhouse
3743
ptime -m ./tools/ci_download_clickhouse
3844

.github/workflows/rust.yml

+19-11
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,22 @@ jobs:
3232
- name: Check style
3333
run: cargo fmt -- --check
3434

35+
check-omicron-deployment:
36+
needs: skip_duplicate_jobs
37+
if: ${{ needs.skip_duplicate_jobs.outputs.should_skip != 'true' }}
38+
runs-on: ${{ matrix.os }}
39+
strategy:
40+
fail-fast: false
41+
matrix:
42+
os: [ ubuntu-18.04, macos-11 ]
43+
steps:
44+
# actions/checkout@v2
45+
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
46+
- name: Report cargo version
47+
run: cargo --version
48+
- name: Check build of deployed Omicron packages
49+
run: cargo run --bin omicron-package -- check
50+
3551
clippy-lint:
3652
needs: skip_duplicate_jobs
3753
if: ${{ needs.skip_duplicate_jobs.outputs.should_skip != 'true' }}
@@ -73,19 +89,11 @@ jobs:
7389
fail-fast: false
7490
matrix:
7591
os: [ ubuntu-18.04, macos-11 ]
76-
# See rust-toolchain for why we're using nightly here.
77-
toolchain: [ nightly-2021-11-24 ]
7892
steps:
7993
# actions/checkout@v2
8094
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
81-
- name: Install Toolchain
82-
# actions-rs/[email protected]
83-
uses: actions-rs/toolchain@b2417cde72dcf67f306c0ae8e0828a81bf0b189f
84-
with:
85-
toolchain: ${{ matrix.toolchain }}
86-
override: true
8795
- name: Report cargo version
88-
run: cargo +${{ matrix.toolchain }} --version
96+
run: cargo --version
8997
- name: Configure GitHub cache for CockroachDB binaries
9098
id: cache-cockroachdb
9199
@@ -111,7 +119,7 @@ jobs:
111119
# also gives us a record of which dependencies were used for each CI
112120
# run. Building with `--locked` ensures that the checked-in Cargo.lock
113121
# is up to date.
114-
run: RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo +${{ matrix.toolchain }} build --locked --all-targets --verbose
122+
run: RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose
115123
- name: Download ClickHouse
116124
if: steps.cache-clickhouse.outputs.cache-hit != 'true'
117125
run: ./tools/ci_download_clickhouse
@@ -123,4 +131,4 @@ jobs:
123131
# rebuild here.
124132
# Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test
125133
# suite.
126-
run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo +${{ matrix.toolchain }} test --workspace --locked --verbose
134+
run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo test --workspace --locked --verbose

Cargo.lock

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

common/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ anyhow = "1.0"
99
api_identity = { path = "../api_identity" }
1010
backoff = { version = "0.3.0", features = [ "tokio" ] }
1111
chrono = { version = "0.4", features = [ "serde" ] }
12-
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" }
12+
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
1313
futures = "0.3.18"
1414
http = "0.2.5"
1515
hyper = "0.14"

oximeter/collector/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ description = "The oximeter metric collection server"
66
license = "MPL-2.0"
77

88
[dependencies]
9-
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" }
9+
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
1010
nexus-client = { path = "../../nexus-client" }
1111
omicron-common = { path = "../../common" }
1212
oximeter = { path = "../oximeter" }

oximeter/instruments/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ license = "MPL-2.0"
66

77
[dependencies]
88
chrono = { version = "0.4", features = [ "serde" ] }
9-
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" }
9+
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
1010
futures = "0.3.18"
1111
oximeter = { path = "../oximeter" }
1212
http = { version = "0.2.5", optional = true }

package/src/bin/omicron-package.rs

+25-3
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ enum SubCommand {
5050
)]
5151
release: bool,
5252
},
53+
/// Checks the packages specified in a manifest, without building.
54+
Check,
5355
/// Installs the packages to a target machine.
5456
Install {
5557
/// The directory from which artifacts will be pulled.
@@ -98,11 +100,15 @@ struct Args {
98100
subcommand: SubCommand,
99101
}
100102

101-
fn build_rust_package(package: &str, release: bool) -> Result<()> {
103+
fn run_cargo_on_package(
104+
subcmd: &str,
105+
package: &str,
106+
release: bool,
107+
) -> Result<()> {
102108
let mut cmd = Command::new("cargo");
103109
// We rely on the rust-toolchain.toml file for toolchain information,
104110
// rather than specifying one within the packaging tool.
105-
cmd.arg("build").arg("-p").arg(package);
111+
cmd.arg(subcmd).arg("-p").arg(package);
106112
if release {
107113
cmd.arg("--release");
108114
}
@@ -173,7 +179,14 @@ impl PackageInfo {
173179
// Builds the requested package.
174180
fn build(&self, package_name: &str, release: bool) -> Result<()> {
175181
match &self.build {
176-
Build::Rust => build_rust_package(package_name, release),
182+
Build::Rust => run_cargo_on_package("build", package_name, release),
183+
}
184+
}
185+
186+
// Checks the requested package.
187+
fn check(&self, package_name: &str) -> Result<()> {
188+
match &self.build {
189+
Build::Rust => run_cargo_on_package("check", package_name, false),
177190
}
178191
}
179192
}
@@ -193,6 +206,14 @@ fn parse<P: AsRef<Path>>(path: P) -> Result<Config, ParseError> {
193206
Ok(cfg)
194207
}
195208

209+
async fn do_check(config: &Config) -> Result<()> {
210+
for (package_name, package) in &config.packages {
211+
println!("Checking {}", package_name);
212+
package.check(&package_name)?;
213+
}
214+
Ok(())
215+
}
216+
196217
async fn do_package(
197218
config: &Config,
198219
output_directory: &Path,
@@ -446,6 +467,7 @@ async fn main() -> Result<()> {
446467
SubCommand::Package { artifact_dir, release } => {
447468
do_package(&config, &artifact_dir, *release).await?;
448469
}
470+
SubCommand::Check => do_check(&config).await?,
449471
SubCommand::Install { artifact_dir, install_dir } => {
450472
do_install(&config, &artifact_dir, &install_dir)?;
451473
}

rust-toolchain.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#
99

1010
[toolchain]
11-
# NOTE: This toolchain is also specified within .github/workflows/rust.yml
11+
# NOTE: This toolchain is also specified within .github/buildomat/jobs/build-and-test.sh.
1212
# If you update it here, update that file too.
1313
channel = "nightly-2021-11-24"
1414
profile = "default"

sled-agent/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ bincode = "1.3.3"
1111
bytes = "1.1"
1212
cfg-if = "1.0"
1313
chrono = { version = "0.4", features = [ "serde" ] }
14-
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" }
14+
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
1515
futures = "0.3.18"
1616
ipnetwork = "0.18"
1717
nexus-client = { path = "../nexus-client" }

0 commit comments

Comments
 (0)