diff --git a/CHANGELOG.md b/CHANGELOG.md index b4945c9..3cd84c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,12 @@ Breaking. `packdb-query` is retired in favour of `packdb`, whose command library this repository now provides; its 0.2.x archives are unchanged. See `crates/packdb-cli/COMPATIBILITY.md` for the contract and the migration table. -- The package is `packdb-cli`, a library with no executable. `packdb query sql` - replaces `packdb-query`; the `packdb` binary is assembled with the server. +- The package is `packdb-cli`. It builds the `packdb` client binary, and its + library (`Command`, `run`, `Io`) stays the shared API. `packdb query sql` + replaces `packdb-query`. `packdb` never starts a server and depends on no + server package; the single-node server is `packdb-server`, built from the + server repository. The test-only `packdb-cli-test-harness` is removed: its + process tests now run the real binary. - Credentials never come from argument values: `--api-key-file`, `--token-file` and `--basic-auth-file`, or `PACKDB_API_KEY`, `PACKDB_TOKEN`, `PACKDB_BASIC_AUTH` and their `_FILE` forms. Two credentials are refused instead of the first diff --git a/Cargo.lock b/Cargo.lock index 3ed68b2..36fc3cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1654,17 +1654,6 @@ dependencies = [ "zstd", ] -[[package]] -name = "packdb-cli-test-harness" -version = "0.2.4" -dependencies = [ - "clap", - "packdb-cli", - "serde_json", - "tokio", - "wiremock", -] - [[package]] name = "packdb-client" version = "0.2.4" diff --git a/Cargo.toml b/Cargo.toml index 9b2421d..c4dd70f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ [workspace] resolver = "3" -members = ["crates/packdb-cli", "crates/packdb-cli-test-harness"] +members = ["crates/packdb-cli"] [workspace.package] edition = "2024" diff --git a/README.md b/README.md index 949b92c..293d7c5 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,18 @@ # PackDB command library -`packdb-cli` is the command library behind `packdb`, PackDB's command-line -client. It implements `packdb query sql|promql|logql|traceql`, `packdb trace`, +`packdb-cli` builds `packdb`, PackDB's command-line client, and the command +library behind it. It implements `packdb query sql|promql|logql|traceql`, `packdb trace`, `packdb doctor` (limited diagnostics), `packdb smoke-test`, `packdb support-bundle` and `packdb usage-report`. | Package | Scope | | --- | --- | -| `packdb-cli` | The command library: arguments, credentials, output, completeness and exit codes | -| `packdb-cli-test-harness` | Test-only executable that runs the library as a process; never released | +| `packdb-cli` | The `packdb` binary and its command library: arguments, credentials, output, completeness and exit codes | -This repository ships no executable. The `packdb` binary that embeds the library -is assembled with the server. `packdb-query`, the binary of releases 0.2.0 to -0.2.4, is retired; its archives remain on +`packdb` is a client only. It never starts a server and depends on no server +package; the single-node server is `packdb-server`, built from the server +repository. `packdb-query`, the binary of releases 0.2.0 to 0.2.4, is retired; +its archives remain on [GitHub Releases](https://github.com/packdb-oss/packdb-cli/releases) unchanged. This workspace provides client-side commands. It does not implement the server diff --git a/RELEASING.md b/RELEASING.md index 0664e13..38b3ba3 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -10,9 +10,10 @@ GitHub releases and crates.io publication are separate decisions; crates.io is p cannot resolve from the registry, until `packdb-otel` and then `packdb-client` are on crates.io. Do not attempt them in the other order. -This repository releases source only. The `packdb` binary that embeds the library -is built and released with the server; `packdb-query` binary archives of 0.2.0 to -0.2.4 stay on their GitHub Releases unchanged. +This repository releases source. The `packdb` client binary builds from it; how +`packdb` and `packdb-server` binaries are distributed is decided separately. +`packdb-query` binary archives of 0.2.0 to 0.2.4 stay on their GitHub Releases +unchanged. ## Private candidates @@ -21,7 +22,7 @@ is built and released with the server; `packdb-query` binary archives of 0.2.0 t 3. Inspect `cargo package --list`. The package must include its README, LICENSE and COMPATIBILITY.md, and exclude private data and internal-only source. 4. Dispatch `oss-candidate`. It reruns every gate on the exact commit, including - the test harness's process-level contract tests, and records the source archive, + the `packdb` binary's process-level contract tests, and records the source archive, package and source inventories, dependency inventory, checksums and the SDK revision it was built against. 5. Review the source archive, dependency inventory, checksums and release record. A diff --git a/crates/packdb-cli-test-harness/Cargo.toml b/crates/packdb-cli-test-harness/Cargo.toml deleted file mode 100644 index 0d1bf4f..0000000 --- a/crates/packdb-cli-test-harness/Cargo.toml +++ /dev/null @@ -1,30 +0,0 @@ -[package] -name = "packdb-cli-test-harness" -version = "0.2.4" -edition = "2024" -rust-version = "1.94.1" -description = "Test-only executable that runs the packdb-cli command library as a process. Never released." -license = "Apache-2.0" -repository = "https://github.com/packdb-oss/packdb-cli" -publish = false - -[[bin]] -name = "packdb-cli-test-harness" -path = "src/main.rs" - -[dependencies] -packdb-cli = { version = "0.2.4", path = "../packdb-cli" } -clap = { version = "4.5", features = ["derive"] } -tokio = { version = "1.49", features = ["rt"] } - -[dev-dependencies] -serde_json = "1" -tokio = { version = "1.49", features = ["macros", "rt", "rt-multi-thread"] } -wiremock = "0.6" - -[features] -default = [] -flight-sql = ["packdb-cli/flight-sql"] - -[lints.rust] -missing_docs = "deny" diff --git a/crates/packdb-cli-test-harness/src/main.rs b/crates/packdb-cli-test-harness/src/main.rs deleted file mode 100644 index cfd2bdf..0000000 --- a/crates/packdb-cli-test-harness/src/main.rs +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2025 PackDB Contributors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Runs the `packdb-cli` command library as a process, so tests can assert -//! real stdout, stderr and exit codes. Test-only: it is never released, and -//! it is not the `packdb` binary, which is assembled with the server. - -use std::process::ExitCode; - -use clap::Parser; - -#[derive(Parser)] -#[command( - name = "packdb", - version, - about = "PackDB client commands (test harness)", - arg_required_else_help = true, - subcommand_required = true -)] -struct Cli { - #[command(subcommand)] - command: packdb_cli::Command, -} - -fn main() -> ExitCode { - // clap prints help and exits 2 when no subcommand is given, exits 2 on any - // other usage error, and exits 0 for --help and --version. - let cli = Cli::parse(); - let runtime = match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - { - Ok(runtime) => runtime, - Err(error) => { - eprintln!("error: cannot start the runtime: {error}"); - return ExitCode::from(packdb_cli::Exit::Usage.code()); - } - }; - let mut stdout = std::io::stdout().lock(); - let mut stderr = std::io::stderr().lock(); - let exit = runtime.block_on(packdb_cli::run( - cli.command, - packdb_cli::Io { - stdout: &mut stdout, - stderr: &mut stderr, - env: &packdb_cli::ProcessEnvironment, - }, - )); - ExitCode::from(exit.code()) -} diff --git a/crates/packdb-cli/COMPATIBILITY.md b/crates/packdb-cli/COMPATIBILITY.md index f26f82f..36bc8d9 100644 --- a/crates/packdb-cli/COMPATIBILITY.md +++ b/crates/packdb-cli/COMPATIBILITY.md @@ -1,9 +1,9 @@ # `packdb` command contract What the `packdb` client commands promise their callers. This library implements -them; the `packdb` binary that embeds it is assembled with the server. Until that -binary exists, the contract is tested through `packdb-cli-test-harness`, a -test-only executable that is never released. +them, and this crate's `packdb` binary runs them. `packdb` is a client only: the +single-node server is `packdb-server`, built from the server repository, and each +reports its own version. Read as: anything in **The contract** changes only with a deliberate, documented decision. Anything under **Not promised** may change without one. @@ -463,10 +463,12 @@ refuses an answer whose declarations differ from these: unavailable value reported as missing evidence and never as zero, a server without the route, answers with a wrong schema, scope, period, omitted or negative value, `401`, `403` and an unreachable server. -- `crates/packdb-cli-test-harness/tests/process.rs`: the same contract as a process - sees it, covering help, the refused secret argument, stdout/stderr separation, - exit codes, credential files, conflicts, a closed stdout (exit `8`), the PromQL, - trace and time-argument cases, `doctor` and `usage-report`. +- `crates/packdb-cli/tests/process.rs`: the same contract as the `packdb` binary + shows it, covering help for every command, `--version`, bare `packdb` exiting `2` + promptly without creating files, the refused secret argument, stdout/stderr + separation, exit codes, credential files, conflicts, a closed stdout (exit `8`), + help, `--version`, usage errors and bare `packdb` on an unwritable stream (exit `8`), + the PromQL, trace and time-argument cases, `doctor` and `usage-report`. - `packdb-client`'s own tests cover the transport fallback rules, the query API requests and error classification. diff --git a/crates/packdb-cli/Cargo.toml b/crates/packdb-cli/Cargo.toml index 7094894..78cdeca 100644 --- a/crates/packdb-cli/Cargo.toml +++ b/crates/packdb-cli/Cargo.toml @@ -3,7 +3,7 @@ name = "packdb-cli" version = "0.2.4" edition = "2024" rust-version = "1.94.1" -description = "The command library behind the PackDB `packdb` command: queries and operator commands against a running server." +description = "PackDB's `packdb` command-line client and the command library behind it: queries and operator commands against a running server." license = "Apache-2.0" readme = "README.md" keywords = ["packdb", "cli", "sql", "query"] @@ -12,6 +12,12 @@ repository = "https://github.com/packdb-oss/packdb-cli" # a separate reviewed allowlist change and release authorization. publish = false +# The `packdb` client. The library stays the shared API: `Command`, `run` and +# `Io` are what the binary, and any other front end, call. +[[bin]] +name = "packdb" +path = "src/main.rs" + [dependencies] # Enable the `sql` feature so the SQL client (including Arrow result # formatting) is compiled in. Flight SQL transport is opt-in via the @@ -33,14 +39,14 @@ serde_json = { version = "1", features = ["preserve_order"] } # The support bundle's archive container and its temporary file. tar = { version = "0.4", default-features = false } tempfile = "3" -tokio = { version = "1.49", features = ["macros", "time"] } +tokio = { version = "1.49", features = ["macros", "rt", "time"] } # The `smoke-test` run marker and trace and span identifiers. uuid = { version = "1", features = ["v4"] } # Compresses the support bundle. zstd = { version = "0.14", default-features = false } [dev-dependencies] -tokio = { version = "1.49", features = ["macros", "rt"] } +tokio = { version = "1.49", features = ["macros", "rt", "rt-multi-thread"] } wiremock = "0.6" [features] diff --git a/crates/packdb-cli/README.md b/crates/packdb-cli/README.md index ed21cf5..0bf0021 100644 --- a/crates/packdb-cli/README.md +++ b/crates/packdb-cli/README.md @@ -1,12 +1,13 @@ # `packdb-cli` -The command library behind `packdb`: queries, trace lookup, limited `doctor` -diagnostics, `smoke-test`, `support-bundle` and `usage-report` against a running -PackDB server. +`packdb`, PackDB's command-line client, and the command library behind it: +queries, trace lookup, limited `doctor` diagnostics, `smoke-test`, +`support-bundle` and `usage-report` against a running PackDB server. -This crate ships no executable. The `packdb` binary that embeds it is assembled -with the server. `packdb-query`, the binary of releases 0.2.0 to 0.2.4, is -retired; its released archives are unchanged. +The crate builds the `packdb` binary and exports the library it runs (`Command`, +`run`, `Io`). `packdb` never starts a server; the single-node server is +`packdb-server`, built from the server repository. `packdb-query`, the binary of +releases 0.2.0 to 0.2.4, is retired; its released archives are unchanged. The supported surface — arguments, environment variables, output, completeness and exit codes — is written down in [`COMPATIBILITY.md`](./COMPATIBILITY.md), diff --git a/crates/packdb-cli/src/lib.rs b/crates/packdb-cli/src/lib.rs index fdc5275..aa1747b 100644 --- a/crates/packdb-cli/src/lib.rs +++ b/crates/packdb-cli/src/lib.rs @@ -15,8 +15,8 @@ //! The command library behind `packdb`. //! //! [`Command`] is a clap subcommand set; [`run`] executes one against a running -//! PackDB server and returns its [`Exit`] status. The library ships no -//! executable: the `packdb` binary that embeds it is assembled with the server. +//! PackDB server and returns its [`Exit`] status. This crate's `packdb` binary +//! runs it; the library is the shared API for any other front end. //! `COMPATIBILITY.md` is the supported contract: arguments, environment, //! output and exit codes. diff --git a/crates/packdb-cli/src/main.rs b/crates/packdb-cli/src/main.rs new file mode 100644 index 0000000..34afb95 --- /dev/null +++ b/crates/packdb-cli/src/main.rs @@ -0,0 +1,126 @@ +// Copyright 2025 PackDB Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! `packdb`, PackDB's command-line client. It parses a [`packdb_cli::Command`] +//! and exits with the status [`packdb_cli::run`] returns; the commands +//! themselves live in the library. It never starts a server: the single-node +//! server is `packdb-server`, built from the server repository. + +#![deny(clippy::unwrap_used)] +#![deny(clippy::expect_used)] +#![deny(clippy::indexing_slicing)] +#![deny(clippy::panic)] +#![deny(clippy::unreachable)] + +use std::io::{IsTerminal, Write}; +use std::process::ExitCode; + +use clap::Parser; +use packdb_cli::Exit; + +#[derive(Parser)] +#[command( + name = "packdb", + version, + about = "PackDB command-line client: queries and operator commands against a running PackDB server", + arg_required_else_help = true, + subcommand_required = true +)] +struct Cli { + #[command(subcommand)] + command: packdb_cli::Command, +} + +fn main() -> ExitCode { + let mut stdout = std::io::stdout().lock(); + let mut stderr = std::io::stderr().lock(); + // Parsing comes first, so help, version and usage errors never build a + // runtime or open a connection. clap renders them; this process writes + // them, so a failed write is reported like any other output failure. + let exit = match Cli::try_parse() { + Ok(cli) => execute(cli.command, &mut stdout, &mut stderr), + Err(error) => deliver_parse_outcome(&error, &mut stdout, &mut stderr), + }; + ExitCode::from(exit.code()) +} + +fn execute(command: packdb_cli::Command, stdout: &mut dyn Write, stderr: &mut dyn Write) -> Exit { + let runtime = match tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + { + Ok(runtime) => runtime, + Err(error) => { + return match writeln!(stderr, "error: cannot start the runtime: {error}") + .and_then(|()| stderr.flush()) + { + Ok(()) => Exit::Usage, + Err(_) => Exit::OutputFailed, + }; + } + }; + runtime.block_on(packdb_cli::run( + command, + packdb_cli::Io { + stdout, + stderr, + env: &packdb_cli::ProcessEnvironment, + }, + )) +} + +/// Write what clap rendered: `--help` and `--version` on stdout with `0`; +/// bare `packdb`'s help and every usage error on stderr with `2`. Output that +/// cannot be written or flushed exits `8`, as the contract promises. +fn deliver_parse_outcome( + error: &clap::Error, + stdout: &mut dyn Write, + stderr: &mut dyn Write, +) -> Exit { + let exit = if error.exit_code() == 0 { + Exit::Success + } else { + Exit::Usage + }; + let on_stderr = error.use_stderr(); + let styled = if on_stderr { + std::io::stderr().is_terminal() + } else { + std::io::stdout().is_terminal() + }; + let rendered = error.render(); + let text = if styled { + rendered.ansi().to_string() + } else { + rendered.to_string() + }; + let stream: &mut dyn Write = if on_stderr { stderr } else { stdout }; + match stream + .write_all(text.as_bytes()) + .and_then(|()| stream.flush()) + { + Ok(()) => exit, + Err(failure) => { + if !on_stderr { + // Best effort: if this report cannot be written either, the + // exit code still says the output failed. + drop( + writeln!(stderr, "error: cannot write to stdout: {failure}") + .and_then(|()| stderr.flush()), + ); + } + Exit::OutputFailed + } + } +} diff --git a/crates/packdb-cli-test-harness/tests/process.rs b/crates/packdb-cli/tests/process.rs similarity index 62% rename from crates/packdb-cli-test-harness/tests/process.rs rename to crates/packdb-cli/tests/process.rs index 45e7b89..9446ab3 100644 --- a/crates/packdb-cli-test-harness/tests/process.rs +++ b/crates/packdb-cli/tests/process.rs @@ -24,7 +24,7 @@ use serde_json::{Value, json}; use wiremock::matchers::{header, method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; -const HARNESS: &str = env!("CARGO_BIN_EXE_packdb-cli-test-harness"); +const PACKDB: &str = env!("CARGO_BIN_EXE_packdb"); /// Every variable the commands read, removed so the runner's own environment /// cannot leak into a case. @@ -42,22 +42,49 @@ const VARIABLES: [&str; 11] = [ "PACKDB_URL", ]; -async fn harness(args: &[&str], env: &[(&str, &str)]) -> Output { +/// Held while a child is spawned and while a pipe's reading end is closed. +/// +/// A spawned child holds a copy of every descriptor this process has open +/// until it execs. A closed-pipe case is only deterministic if no such copy +/// of its reading end survives the close, so no spawn may be in flight then. +/// `spawn` returns after the child has exec'd, so holding this across each +/// spawn and each close is enough. +static SPAWN: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +fn spawn_lock() -> std::sync::MutexGuard<'static, ()> { + SPAWN + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// Spawn `command` with the lock held, then wait for its output without it. +fn output(command: &mut Command) -> Output { + let child = { + let _spawning = spawn_lock(); + command.spawn().expect("spawn packdb") + }; + child.wait_with_output().expect("wait for packdb") +} + +async fn packdb(args: &[&str], env: &[(&str, &str)]) -> Output { let args: Vec = args.iter().map(|arg| (*arg).to_string()).collect(); let env: Vec<(String, String)> = env .iter() .map(|(k, v)| ((*k).to_string(), (*v).to_string())) .collect(); tokio::task::spawn_blocking(move || { - let mut command = Command::new(HARNESS); + let mut command = Command::new(PACKDB); for variable in VARIABLES { command.env_remove(variable); } - command - .args(&args) - .envs(env) - .output() - .expect("run the harness") + output( + command + .args(&args) + .envs(env) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()), + ) }) .await .expect("join") @@ -102,7 +129,7 @@ async fn sql_server(response: ResponseTemplate) -> MockServer { #[tokio::test(flavor = "multi_thread")] async fn no_subcommand_prints_help_on_stderr_and_exits_2() { - let output = harness(&[], &[]).await; + let output = packdb(&[], &[]).await; assert_eq!(output.status.code(), Some(2)); assert!(output.stdout.is_empty()); assert!( @@ -114,14 +141,108 @@ async fn no_subcommand_prints_help_on_stderr_and_exits_2() { #[tokio::test(flavor = "multi_thread")] async fn help_exits_0() { - let output = harness(&["--help"], &[]).await; + let output = packdb(&["--help"], &[]).await; assert_eq!(output.status.code(), Some(0)); assert!(text(&output.stdout).contains("query")); } +#[tokio::test(flavor = "multi_thread")] +async fn version_prints_the_package_version_and_exits_0() { + let output = packdb(&["--version"], &[]).await; + assert_eq!(output.status.code(), Some(0)); + assert_eq!( + text(&output.stdout), + format!("packdb {}\n", env!("CARGO_PKG_VERSION")) + ); + assert!(output.stderr.is_empty(), "{}", text(&output.stderr)); +} + +#[tokio::test(flavor = "multi_thread")] +async fn every_command_has_help_that_exits_0() { + for command in [ + &["query", "sql"][..], + &["query", "promql"], + &["query", "logql"], + &["query", "traceql"], + &["trace"], + &["doctor"], + &["smoke-test"], + &["support-bundle"], + &["usage-report"], + ] { + let mut args = command.to_vec(); + args.push("--help"); + let output = packdb(&args, &[]).await; + let usage = format!("Usage: packdb {}", command.join(" ")); + assert_eq!(output.status.code(), Some(0), "{command:?}"); + assert!( + text(&output.stdout).contains(&usage), + "{command:?}: {}", + text(&output.stdout) + ); + } +} + +/// Bare `packdb` is a client asking for a command, never a server starting up: +/// it exits promptly with `2`, and leaves nothing behind in its working +/// directory or home. +#[tokio::test(flavor = "multi_thread")] +async fn bare_packdb_exits_2_promptly_and_creates_no_files() { + let cwd = tempfile::tempdir().expect("working directory"); + let home = tempfile::tempdir().expect("home"); + let (cwd_path, home_path) = (cwd.path().to_path_buf(), home.path().to_path_buf()); + let output = tokio::task::spawn_blocking(move || { + let mut command = Command::new(PACKDB); + for variable in VARIABLES { + command.env_remove(variable); + } + let spawning = spawn_lock(); + let mut child = command + .current_dir(&cwd_path) + .env("HOME", &home_path) + .env("XDG_CONFIG_HOME", home_path.join(".config")) + .env("XDG_DATA_HOME", home_path.join(".local/share")) + .env("XDG_STATE_HOME", home_path.join(".local/state")) + .env("XDG_CACHE_HOME", home_path.join(".cache")) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn packdb"); + drop(spawning); + // A process still running after this long is serving, not printing + // help. Stop it so the failure is reported rather than hanging the run. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while child.try_wait().expect("poll packdb").is_none() { + if std::time::Instant::now() >= deadline { + child.kill().expect("stop packdb"); + panic!("bare packdb was still running after 30 s"); + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + child.wait_with_output().expect("collect packdb's output") + }) + .await + .expect("join"); + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty(), "{}", text(&output.stdout)); + assert!( + text(&output.stderr).contains("Usage: packdb"), + "{}", + text(&output.stderr) + ); + for dir in [cwd.path(), home.path()] { + let entries: Vec<_> = std::fs::read_dir(dir) + .expect("list directory") + .map(|entry| entry.expect("entry").file_name()) + .collect(); + assert!(entries.is_empty(), "{}: {entries:?}", dir.display()); + } +} + #[tokio::test(flavor = "multi_thread")] async fn a_secret_cannot_be_passed_as_an_argument_value() { - let output = harness(&["query", "sql", "SELECT 1", "--api-key", "secret"], &[]).await; + let output = packdb(&["query", "sql", "SELECT 1", "--api-key", "secret"], &[]).await; assert_eq!(output.status.code(), Some(2)); assert!(!text(&output.stdout).contains("secret")); } @@ -134,7 +255,7 @@ async fn json_on_stdout_warnings_on_stderr_and_exit_3_for_a_partial_result() { .insert_header("x-packdb-partial-causes", "cold_tier_unavailable"), ) .await; - let output = harness( + let output = packdb( &[ "query", "sql", @@ -167,7 +288,7 @@ async fn each_failure_has_its_exit_code() { ]; for (status, body, code) in cases { let server = sql_server(ResponseTemplate::new(status).set_body_string(body)).await; - let output = harness( + let output = packdb( &["query", "sql", "SELECT 1", "--transport", "http"], &[("PACKDB_ENDPOINT", &server.uri())], ) @@ -182,7 +303,7 @@ async fn each_failure_has_its_exit_code() { async fn an_unreachable_server_exits_6() { let refusing = refusing_endpoint(); let endpoint = refusing.endpoint.clone(); - let output = harness( + let output = packdb( &[ "query", "sql", @@ -210,7 +331,7 @@ async fn a_credential_file_is_read_and_its_secret_is_never_printed() { .await; let file = std::env::temp_dir().join(format!("packdb-cli-token-{}", std::process::id())); std::fs::write(&file, "secret-token\n").expect("write token"); - let output = harness( + let output = packdb( &[ "query", "sql", @@ -239,7 +360,7 @@ async fn conflicting_credentials_exit_2_without_contacting_the_server() { .expect(0) .mount(&server) .await; - let output = harness( + let output = packdb( &["query", "sql", "SELECT 1"], &[ ("PACKDB_ENDPOINT", &server.uri()), @@ -254,11 +375,75 @@ async fn conflicting_credentials_exit_2_without_contacting_the_server() { assert!(!stderr.contains("secret-"), "{stderr}"); } +/// Which of `packdb`'s streams a case makes unwritable. +#[derive(Debug, Clone, Copy)] +enum Closed { + Stdout, + Stderr, +} + +/// Run `packdb` with one stream a pipe whose reading end is already closed, +/// so every write to it fails with a broken pipe. (A read-only file would not +/// do: the standard library treats `EBADF` on stdout as a closed stdout and +/// discards the write.) The other stream is captured. +async fn with_closed(args: &[&str], closed: Closed) -> Output { + let args: Vec = args.iter().map(|arg| (*arg).to_string()).collect(); + tokio::task::spawn_blocking(move || { + let writer = { + let _closing = spawn_lock(); + let (reader, writer) = std::io::pipe().expect("pipe"); + drop(reader); + writer + }; + let mut command = Command::new(PACKDB); + for variable in VARIABLES { + command.env_remove(variable); + } + command.args(&args).stdin(Stdio::null()); + match closed { + Closed::Stdout => command.stdout(writer).stderr(Stdio::piped()), + Closed::Stderr => command.stdout(Stdio::piped()).stderr(writer), + }; + output(&mut command) + }) + .await + .expect("join") +} + +/// What clap renders -- help, the version, a usage error, bare `packdb`'s +/// help -- is output like any other: when its stream cannot be written, +/// `packdb` exits 8 instead of 0 or 2. +#[tokio::test(flavor = "multi_thread")] +async fn help_version_and_usage_errors_on_a_closed_stream_exit_8() { + for (args, closed) in [ + (&["--help"][..], Closed::Stdout), + (&["query", "sql", "--help"], Closed::Stdout), + (&["--version"], Closed::Stdout), + (&["query", "sql"], Closed::Stderr), + (&["no-such-command"], Closed::Stderr), + (&[], Closed::Stderr), + ] { + let output = with_closed(args, closed).await; + assert_eq!( + output.status.code(), + Some(8), + "{args:?} with {closed:?} unwritable" + ); + if let Closed::Stdout = closed { + assert!( + text(&output.stderr).contains("cannot write to stdout"), + "{args:?}: {}", + text(&output.stderr) + ); + } + } +} + /// A reader that goes away before the result is written: the write fails with /// a broken pipe, and the command reports it as exit 8 instead of succeeding. /// -/// The server holds its reply until the test has closed the harness's stdout, -/// so the pipe is closed before the harness can write, whatever the scheduling. +/// The server holds its reply until the test has closed packdb's stdout, +/// so the pipe is closed before packdb can write, whatever the scheduling. #[tokio::test(flavor = "multi_thread")] async fn a_closed_stdout_exits_8() { use std::io::{Read, Write}; @@ -282,10 +467,11 @@ async fn a_closed_stdout_exits_8() { .expect("answer the query"); }); let status = tokio::task::spawn_blocking(move || { - let mut command = Command::new(HARNESS); + let mut command = Command::new(PACKDB); for variable in VARIABLES { command.env_remove(variable); } + let spawning = spawn_lock(); let mut child = command .args([ "query", @@ -299,10 +485,12 @@ async fn a_closed_stdout_exits_8() { .stdout(Stdio::piped()) .stderr(Stdio::null()) .spawn() - .expect("spawn the harness"); + .expect("spawn packdb"); + // Still holding the lock: no other spawn can copy the reading end. drop(child.stdout.take()); + drop(spawning); closed.send(()).expect("release the reply"); - child.wait().expect("wait for the harness") + child.wait().expect("wait for packdb") }) .await .expect("join"); @@ -312,9 +500,9 @@ async fn a_closed_stdout_exits_8() { #[tokio::test(flavor = "multi_thread")] async fn help_lists_every_query_language_and_trace() { - let top = harness(&["--help"], &[]).await; + let top = packdb(&["--help"], &[]).await; assert!(text(&top.stdout).contains("trace"), "{}", text(&top.stdout)); - let query = harness(&["query", "--help"], &[]).await; + let query = packdb(&["query", "--help"], &[]).await; assert_eq!(query.status.code(), Some(0)); let help = text(&query.stdout); for language in ["sql", "promql", "logql", "traceql"] { @@ -333,7 +521,7 @@ async fn a_promql_answer_without_a_carrier_exits_3_as_unknown() { )) .mount(&server) .await; - let output = harness( + let output = packdb( &["query", "promql", "1", "--format", "json"], &[ ("PACKDB_ENDPOINT", &server.uri()), @@ -368,13 +556,13 @@ async fn a_found_trace_exits_0_and_a_missing_one_exits_1() { ) .mount(&server) .await; - let found = harness(&["trace", "ab12"], &[("PACKDB_ENDPOINT", &server.uri())]).await; + let found = packdb(&["trace", "ab12"], &[("PACKDB_ENDPOINT", &server.uri())]).await; assert_eq!(found.status.code(), Some(0), "{}", text(&found.stderr)); let envelope: Value = serde_json::from_slice(&found.stdout).expect("one JSON object"); assert_eq!(envelope["completeness"], "complete"); assert!(found.stderr.is_empty(), "{}", text(&found.stderr)); - let missing = harness(&["trace", "ff"], &[("PACKDB_ENDPOINT", &server.uri())]).await; + let missing = packdb(&["trace", "ff"], &[("PACKDB_ENDPOINT", &server.uri())]).await; assert_eq!(missing.status.code(), Some(1), "{}", text(&missing.stderr)); assert!( text(&missing.stderr).contains("trace not found: ff"), @@ -403,7 +591,7 @@ async fn conflicting_or_incomplete_time_arguments_exit_2() { &["trace", "ab12", "--start", "now-1h"][..], &["query", "traceql", "{}", "--start", "tomorrow"][..], ] { - let output = harness(args, &[("PACKDB_ENDPOINT", &refusing.endpoint)]).await; + let output = packdb(args, &[("PACKDB_ENDPOINT", &refusing.endpoint)]).await; assert_eq!(output.status.code(), Some(2), "{args:?}"); assert!(output.stdout.is_empty(), "{args:?}"); } @@ -411,17 +599,17 @@ async fn conflicting_or_incomplete_time_arguments_exit_2() { #[tokio::test(flavor = "multi_thread")] async fn doctor_is_listed_refuses_csv_and_exits_6_when_unreachable() { - let help = harness(&["--help"], &[]).await; + let help = packdb(&["--help"], &[]).await; assert!( text(&help.stdout).contains("doctor"), "{}", text(&help.stdout) ); - let csv = harness(&["doctor", "--format", "csv"], &[]).await; + let csv = packdb(&["doctor", "--format", "csv"], &[]).await; assert_eq!(csv.status.code(), Some(2)); assert!(csv.stdout.is_empty()); let refusing = refusing_endpoint(); - let output = harness(&["doctor"], &[("PACKDB_ENDPOINT", &refusing.endpoint)]).await; + let output = packdb(&["doctor"], &[("PACKDB_ENDPOINT", &refusing.endpoint)]).await; assert_eq!(output.status.code(), Some(6), "{}", text(&output.stderr)); assert!( text(&output.stdout).starts_with("packdb doctor (limited diagnostics): "), @@ -437,17 +625,17 @@ async fn doctor_is_listed_refuses_csv_and_exits_6_when_unreachable() { #[tokio::test] async fn usage_report_is_listed_refuses_csv_and_exits_6_when_unreachable() { - let help = harness(&["--help"], &[]).await; + let help = packdb(&["--help"], &[]).await; assert!( text(&help.stdout).contains("usage-report"), "{}", text(&help.stdout) ); - let csv = harness(&["usage-report", "--format", "csv"], &[]).await; + let csv = packdb(&["usage-report", "--format", "csv"], &[]).await; assert_eq!(csv.status.code(), Some(2)); assert!(csv.stdout.is_empty()); let refusing = refusing_endpoint(); - let output = harness( + let output = packdb( &["usage-report"], &[("PACKDB_ENDPOINT", &refusing.endpoint)], ) diff --git a/scripts/oss-ci.sh b/scripts/oss-ci.sh index f5d2cd5..fa9cf36 100755 --- a/scripts/oss-ci.sh +++ b/scripts/oss-ci.sh @@ -106,6 +106,21 @@ EOF log "sibling SDK checkout: $(cd "$sdk" && git rev-parse HEAD 2>/dev/null || echo 'not a git checkout')" } +# `packdb` is a client: it must never be able to start a server, so it may not +# depend on one. Every PackDB package in its normal and build dependency closure, +# with every feature enabled, must be this package or the SDK it builds on. A +# server package -- `packdb` or any other `packdb-*` crate -- fails the gate. +readonly CLIENT_PACKDB_PACKAGES="packdb-cli packdb-client packdb-otel" +check_client_dependencies() { + log "client dependency closure" + local closure unexpected + closure="$(cargo tree --locked -p packdb-cli --all-features -e normal,build \ + --prefix none --format '{p}')" || die "cargo tree failed for packdb-cli" + unexpected="$(awk '{print $1}' <<<"$closure" | sort -u | + grep -E '^packdb($|-)' | grep -vxF -f <(tr ' ' '\n' <<<"$CLIENT_PACKDB_PACKAGES") || true)" + [[ -z "$unexpected" ]] || die "packdb-cli depends on a server package: $unexpected" +} + check_policy() { log "policy files and contacts" local f @@ -164,9 +179,9 @@ PY for package in "${PACKAGES[@]}"; do listed="$(cargo package --list --locked --allow-dirty -p "$package" 2>/dev/null)" || die "cargo package --list failed for $package" - # A library crate -- and the contract must travel inside the archive, + # The library and the binary -- and the contract must travel inside the archive, # not merely exist in the repo. - for required in Cargo.toml README.md LICENSE COMPATIBILITY.md src/lib.rs; do + for required in Cargo.toml README.md LICENSE COMPATIBILITY.md src/lib.rs src/main.rs; do grep -qx "$required" <<<"$listed" || die "$package package is missing $required" done if grep -E '(^|/)(target|dist)/|\.profraw$' <<<"$listed"; then @@ -175,10 +190,10 @@ PY done } -# The workspace has one library, `packdb-cli`, and one test-only executable, -# `packdb-cli-test-harness`, whose tests run the library as a process to check -# stdout, stderr and exit codes. `--all-targets` runs both; `--doc` covers the -# library's documentation examples. +# The workspace has one package, `packdb-cli`: the command library and the +# `packdb` binary, whose process tests check stdout, stderr and exit codes. +# `--all-targets` runs both; `--doc` covers the library's documentation +# examples. feature_matrix() { local toolchain="$1" cargo "+$toolchain" test --locked --workspace --all-targets @@ -197,6 +212,7 @@ cmd_checks() { python3 scripts/check_action_pins.py --self-test python3 scripts/check_source_headers.py require_sdk_sibling + check_client_dependencies python3 scripts/export_clean.py python3 scripts/coverage_gate.py --self-test cargo fmt --all -- --check @@ -229,10 +245,8 @@ cmd_compatibility() { check_gate_input compatibility scope >/dev/null require_sdk_sibling - # The command contract is checked through the test harness: its tests run - # the library as a process and assert stdout, stderr and exit codes. This - # repository ships no executable; the `packdb` binary is assembled with the - # server. + # The command contract is checked through the `packdb` binary: its process + # tests assert stdout, stderr and exit codes. feature_matrix "$MSRV" }