diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97e3b3edd..269674a4d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,7 @@ env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 RG_VERSION: 15.1.0 + JQ_VERSION: 1.8.1 jobs: lint: @@ -302,6 +303,12 @@ jobs: - name: Install pinned ripgrep for differential tests run: scripts/install-ripgrep-ci.sh "$RG_VERSION" + # Same reason for `jq`, and the runner image already proved it: an + # unpinned oracle made the jq parity suite a function of which + # `ubuntu-latest` image the job landed on. + - name: Install pinned jq for differential tests + run: scripts/install-jq-ci.sh "$JQ_VERSION" + # Examples have a dedicated job below; skipping them here avoids # duplicated example links that can exhaust runner disk on main. - name: Run tests diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index bc34ca831..c9ff66a5b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -20,6 +20,7 @@ env: CARGO_TERM_COLOR: always LLVM_COV_IGNORE_FILENAME_REGEX: "(tests/|test_cases/|/tests\\.rs$)" RG_VERSION: 15.1.0 + JQ_VERSION: 1.8.1 jobs: coverage: @@ -48,6 +49,9 @@ jobs: - name: Install pinned ripgrep for differential tests run: scripts/install-ripgrep-ci.sh "$RG_VERSION" + - name: Install pinned jq for differential tests + run: scripts/install-jq-ci.sh "$JQ_VERSION" + # bashkit-eval is excluded: it enables bashkit's cpython and sqlite # features, and under --workspace feature unification that would pull # the slow CPython/sqlite tests into the instrumented bashkit binaries diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 045f3a294..afef1f065 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -14,6 +14,7 @@ env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 RG_VERSION: 15.1.0 + JQ_VERSION: 1.8.1 jobs: miri: @@ -149,10 +150,14 @@ jobs: with: cache-on-failure: true - # ASAN runs the Rust test suite, including `rg` differential tests. + # ASAN runs the Rust test suite, including the `rg` and `jq` + # differential tests. - name: Install pinned ripgrep for differential tests run: scripts/install-ripgrep-ci.sh "$RG_VERSION" + - name: Install pinned jq for differential tests + run: scripts/install-jq-ci.sh "$JQ_VERSION" + - name: Run tests with AddressSanitizer env: RUSTFLAGS: "-Z sanitizer=address" diff --git a/crates/bashkit/docs/jq.md b/crates/bashkit/docs/jq.md index f4bafdf96..37fd893aa 100644 --- a/crates/bashkit/docs/jq.md +++ b/crates/bashkit/docs/jq.md @@ -27,6 +27,7 @@ Implemented: | `-e`, `--exit-status` | Set exit code based on the output | | `-j` | Like `-r` but suppresses trailing newlines | | `--tab` | Use tabs for indentation | +| `--indent N` | Indent with N spaces, 0..7 (default 2). `0` still pretty-prints — newlines, no leading spaces — so `-c` is the only way to get compact output | | `-a`, `--ascii-output` | Escape non-ASCII characters as `\uXXXX` (strings stay quoted even with `-r`) | | `--raw-output0` | Like `-r` with a NUL after each output; a string containing NUL is an error | | `--seq` | Write RS (0x1e) before each output; RS separates input values | @@ -38,6 +39,10 @@ Implemented: | `-h`, `--help` | Print help | | Combined flags like `-snr` | Treated as the union of the individual flags | +`-c`, `--tab` and `--indent N` all choose the same thing — the output shape — +so the last one on the command line wins. `jq --indent 4 -c` is compact and +`jq -c --indent 4` is indented four spaces. + ## Variables | Variable | Behaviour | diff --git a/crates/bashkit/src/builtins/jq/args.rs b/crates/bashkit/src/builtins/jq/args.rs index 5b0d7b93e..d59e39a51 100644 --- a/crates/bashkit/src/builtins/jq/args.rs +++ b/crates/bashkit/src/builtins/jq/args.rs @@ -41,7 +41,6 @@ pub(super) struct JqArgs<'a> { pub raw_input: bool, pub raw_output: bool, pub join_output: bool, - pub compact_output: bool, pub null_input: bool, pub slurp: bool, pub sort_keys: bool, @@ -55,6 +54,10 @@ pub(super) struct JqArgs<'a> { pub stream: bool, /// `-e` flag — set exit status from output. pub exit_status: bool, + /// Output shape, set by `-c`/`--compact-output`, `--tab` and + /// `--indent N`. One field rather than a compact flag beside an indent + /// width, because real jq lets the last of those three on the command + /// line win: `jq --indent 4 -c` is compact and `jq -c --indent 4` is not. pub indent: Indent, pub file_args: Vec<&'a str>, /// Named string/JSON variables: `(name_with_$, value)`. @@ -147,7 +150,6 @@ pub(super) fn parse<'a>(args: &'a [String]) -> ParseOutcome<'a> { raw_input: false, raw_output: false, join_output: false, - compact_output: false, null_input: false, slurp: false, ascii_output: false, @@ -212,7 +214,7 @@ pub(super) fn parse<'a>(args: &'a [String]) -> ParseOutcome<'a> { match arg.as_str() { "--raw-output" => out.raw_output = true, "--raw-input" => out.raw_input = true, - "--compact-output" => out.compact_output = true, + "--compact-output" => out.indent = Indent::Compact, "--null-input" => out.null_input = true, "--sort-keys" => out.sort_keys = true, "--slurp" => out.slurp = true, @@ -230,9 +232,7 @@ pub(super) fn parse<'a>(args: &'a [String]) -> ParseOutcome<'a> { "--indent" => match args.get(i + 1) { Some(n) => match n.parse::() { Ok(n) if n <= MAX_JQ_INDENT => { - if !matches!(out.indent, Indent::Tab) { - out.indent = Indent::Spaces(n); - } + out.indent = Indent::Spaces(n); i += 2; continue; } @@ -338,7 +338,7 @@ pub(super) fn parse<'a>(args: &'a [String]) -> ParseOutcome<'a> { 'f' => takes_file = true, 'r' => out.raw_output = true, 'R' => out.raw_input = true, - 'c' => out.compact_output = true, + 'c' => out.indent = Indent::Compact, 'n' => out.null_input = true, 'S' => out.sort_keys = true, 's' => out.slurp = true, diff --git a/crates/bashkit/src/builtins/jq/format.rs b/crates/bashkit/src/builtins/jq/format.rs index 7b9e2c7b6..a817402bc 100644 --- a/crates/bashkit/src/builtins/jq/format.rs +++ b/crates/bashkit/src/builtins/jq/format.rs @@ -7,8 +7,8 @@ use super::convert::JqJson; -/// Output indentation choice. `Spaces(0)` matches `-c` semantics in tools -/// that bypass the compact path. +/// Output indentation choice. `-c` is the only way to get compact output; +/// `Spaces(0)` is still pretty-printed, just with zero-width indentation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum Indent { /// Compact output, no whitespace between tokens (matches `-c`). @@ -36,13 +36,14 @@ pub(super) fn sort_keys(v: JqJson) -> JqJson { /// Render a `JqJson` value using the chosen indent style. /// -/// Note: real jq treats `--indent 0` as equivalent to `-c` (compact, no -/// newlines), not "newlines without indentation". We match that. +/// Note: `--indent 0` is pretty output with zero-width indentation — newlines +/// between tokens, no leading spaces — not compact output. jq 1.7 and earlier +/// let `--indent 0` imply `-c`; jq 1.8 removed that implication +/// (jqlang/jq#3232) and we match current jq. `-c` is the only compact switch. pub(super) fn render(v: &JqJson, indent: Indent) -> String { let mut out = String::new(); match indent { Indent::Compact => render_compact(v, &mut out), - Indent::Spaces(0) => render_compact(v, &mut out), Indent::Spaces(n) => render_pretty(v, &mut out, &" ".repeat(n as usize), 0), Indent::Tab => render_pretty(v, &mut out, "\t", 0), } @@ -194,11 +195,25 @@ mod tests { } #[test] - fn indent_zero_renders_compact() { - // Real jq: --indent 0 is equivalent to -c (no newlines at all). + fn indent_zero_renders_newlines_without_indent() { + // jq >= 1.8: --indent 0 keeps the newlines and drops the spaces. let v = JqJson::Object(vec![("a".into(), n("1"))]); let out = render(&v, Indent::Spaces(0)); - assert_eq!(out, "{\"a\":1}"); + assert_eq!(out, "{\n\"a\": 1\n}"); + } + + #[test] + fn indent_zero_nests_without_indent() { + let v = JqJson::Object(vec![("a".into(), JqJson::Array(vec![n("1"), n("2")]))]); + let out = render(&v, Indent::Spaces(0)); + assert_eq!(out, "{\n\"a\": [\n1,\n2\n]\n}"); + } + + #[test] + fn compact_is_the_only_compact_switch() { + // -c stays byte-for-byte compact even though --indent 0 no longer is. + let v = JqJson::Object(vec![("a".into(), JqJson::Array(vec![n("1"), n("2")]))]); + assert_eq!(render(&v, Indent::Compact), "{\"a\":[1,2]}"); } #[test] diff --git a/crates/bashkit/src/builtins/jq/mod.rs b/crates/bashkit/src/builtins/jq/mod.rs index 501cabf10..517feef81 100644 --- a/crates/bashkit/src/builtins/jq/mod.rs +++ b/crates/bashkit/src/builtins/jq/mod.rs @@ -71,7 +71,7 @@ use compat::{ }; use convert::{JqJson, jq_to_val, parse_json_stream, val_to_jq_capped}; use errors::{format_compile_errors, format_load_errors, format_runtime_error_at}; -use format::{Indent, render, sort_keys as sort_jq_keys}; +use format::{render, sort_keys as sort_jq_keys}; /// Custom DataT that holds both the LUT and a shared input iterator. /// Required by jaq 3.0 for `input`/`inputs` filter support. @@ -455,11 +455,7 @@ async fn run_jq(ctx: Context<'_>, parsed: JqArgs<'_>) -> Result { } } - let indent = if parsed.compact_output { - Indent::Compact - } else { - parsed.indent - }; + let indent = parsed.indent; let mut output = String::new(); diff --git a/crates/bashkit/src/builtins/jq/tests.rs b/crates/bashkit/src/builtins/jq/tests.rs index 1330f93f6..11f7af3e4 100644 --- a/crates/bashkit/src/builtins/jq/tests.rs +++ b/crates/bashkit/src/builtins/jq/tests.rs @@ -270,12 +270,45 @@ async fn indent_4_uses_4_spaces() { } #[tokio::test] -async fn indent_0_renders_compact() { - // Real jq: --indent 0 is equivalent to -c. +async fn indent_0_renders_newlines_without_indent() { + // jq >= 1.8: --indent 0 keeps the newlines and drops the spaces. Only -c + // is compact. let result = run_jq_with_args(&["--indent", "0", "."], r#"{"a":1}"#) .await .unwrap(); - assert_eq!(result.trim(), r#"{"a":1}"#); + assert_eq!(result, "{\n\"a\": 1\n}\n"); +} + +#[tokio::test] +async fn output_shape_flags_are_last_wins() { + // -c, --tab and --indent N set the same thing, so the last one wins. + let cases: [(&[&str], &str); 6] = [ + (&["-c", "--indent", "0", "."], "{\n\"a\": [\n1,\n2\n]\n}\n"), + (&["--indent", "0", "-c", "."], "{\"a\":[1,2]}\n"), + ( + &["-c", "--indent", "4", "."], + "{\n \"a\": [\n 1,\n 2\n ]\n}\n", + ), + (&["--indent", "4", "-c", "."], "{\"a\":[1,2]}\n"), + (&["--tab", "-c", "."], "{\"a\":[1,2]}\n"), + ( + &["-c", "--tab", "."], + "{\n\t\"a\": [\n\t\t1,\n\t\t2\n\t]\n}\n", + ), + ]; + for (args, expected) in cases { + let result = run_jq_with_args(args, r#"{"a":[1,2]}"#).await.unwrap(); + assert_eq!(result, expected, "args: {args:?}"); + } +} + +#[tokio::test] +async fn later_indent_overrides_tab() { + // --tab used to stick regardless of a later --indent N. + let result = run_jq_with_args(&["--tab", "--indent", "4", "."], r#"{"a":1}"#) + .await + .unwrap(); + assert_eq!(result, "{\n \"a\": 1\n}\n"); } #[tokio::test] @@ -1512,11 +1545,48 @@ mod differential { use super::*; fn real_jq_available() -> bool { - std::process::Command::new("jq") + real_jq_version().is_some() + } + + /// `(major, minor)` of the host `jq`, or `None` when there is no usable + /// one. `jq --version` prints `jq-1.8.1`, older builds `jq-1.7`. + fn real_jq_version() -> Option<(u32, u32)> { + let out = std::process::Command::new("jq") .arg("--version") .output() - .map(|o| o.status.success()) - .unwrap_or(false) + .ok()?; + if !out.status.success() { + return None; + } + let text = String::from_utf8_lossy(&out.stdout); + let version = text.trim().strip_prefix("jq-")?; + let mut parts = version.split('.'); + let major = parts.next()?.parse().ok()?; + // `jq-1.7` has no minor component; treat the missing one as 0. + let minor = parts.next().unwrap_or("0").parse().unwrap_or(0); + Some((major, minor)) + } + + /// Like [`assert_matches`], for a case whose expected output only holds + /// from a given jq upwards. An older host oracle skips just that case + /// rather than reddening the suite, the same bargain + /// `minimum_bash_major` strikes for the competitor corpus. + /// + /// The one case that needs it today is `--indent 0`: jq 1.7 and earlier + /// let it imply `-c`, jq 1.8 emits newlines without indentation + /// (jqlang/jq#3232). CI pins the oracle (`scripts/install-jq-ci.sh`), so + /// this only ever skips on a developer machine with an older jq. + async fn assert_matches_from_jq(min: (u32, u32), args: &[&str], input: &str) { + match real_jq_version() { + Some(found) if found >= min => assert_matches(args, input).await, + Some((major, minor)) => eprintln!( + "host jq {major}.{minor} predates {}.{}; skipping `jq {}`", + min.0, + min.1, + args.join(" ") + ), + None => eprintln!("real jq not present; skipping differential test"), + } } fn run_real_jq(args: &[&str], input: &str) -> Option<(String, i32)> { @@ -1640,7 +1710,34 @@ mod differential { #[tokio::test] async fn diff_indent_0() { - assert_matches(&["--indent", "0", "."], r#"{"a":1}"#).await; + assert_matches_from_jq((1, 8), &["--indent", "0", "."], r#"{"a":1}"#).await; + } + + #[tokio::test] + async fn diff_indent_0_nested() { + assert_matches_from_jq((1, 8), &["--indent", "0", "."], r#"{"a":[1,2],"b":{"c":3}}"#).await; + } + + /// `-c`, `--tab` and `--indent N` all set the same thing, so the last one + /// on the command line wins. Each pair below disagrees with its reverse. + /// jq has behaved this way since at least 1.7; only what `--indent 0` + /// itself means changed in 1.8, which is why one case is gated. + #[tokio::test] + async fn diff_output_shape_flags_are_last_wins() { + let input = r#"{"a":[1,2]}"#; + for args in [ + ["-c", "--indent", "4", "."].as_slice(), + ["--indent", "4", "-c", "."].as_slice(), + ["--indent", "0", "-c", "."].as_slice(), + ["-c", "--tab", "."].as_slice(), + ["--tab", "-c", "."].as_slice(), + ["--tab", "--indent", "4", "."].as_slice(), + ["--indent", "4", "--tab", "."].as_slice(), + ["--indent", "0", "--indent", "4", "."].as_slice(), + ] { + assert_matches(args, input).await; + } + assert_matches_from_jq((1, 8), &["-c", "--indent", "0", "."], input).await; } #[tokio::test] diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index f48f285d0..9872e8263 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -576,6 +576,35 @@ publishes one headline number, bashkit passes over real-bash passes. It is a measurement, not a gate: no floor, no CI job. See [Oils Spec Pass Rate](oils-spec.md). +## Pinned External Oracles + +A differential test that asserts byte-for-byte parity against a tool on +`PATH` is only as reproducible as that tool's version. Leave it unpinned and +the suite becomes a function of the runner image, which changes on GitHub's +schedule rather than ours. `ripgrep` (`scripts/install-ripgrep-ci.sh`, +`RG_VERSION`) and `jq` (`scripts/install-jq-ci.sh`, `JQ_VERSION`) are both +installed from a digest-pinned release in every job that runs their +differential tests — today `ci.yml`'s `test` job, `coverage.yml`, and +nightly's `asan` job. Both scripts verify a SHA-256 before the binary reaches +`PATH`, so a compromised release asset cannot become CI code execution, and +both prepend their install directory via `GITHUB_PATH` so the pin wins over +an image-preinstalled copy. + +The jq pin was added after the cost was paid: when `ubuntu-latest` began +rolling from 24.04 to 26.04 the two images shipped jq 1.7 and jq 1.8, which +disagree on `--indent 0` (1.7 implied `--compact-output`, 1.8 emits newlines +without indentation, jqlang/jq#3232). One parity case went red on whichever +half of the fleet disagreed with the implementation, with no change on our +side, and the two halves could not both be satisfied. + +Pinning the oracle in CI does not help a developer whose distro ships an +older build, so a case whose expectation only holds from some version upwards +is gated rather than asserted unconditionally: +`assert_matches_from_jq((1, 8), …)` skips just that case on an older host +oracle and prints why. This is the same bargain `minimum_bash_major` strikes +for the competitor corpus. Gate the individual case, never the whole suite — +a skipped suite is indistinguishable from a passing one. + ## SQLite CLI Differential Tests CSV comparisons explicitly set the host CLI row separator to LF. SQLite diff --git a/scripts/install-jq-ci.sh b/scripts/install-jq-ci.sh new file mode 100755 index 000000000..982310f8b --- /dev/null +++ b/scripts/install-jq-ci.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +set -euo pipefail + +# CI executes the installed `jq` as the oracle for the jq differential tests; +# pin the release binary's digest before it lands on PATH so a compromised +# release asset or redirect cannot become CI code execution. +# +# Pinning the version matters on its own here, not just the digest. The jq +# differential tests assert byte-for-byte parity against whatever `jq` is on +# PATH, so an unpinned oracle makes the suite a function of the runner image: +# jq 1.7 and 1.8 disagree on `--indent 0` (1.7 implied compact output, 1.8 +# emits newlines without indentation, jqlang/jq#3232), and when `ubuntu-latest` +# began rolling from 24.04 to 26.04 the two images shipped the two versions. +# That turned one parity case red on whichever half of the fleet disagreed with +# the implementation, with no change on our side. +version="${1:-1.8.1}" +os="$(uname -s)" +arch="$(uname -m)" + +case "${os}:${arch}" in + Linux:x86_64) + target="linux-amd64" + ;; + Linux:aarch64 | Linux:arm64) + target="linux-arm64" + ;; + Darwin:x86_64) + target="macos-amd64" + ;; + Darwin:aarch64 | Darwin:arm64) + target="macos-arm64" + ;; + *) + echo "unsupported platform for pinned jq install: ${os} ${arch}" >&2 + exit 1 + ;; +esac + +case "${version}:${target}" in + 1.8.1:linux-amd64) + expected_sha256="020468de7539ce70ef1bceaf7cde2e8c4f2ca6c3afb84642aabc5c97d9fc2a0d" + ;; + 1.8.1:linux-arm64) + expected_sha256="6bc62f25981328edd3cfcfe6fe51b073f2d7e7710d7ef7fcdac28d4e384fc3d4" + ;; + 1.8.1:macos-amd64) + expected_sha256="e80dbe0d2a2597e3c11c404f03337b981d74b4a8504b70586c354b7697a7c27f" + ;; + 1.8.1:macos-arm64) + expected_sha256="a9fe3ea2f86dfc72f6728417521ec9067b343277152b114f4e98d8cb0e263603" + ;; + *) + echo "missing pinned SHA-256 for jq ${version} ${target}" >&2 + exit 1 + ;; +esac + +sha256_file() { + local file="$1" + + if command -v sha256sum > /dev/null 2>&1; then + sha256sum "${file}" | awk '{print $1}' + elif command -v shasum > /dev/null 2>&1; then + shasum -a 256 "${file}" | awk '{print $1}' + else + echo "No SHA-256 tool found (need sha256sum or shasum)" >&2 + exit 1 + fi +} + +verify_sha256() { + local file="$1" + local expected="$2" + local actual + + actual="$(sha256_file "${file}")" + if [[ "${actual}" != "${expected}" ]]; then + echo "SHA-256 mismatch for ${file}" >&2 + echo "Expected: ${expected}" >&2 + echo "Actual: ${actual}" >&2 + exit 1 + fi +} + +binary="jq-${target}" +url="https://github.com/jqlang/jq/releases/download/jq-${version}/${binary}" +tmp="$(mktemp -d)" +trap 'rm -rf "${tmp}"' EXIT + +curl -fsSL "${url}" -o "${tmp}/jq" +verify_sha256 "${tmp}/jq" "${expected_sha256}" + +# Ahead of the image's own /usr/bin/jq on PATH: GitHub prepends each +# GITHUB_PATH entry, so the pinned build wins over a preinstalled one. +install_dir="${RUNNER_TOOL_CACHE:-${HOME}/.cache}/jq-${version}-${target}/bin" +mkdir -p "${install_dir}" +cp "${tmp}/jq" "${install_dir}/jq" +chmod +x "${install_dir}/jq" + +if [[ -n "${GITHUB_PATH:-}" ]]; then + echo "${install_dir}" >> "${GITHUB_PATH}" +fi + +"${install_dir}/jq" --version