diff --git a/.cargo/config.toml b/.cargo/config.toml index 01ff41a155..02046e124c 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -12,8 +12,8 @@ CARGO_LLVM_COV_TARGET_DIR = { value = "target/llvm-cov/build", relative = true, CARGO_LLVM_COV_BUILD_DIR = { value = "target/llvm-cov/target", relative = true, force = false } [build] -# Register `emulated` so cfg_attr sites don't trip unexpected_cfgs natively. -rustflags = ["--cfg=tokio_unstable", "--check-cfg=cfg(emulated)"] +# Register `emulated` and `instrumented` so cfg sites do not trip unexpected_cfgs natively. +rustflags = ["--cfg=tokio_unstable", "--check-cfg=cfg(emulated)", "--check-cfg=cfg(instrumented)"] [target.wasm32-wasip1] # Trailing `--` separates wasmtime's CLI from the module + module args diff --git a/.github/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 2336aabfca..6dee11c64a 100644 --- a/.github/actions/nix-shell/action.yml +++ b/.github/actions/nix-shell/action.yml @@ -32,6 +32,12 @@ runs: signingKey: '${{ inputs.cachix_signing_key }}' # prettier-ignore authToken: '${{ inputs.cachix_auth_token }}' + # Exclude images and dockerTools assembly paths, whose customization + # layers can reintroduce the source closure. Match artifact shapes so new + # images are covered automatically; `check-push-filter` verifies them. + # Anchor `stream-` to a store-path name so cached crates such as + # tokio-stream are not swept up by it. + pushFilter: '(-dataplane-volatile-|-customisation-layer$|-(base|conf|layers)\.json$|-excludePaths$|/[0-9a-z]{32}-stream-|-env$)' - name: "use nix shell" uses: "rrbutani/use-nix-shell-action@59a52b2b9bbfe3cc0e7deb8f9059abe37a439edf" # v1.1.0 diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 9eab13c3c6..d28097881f 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -74,9 +74,11 @@ Production artifacts are produced via nix builds in a separate CI workflow. overrides it, because the merge queue has no labels to read and would run the upgrade legs anyway; a `merge-ready` run that skipped them would not be the preview it claims to be +- `ci:-vlab` - Skip VLAB and HLAB tests on this PR, even with `ci:+merge-ready` Labels are additive, and optional: a pull request needs none of them. -`ci:-upgrade` is the sole exception, subtracting a job that would otherwise run. +`ci:-vlab` and `ci:-upgrade` are the exceptions, subtracting jobs that would +otherwise run. Adding a label starts a **new** workflow run, and that run repeats the default jobs as well as the ones the label enabled. This applies to _every_ label, not diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 0afab3591d..5cb2a33d5c 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -67,6 +67,7 @@ jobs: runs-on: "ubuntu-latest" outputs: container_profiles: "${{ steps.container-profiles.outputs.value }}" + parallel: "${{ steps.parallel.outputs.value }}" profiles: "${{ steps.profiles.outputs.value }}" concurrency: "${{ steps.concurrency.outputs.value }}" cross: "${{ steps.cross.outputs.value }}" @@ -81,6 +82,19 @@ jobs: with: persist-credentials: "false" + # Let merge-gating runs finish quickly without allowing pull requests to + # crowd them out. `strategy` cannot read `env`, so expose this via `plan`. + - id: "parallel" + env: + EVENT: "${{ github.event_name }}" + run: | + set -euo pipefail + case "${EVENT}" in + merge_group | push) value="4" ;; + *) value="1" ;; + esac + printf 'value=%s\n' "${value}" >>"${GITHUB_OUTPUT}" + - id: "miri" uses: &gate "./.github/actions/ci-gate" with: @@ -183,10 +197,14 @@ jobs: JUST_VARS: "" strategy: fail-fast: false - # Keep one pull request from occupying the shared lab pool. - max-parallel: 1 + # Each entry gets its own runner; this limits shared-pool occupancy. + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" + exclude: + # Fuzz repeats the release compile here; coverage, sanitizers, and + # fuzzing jobs already exercise that profile. + - profile: "fuzz" steps: - *checkout @@ -219,6 +237,13 @@ jobs: recipe: "ci::check-doctest" recipe_args: "${{ matrix.profile }}" + # Build API docs so links and cfg declarations cannot rot unnoticed. + - name: "docs" + uses: *just + with: + recipe: "ci::check-docs" + recipe_args: "${{ matrix.profile }}" + - &verify-clean-tree name: "verify-clean-tree" run: | @@ -319,6 +344,35 @@ jobs: with: recipe: "markdownlint" + - name: "nixfmt" + id: "nixfmt" + continue-on-error: true + uses: *just + with: + recipe: "nixfmt" + + - name: "check-lint-wiring" + id: "check-lint-wiring" + continue-on-error: true + uses: *just + with: + recipe: "check-lint-wiring" + + - name: "check-push-filter" + id: "check-push-filter" + continue-on-error: true + uses: *just + with: + recipe: "check-push-filter" + + # Cache misses still pass, so guard dependency reuse explicitly. + - name: "check-deps-reuse" + id: "check-deps-reuse" + continue-on-error: true + uses: *just + with: + recipe: "check-deps-reuse" + - name: "license-headers" id: "license-headers" continue-on-error: true @@ -337,6 +391,10 @@ jobs: pinact=${{ steps.pinact.outcome }} actionlint=${{ steps.actionlint.outcome }} markdownlint=${{ steps.markdownlint.outcome }} + nixfmt=${{ steps.nixfmt.outcome }} + check-lint-wiring=${{ steps.check-lint-wiring.outcome }} + check-push-filter=${{ steps.check-push-filter.outcome }} + check-deps-reuse=${{ steps.check-deps-reuse.outcome }} license-headers=${{ steps.license-headers.outcome }} run: | set -euo pipefail @@ -365,7 +423,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: nix-target: - frr.dataplane @@ -399,7 +457,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: sanitizer: - thread @@ -444,7 +502,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} # Fuzz provides optimized coverage while retaining safety checks. matrix: profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" @@ -632,6 +690,7 @@ jobs: JUST_VARS: "" strategy: fail-fast: false + # Keep cross serial so one pull request cannot occupy the lab. max-parallel: 1 matrix: platform: @@ -699,6 +758,7 @@ jobs: with: # ci:+hlab is required to enable hybrid lab tests on PR # ci:+vlab is required to enable virtual lab tests on PR + # ci:-vlab opts back out, including for ci:+merge-ready # ci:-upgrade disables upgrade tests on PR # hlab is disabled for main and merge_queue till we have gateway tests for it # ci:+merge-ready mirrors the merge queue, which skips HLAB. @@ -706,7 +766,8 @@ jobs: ${{ github.event_name == 'pull_request' && ( - matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+hlab') + contains(github.event.pull_request.labels.*.name, 'ci:-vlab') + || matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+hlab') || !matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+vlab') && !contains(github.event.pull_request.labels.*.name, 'ci:+merge-ready') diff --git a/acl/src/dpdk/install.rs b/acl/src/dpdk/install.rs index 65aa107296..939859c91e 100644 --- a/acl/src/dpdk/install.rs +++ b/acl/src/dpdk/install.rs @@ -14,7 +14,7 @@ use crate::dpdk::rule::RuleSpec; /// /// The `rte_acl` field count is computed from `K`'s layout at runtime and /// dispatched to the const-`N` builder shared with the dynamic install path -/// (see [`dispatch_build_classifier`]). The resulting `DpdkAclLookup` +/// (see `dispatch_build_classifier`, crate-private). The resulting `DpdkAclLookup` /// carries no field-count or stride const generics, so one type covers every /// monomorphization of a generic key. pub fn install_table( diff --git a/acl/src/lib.rs b/acl/src/lib.rs index 789d2ae7e7..a32cc0a40f 100644 --- a/acl/src/lib.rs +++ b/acl/src/lib.rs @@ -18,8 +18,8 @@ //! - `dpdk` module (`dpdk` feature): production `rte_acl` backend -- //! layout planner, rule lowering, install, and the single-shot / //! batched classify path. -//! - [`reference`](mod@reference): linear-scan software classifier; -//! differential oracle for the `dpdk` backend. Always built. +//! - `reference` (behind the `reference` feature): linear-scan software +//! classifier; differential oracle for the `dpdk` backend. //! //! [`lookup::Lookup`]: lookup::Lookup //! [`match_action::MatchKey`]: match_action::MatchKey diff --git a/ci.just b/ci.just index 1bc2fc076a..f4c5406f8f 100644 --- a/ci.just +++ b/ci.just @@ -10,13 +10,25 @@ set script-interpreter := ["/usr/bin/env", "bash", "-euo", "pipefail"] debug_justfile := env("CI_DEBUG_JUSTFILE", "false") -# Nix build budget for a 10-core lab runner. +# The container cannot discover its ten-core cgroup limit. Keep `jobs * cores` +# within that budget and give single large derivations all of it. jobs := "1" -cores := "8" +cores := "10" + +# Small per-package derivations use the budget better when split two ways. +test_each_jobs := "2" + +test_each_cores := "5" + +[private] +_lab-common := " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile [private] -_lab := "jobs=" + jobs + " cores=" + cores + " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile +_lab := "jobs=" + jobs + " cores=" + cores + _lab-common + +[private] +_lab-test-each := "jobs=" + test_each_jobs + " cores=" + test_each_cores + _lab-common [default] [private] @@ -37,11 +49,14 @@ check-clippy profile: check-doctest profile: just {{ _lab }} profile={{ profile }} doctest +check-docs profile: + just {{ _lab }} profile={{ profile }} docs + sanitize san profile="fuzz": just {{ _lab }} profile={{ profile }} sanitize={{ san }} test test-each profile="debug": - just {{ _lab }} profile={{ profile }} test-each + just {{ _lab-test-each }} profile={{ profile }} test-each coverage profile="debug": just {{ _lab }} profile={{ profile }} instrument=coverage coverage-archive diff --git a/concurrency/src/slot.rs b/concurrency/src/slot.rs index 7d13c02eb2..797ecf87ea 100644 --- a/concurrency/src/slot.rs +++ b/concurrency/src/slot.rs @@ -26,7 +26,7 @@ //! the miri job (which runs against the real `ArcSwap` in //! permissive-provenance mode) is where it lives. //! -//! [`Subscriber::snapshot`]: crate::Subscriber::snapshot +//! [`Subscriber::snapshot`]: crate::quiescent::Subscriber::snapshot // Strict provenance checks fail with arc-swap since it uses hazard pointers and does not (yet) use the new // std features to expose provenance information in their mechanics. diff --git a/concurrency/src/stress.rs b/concurrency/src/stress.rs index 725608e7e5..965061a975 100644 --- a/concurrency/src/stress.rs +++ b/concurrency/src/stress.rs @@ -30,7 +30,7 @@ pub fn shuttle_config() -> shuttle::Config { /// /// * default backend -- one direct call, no scheduling exploration. /// * `loom` -- `loom::model`. -/// * `shuttle` -- the [`shuttle_config`]-configured `PortfolioRunner` +/// * `shuttle` -- the `shuttle_config`-configured `PortfolioRunner` /// (`RandomScheduler` + `PctScheduler`, plus `DfsScheduler` under /// `shuttle_dfs`). /// diff --git a/concurrency/src/thread/mod.rs b/concurrency/src/thread/mod.rs index 751c684a8e..8ef07c5fc8 100644 --- a/concurrency/src/thread/mod.rs +++ b/concurrency/src/thread/mod.rs @@ -7,7 +7,7 @@ //! //! `std::thread::scope` (stable since 1.63) and `shuttle::thread::scope` //! are re-exported directly. `loom` 0.7 does not provide `scope`, so we -//! ship a local shim in [`loom_scope`] that matches the std API on top +//! ship a local shim in `loom_scope` that matches the std API on top //! of loom's `spawn` + `park`/`unpark` + atomic primitives, with a //! narrow `unsafe` lifetime launder (same trick std uses internally). //! diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 342a06fc33..ffbd037bfa 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -50,7 +50,7 @@ pub struct AclPattern { /// Port ranges for match entries that specified neither `cidr` nor `vpcSubnet`, meaning "any /// address within the peering, restricted to these ports". These can't be resolved into /// concrete prefixes until the peering's manifests are known, so they're materialized into - /// `src`/`dst` during [`AclRule::validate_patterns_coverage`] rather than at conversion time. + /// `src`/`dst` during `AclRule::validate_patterns_coverage` rather than at conversion time. pub src_any_ports: Vec, pub dst_any_ports: Vec, pub proto: AclProtoMatch, diff --git a/default.nix b/default.nix index 19dffb5037..301dba5408 100644 --- a/default.nix +++ b/default.nix @@ -171,6 +171,7 @@ let llvmPackages'.clang # you need the host compiler in order to link proc macros llvmPackages'.llvm # needed for coverage markdownlint-cli2 + nixfmt npins opengrep openssl @@ -212,25 +213,48 @@ let PKG_CONFIG_ALLOW_CROSS = "1"; }; }; - justfileFilter = p: _type: builtins.match ".*\.justfile$" p != null; - markdownFilter = p: _type: builtins.match ".*\.md$" p != null; - jsonFilter = p: _type: builtins.match ".*\.json$" p != null; - cHeaderFilter = p: _type: builtins.match ".*\.h$" p != null; - outputsFilter = p: _type: (p != "target") && (p != "sysroot") && (p != "devroot") && (p != ".git"); + # Nix escaping made the old regexes match unrelated .sh and .patch files. + cHeaderFilter = p: _type: lib.hasSuffix ".h" p; + # Prose lives at the repository root, under `development/`, and under + # `.github/`; Markdown anywhere else is either compiled into a crate's docs by + # `include_str!` or sits beside code that is. Excluding by location rather + # than listing the `include_str!` targets keeps a new one from having to be + # registered here -- the worst case becomes an unnecessary rebuild instead of + # a build that cannot find its README. + markdownFilter = + rel: _type: + lib.hasSuffix ".md" rel + && lib.hasInfix "/" rel + && !(lib.hasPrefix "development/" rel) + && !(lib.hasPrefix ".github/" rel); + # `.cargo/config.toml` names this script, so include it deliberately. + shellFilter = p: _type: lib.hasSuffix ".sh" p; + # `cleanSource` does not read gitignore, so `results` needs excluding by hand + # or every developer who has built carries a private `src` hash. + outputsFilter = + p: _type: + (p != "target") && (p != "sysroot") && (p != "devroot") && (p != "results") && (p != ".git"); + # `builtins.path` and friends hand the filter an absolute path; the markdown + # allowlist is written relative to the repository, so strip the root off. + src-root = toString ./.; src = pkgs.lib.cleanSourceWith { filter = full-path: t: let p = baseNameOf full-path; + rel = lib.removePrefix (src-root + "/") (toString full-path); in - (justfileFilter p t) - || (markdownFilter p t) - || (jsonFilter p t) + (markdownFilter rel t) || (cHeaderFilter p t) + || (shellFilter p t) || ((outputsFilter p t) && (craneLib.filterCargoSources full-path t)); src = lib.cleanSource ./.; name = "source"; }; + # Keep dependency fingerprints independent of the source store path. + # Consumers resolve this relative prefix from the workspace root. + src-prefix = "."; + # Hash every git dependency so crane uses cacheable fixed-output derivations # instead of cloning whole repositories during evaluation. Keys must match # Cargo.lock sources after percent-decoding branch names: a wrong hash fails @@ -259,6 +283,13 @@ let "sha256-XwEKLdc2Y7fteSKKOERgjKTdxELy7K/wOVuB/SSj3ng="; }; }; + # Rename per-revision images so the CI push filter keeps them out of Cachix; + # each is built once and shipped through GHCR. Not applied to workspace + # builds, which are worth substituting. + source-volatile = orig: { + name = "dataplane-volatile-${orig.name or "${orig.pname}-${orig.version}"}"; + allowSubstitutes = false; + }; # For wasm32, pkgs is the host nixpkgs (no pkgsCross), so ctarget resolves to the # host platform (e.g. x86_64-unknown-linux-gnu). That means is-cross-compile is # false for wasm, which is intentional: we don't want native cross-compilation @@ -277,7 +308,7 @@ let objcopy = if is-cross-compile then "${ctarget}-objcopy" else "objcopy"; package-list = builtins.fromJSON ( builtins.readFile ( - pkgs.runCommandLocal "package-list" + (pkgs.runCommandLocal "package-list" { TOMLQ = "${pkgs.pkgsBuildHost.yq}/bin/tomlq"; JQ = "${pkgs.pkgsBuildHost.jq}/bin/jq"; @@ -294,6 +325,8 @@ let done | $JQ --sort-keys --slurp 'add' > $out '' ) + ).overrideAttrs + source-volatile ) ); version = (craneLib.crateNameFromCargoToml { inherit src; }).version; @@ -338,6 +371,10 @@ let pname = null; cargoArtifacts = null; }, + # Dependency builds retain reusable Cargo artifacts rather than binaries. + for-deps ? false, + # Skip the binary strip/split step for derivations that produce none. + no-bins ? false, profile, cargo-nextest, hwloc, @@ -372,7 +409,9 @@ let ]; env = { - VERSION = tag; + # Dependencies do not read VERSION, so keep their derivation stable + # while workspace consumers receive the per-commit tag. + VERSION = if for-deps then "dependencies" else tag; CARGO_PROFILE = cargo-profile; DATAPLANE_SYSROOT = "${sysroot}"; LIBCLANG_PATH = "${pkgs.pkgsBuildHost.llvmPackages'.libclang.lib}/lib"; @@ -389,18 +428,9 @@ let "-Clinker=${pkgs.pkgsBuildHost.llvmPackages'.clang}/bin/${cxx}" "-Clink-arg=--ld-path=${pkgs.pkgsBuildHost.llvmPackages'.lld}/bin/ld.lld" "-Clink-arg=-L${sysroot}/lib" - # NOTE: this is basically a trick to make our source code available to debuggers. - # Normally remap-path-prefix takes the form --remap-path-prefix=FROM=TO where FROM and TO are directories. - # This is intended to map source code paths to generic, relative, or redacted paths. - # We are sorta using that mechanism in reverse here in that the empty FROM in the next expression maps our - # source code in the debug info from the current working directory to ${src} (the nix store path where we - # have copied our source code). - # - # This is nice in that it should allow us to include ${src} in a container with gdb / lldb + the debug files - # we strip out of the final binaries we cook and include a gdbserver binary in some - # debug/release-with-debug-tools containers. Then, connecting from the gdb/lldb container to the - # gdb/lldbserver container should allow us to actually debug binaries deployed to test machines. - "--remap-path-prefix==${src}" + # Keep debug paths stable across revisions. Source readers + # must resolve this relative prefix from the workspace root. + "--remap-path-prefix==${src-prefix}" ] ) else @@ -409,48 +439,114 @@ let } // args )).overrideAttrs - (orig: { - separateDebugInfo = true; + ( + orig: + if for-deps || no-bins then + { + # Only dependency builds should retain crane's target archive. + doInstallCargoArtifacts = for-deps; + postBuild = (orig.postBuild or "") + '' + unset RUSTFLAGS; + ''; + } + else + { + separateDebugInfo = true; - # I'm not 100% sure if I would call it a bug in crane or a bug in cargo, but cross compile is tricky here. - # There is no easy way to distinguish RUSTFLAGS intended for the build-time dependencies from the RUSTFLAGS - # intended for the runtime dependencies. - # One unfortunate consequence of this is that if you set platform specific RUSTFLAGS then the postBuild hook - # malfunctions. Fortunately, the "fix" is easy: just unset RUSTFLAGS before the postBuild hook actually runs. - # We don't need to set any optimization flags for postBuild tooling anyway. - postBuild = (orig.postBuild or "") + '' - unset RUSTFLAGS; - ''; - postInstall = - (orig.postInstall or "") - + ( - if rustc-target != "wasm32-wasip1" then - '' - mkdir -p $debug/bin - for f in $out/bin/*; do - mv "$f" "$debug/bin/$(basename "$f")" - ${strip} --strip-debug "$debug/bin/$(basename "$f")" -o "$f" - ${objcopy} --add-gnu-debuglink="$debug/bin/$(basename "$f")" "$f" - done - '' + # I'm not 100% sure if I would call it a bug in crane or a bug in cargo, but cross compile is tricky here. + # There is no easy way to distinguish RUSTFLAGS intended for the build-time dependencies from the RUSTFLAGS + # intended for the runtime dependencies. + # One unfortunate consequence of this is that if you set platform specific RUSTFLAGS then the postBuild hook + # malfunctions. Fortunately, the "fix" is easy: just unset RUSTFLAGS before the postBuild hook actually runs. + # We don't need to set any optimization flags for postBuild tooling anyway. + postBuild = (orig.postBuild or "") + '' + unset RUSTFLAGS; + ''; + postInstall = + (orig.postInstall or "") + + ( + if rustc-target != "wasm32-wasip1" then + '' + mkdir -p $debug/bin + for f in $out/bin/*; do + mv "$f" "$debug/bin/$(basename "$f")" + ${strip} --strip-debug "$debug/bin/$(basename "$f")" -o "$f" + ${objcopy} --add-gnu-debuglink="$debug/bin/$(basename "$f")" "$f" + done + '' + else + '' + mkdir -p $debug/bin + for f in $out/bin/*; do + mv "$f" "$debug/bin/$(basename "$f")" + ${pkgs.pkgsBuildHost.binaryen}/bin/wasm-opt "$debug/bin/$(basename "$f")" --strip-debug -O4 -o "$f" + # sadly there is no equivalent of gnu-debuglink in wasm world yet + done + '' + ); + postFixup = (orig.postFixup or "") + '' + rm -f $out/target.tar.zst + ''; + } + ); + + # Share one manifest-based dependency build per flag set across workspace + # packages and revisions. Production and tests remain separate because their + # unwind and development-dependency fingerprints differ. + mk-cargo-artifacts = + { + for-tests, + cmd-prefix, + deps-profile, + }: + pkgs.callPackage invoke { + builder = craneLib.buildDepsOnly; + profile = deps-profile; + for-deps = true; + args = { + pname = if for-tests then "dataplane-tests" else "dataplane"; + cargoArtifacts = null; + buildPhaseCargoCommand = builtins.concatStringsSep " " ( + # Include dev-dependencies required by nextest archives. + ( + if for-tests then + [ + "cargo" + "test" + "--no-run" + "--profile=${cargo-profile}" + ] else - '' - mkdir -p $debug/bin - for f in $out/bin/*; do - mv "$f" "$debug/bin/$(basename "$f")" - ${pkgs.pkgsBuildHost.binaryen}/bin/wasm-opt "$debug/bin/$(basename "$f")" --strip-debug -O4 -o "$f" - # sadly there is no equivalent of gnu-debuglink in wasm world yet - done - '' - ); - postFixup = (orig.postFixup or "") + '' - rm -f $out/target.tar.zst - ''; - }); + [ + "cargo" + "build" + "--profile=${cargo-profile}" + ] + ) + # Use the consumer package set so excluded members cannot pull native + # dependencies that fail to compile for wasm32-wasip1. + ++ (map (pname: "--package=${pname}") (builtins.attrValues package-list)) + ++ cmd-prefix + ); + }; + }; + + cargo-artifacts = mk-cargo-artifacts { + for-tests = false; + cmd-prefix = cargo-cmd-prefix; + deps-profile = profile'; + }; + + cargo-artifacts-tests = mk-cargo-artifacts { + for-tests = true; + cmd-prefix = cargo-cmd-prefix-tests; + deps-profile = profile-tests'; + }; + workspace-builder = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -483,7 +579,7 @@ let workspace-check = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -516,7 +612,7 @@ let test-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -541,7 +637,7 @@ let ++ cargo-cmd-prefix-tests )) # Record the remapped source root without changing normal archives. - + (if instrumentation == "coverage" then "; echo -n '${src}' > $out/source-prefix" else ""); + + (if instrumentation == "coverage" then "; echo -n '${src-prefix}' > $out/source-prefix" else ""); }; }; @@ -559,7 +655,7 @@ let bench-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -593,24 +689,36 @@ let benches = bench-builder { }; + # Preserve all-target linting, using the test profile and dependency artifacts + # required by test and benchmark targets. clippy-builder = { - pname ? null, + package ? null, }: + let + pname = if package != null then package else "all"; + in pkgs.callPackage invoke { builder = craneLib.mkCargoDerivation; - profile = profile'; + profile = profile-tests'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts-tests; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" "clippy" + "--all-targets" "--profile=${cargo-profile}" - "--package=${pname}" ] - ++ cargo-cmd-prefix + # The platform-aware list excludes members that cannot build for WASI. + ++ ( + if package != null then + [ "--package=${pname}" ] + else + map (p: "--package=${p}") (builtins.attrValues package-list) + ) + ++ cargo-cmd-prefix-tests ++ [ "--" "-D warnings" @@ -619,12 +727,51 @@ let }; }; - clippy = builtins.mapAttrs ( - dir: pname: - clippy-builder { - inherit pname; - } - ) package-list; + # Workspace source invalidates every package together, so share one artifact + # unpack instead of paying the fixed cost per package. + clippy = { + all = clippy-builder { }; + pkg = builtins.mapAttrs (dir: package: clippy-builder { inherit package; }) package-list; + }; + + # Cargo cannot build doctests without running them, so execute them in the + # sandbox instead of trying to archive them for the host. + doctest-builder = + { + package ? null, + }: + let + pname = if package != null then package else "all"; + in + pkgs.callPackage invoke { + builder = craneLib.mkCargoDerivation; + profile = profile-tests'; + no-bins = true; + args = { + inherit pname; + cargoArtifacts = cargo-artifacts-tests; + # `cargo test --doc` runs rustdoc, which does not inherit rustc's + # registered cfg declarations either. + RUSTDOCFLAGS = "-D warnings --check-cfg=cfg(emulated) --check-cfg=cfg(instrumented)"; + # The sandbox cannot resolve the runner's `/usr/bin/env bash` shebang. + preBuild = "patchShebangs scripts/test-runner.sh"; + buildPhaseCargoCommand = builtins.concatStringsSep " " ( + [ + "cargo" + "test" + "--doc" + "--profile=${cargo-profile}" + ] + ++ (if package != null then [ "--package=${pname}" ] else [ ]) + ++ cargo-cmd-prefix-tests + ); + }; + }; + + doctests = { + all = doctest-builder { }; + pkg = builtins.mapAttrs (dir: package: doctest-builder { inherit package; }) package-list; + }; docs-builder = { @@ -638,8 +785,9 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; - RUSTDOCFLAGS = "-D warnings"; + cargoArtifacts = cargo-artifacts; + # Rustdoc does not inherit rustc's registered cfg declarations. + RUSTDOCFLAGS = "-D warnings --check-cfg=cfg(emulated) --check-cfg=cfg(instrumented)"; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" @@ -663,178 +811,186 @@ let ) package-list; }; - dataplane.tar = pkgs.stdenv'.mkDerivation { - pname = "dataplane.tar"; - inherit version; - dontUnpack = true; - src = null; - dontPatchShebangs = true; - dontFixup = true; - dontPatchElf = true; - buildPhase = - let - # `libc-pkg` and not `libc` so the outer function-arg `libc` (the - # string "gnu" / "musl" / "none") stays visible inside this scope - # for the conditional below. - libc-pkg = pkgs.pkgsHostHost.libc; - # libgcc_s.so.1 is consumed by glibc-dynamic Rust binaries for - # unwinding. musl Rust targets static-link musl + Rust's - # compiler-builtins, so libgcc has no consumer; bundling it would - # waste closure space and pull in glibc-targeted build outputs that - # are wrong for a musl container. - # - # IMPORTANT: must be the path baked into the matching ld-linux's - # compiled-in search list, which is `pkgs.pkgsHostHost.glibc.libgcc` - # (the `xgcc-...-libgcc` / cross `libgcc--...` derivation). - # `pkgs.stdenv.cc.cc.lib` ships the same `libgcc_s.so.1` content but - # at a different store path that ld-linux doesn't search, so the - # binary can't find it at runtime even though the file exists in - # the tar. - libgcc-tar-input = if libc == "gnu" then "${pkgs.pkgsHostHost.glibc.libgcc}" else ""; - # libc.out is needed by anything dynamically linked in the tar, - # regardless of libc choice. The Rust binaries on musl are - # statically linked and don't need it, but busybox (bundled below - # for `/bin/*` shell utilities) is dynamically linked against - # whichever libc its pkgset uses. Omitting libc.out on musl leaves - # busybox applets referencing a `ld-musl-*.so.1` / `libc.so` that - # isn't present in the image. - libc-tar-input = "${libc-pkg.out}"; - in - '' - tmp="$(mktemp -d)" - mkdir -p "$tmp/"{bin,lib,var,etc,run/dataplane,run/frr/hh,run/netns,home,tmp} - ln -s /run "$tmp/var/run" - for f in "${pkgs.pkgsHostHost.dockerTools.fakeNss}/etc/"* ; do - cp --archive "$(readlink -e "$f")" "$tmp/etc/$(basename "$f")" - done - cd "$tmp" - ln -s "${workspace.dataplane}/bin/dataplane" "$tmp/bin/dataplane" - ln -s "${workspace.cli}/bin/cli" "$tmp/bin/cli" - ln -s "${workspace.init}/bin/dataplane-init" "$tmp/bin/dataplane-init" - for i in "${pkgs.pkgsHostHost.busybox}/bin/"*; do - ln -s "${pkgs.pkgsHostHost.busybox}/bin/busybox" "$tmp/bin/$(basename "$i")" - done - ln -s "${workspace.dataplane}/bin/dataplane" "$tmp/dataplane" - ln -s "${workspace.init}/bin/dataplane-init" "$tmp/dataplane-init" - ln -s "${workspace.cli}/bin/cli" "$tmp/dataplane-cli" - # we take some care to make the tar file reproducible here - tar \ - --create \ - \ - --sort=name \ - \ - --clamp-mtime \ - --mtime=0 \ - \ - --format=posix \ - --numeric-owner \ - --owner=0 \ - --group=0 \ - \ - `# anybody editing the files shipped in the container image is up to no good, block all of that.` \ - `# More, we expressly forbid setuid / setgid anything.` \ - --mode='ugo-sw' \ - \ - `# acls / setcap / selinux isn't going to be reliably copied into the image; skip to make more reproducible` \ - --no-acls \ - --no-xattrs \ - --no-selinux \ - \ - `# we already copied this stuff in to /etc directly, no need to copy it into the store again.` \ - --exclude '${libc-pkg}/etc' \ - \ - `# There are a few components of glibc which have absolutely nothing to do with our goals and present` \ - `# material and trivially avoided hazards just by their presence. Thus, we filter them out here.` \ - `# None of this applies to musl (if we ever decide to ship with musl). That said, these filters will` \ - `# just not do anything in that case. ` \ - \ - `# Anybody even trying to access the glibc audit functionality in our container environment is ` \ - `# 100% up to no good.` \ - `# Intercepting and messing with dynamic library loading is _absolutely_ not on our todo list, and this ` \ - `# stuff has a history of causing security issues (arbitrary code execution). Just disarm this.` \ - `# Go check out this one, it is a classic: ` \ - `# https://www.exploit-db.com/exploits/18105 ` \ - \ - --exclude '${libc-pkg}/lib/audit*' \ - \ - `# The glibc character set conversion code is not only useless to us, is is an increasingly common attack ` \ - `# vector (see CVE-2024-2961 for example). We are 100% unicode only, so all of these legacy character ` \ - `# conversion algorithms can and should be excluded. We wouldn't run on (e.g.) old MAC hardware anyway.` \ - `# More, we have zero need or desire (or meaningful ability) to change glibc locales in the container ` \ - `# and it wouldn't be respected by rust's core/std libs anyway. ` \ - `# This is also how fedora packages glibc, and for the same basic reasons.` \ - `# See https://fedoraproject.org/wiki/Changes/Gconv_package_split_in_glibc` \ - --exclude '${libc-pkg}/lib/gconv*' \ - --exclude '${libc-pkg}/share/i18n*' \ - --exclude '${libc-pkg}/share/locale*' \ - \ - `# getconf isn't even shipped in the container so this is useless. You couldn't change limits in the ` \ - `# container like this anyway. Even if we needed to and could, we wouldn't use setconf et al.` \ - --exclude '${libc-pkg}/libexec*' \ - \ - --verbose \ - --file "$out" \ - \ - . \ - ${libc-tar-input} \ - ${libgcc-tar-input} \ - ${workspace.dataplane} \ - ${workspace.init} \ - ${workspace.cli} \ - ${pkgs.pkgsHostHost.busybox} - ''; - }; + dataplane.tar = + (pkgs.stdenv'.mkDerivation { + pname = "dataplane.tar"; + inherit version; + dontUnpack = true; + src = null; + dontPatchShebangs = true; + dontFixup = true; + dontPatchElf = true; + buildPhase = + let + # `libc-pkg` and not `libc` so the outer function-arg `libc` (the + # string "gnu" / "musl" / "none") stays visible inside this scope + # for the conditional below. + libc-pkg = pkgs.pkgsHostHost.libc; + # libgcc_s.so.1 is consumed by glibc-dynamic Rust binaries for + # unwinding. musl Rust targets static-link musl + Rust's + # compiler-builtins, so libgcc has no consumer; bundling it would + # waste closure space and pull in glibc-targeted build outputs that + # are wrong for a musl container. + # + # IMPORTANT: must be the path baked into the matching ld-linux's + # compiled-in search list, which is `pkgs.pkgsHostHost.glibc.libgcc` + # (the `xgcc-...-libgcc` / cross `libgcc--...` derivation). + # `pkgs.stdenv.cc.cc.lib` ships the same `libgcc_s.so.1` content but + # at a different store path that ld-linux doesn't search, so the + # binary can't find it at runtime even though the file exists in + # the tar. + libgcc-tar-input = if libc == "gnu" then "${pkgs.pkgsHostHost.glibc.libgcc}" else ""; + # libc.out is needed by anything dynamically linked in the tar, + # regardless of libc choice. The Rust binaries on musl are + # statically linked and don't need it, but busybox (bundled below + # for `/bin/*` shell utilities) is dynamically linked against + # whichever libc its pkgset uses. Omitting libc.out on musl leaves + # busybox applets referencing a `ld-musl-*.so.1` / `libc.so` that + # isn't present in the image. + libc-tar-input = "${libc-pkg.out}"; + in + '' + tmp="$(mktemp -d)" + mkdir -p "$tmp/"{bin,lib,var,etc,run/dataplane,run/frr/hh,run/netns,home,tmp} + ln -s /run "$tmp/var/run" + for f in "${pkgs.pkgsHostHost.dockerTools.fakeNss}/etc/"* ; do + cp --archive "$(readlink -e "$f")" "$tmp/etc/$(basename "$f")" + done + cd "$tmp" + ln -s "${workspace.dataplane}/bin/dataplane" "$tmp/bin/dataplane" + ln -s "${workspace.cli}/bin/cli" "$tmp/bin/cli" + ln -s "${workspace.init}/bin/dataplane-init" "$tmp/bin/dataplane-init" + for i in "${pkgs.pkgsHostHost.busybox}/bin/"*; do + ln -s "${pkgs.pkgsHostHost.busybox}/bin/busybox" "$tmp/bin/$(basename "$i")" + done + ln -s "${workspace.dataplane}/bin/dataplane" "$tmp/dataplane" + ln -s "${workspace.init}/bin/dataplane-init" "$tmp/dataplane-init" + ln -s "${workspace.cli}/bin/cli" "$tmp/dataplane-cli" + # we take some care to make the tar file reproducible here + tar \ + --create \ + \ + --sort=name \ + \ + --clamp-mtime \ + --mtime=0 \ + \ + --format=posix \ + --numeric-owner \ + --owner=0 \ + --group=0 \ + \ + `# anybody editing the files shipped in the container image is up to no good, block all of that.` \ + `# More, we expressly forbid setuid / setgid anything.` \ + --mode='ugo-sw' \ + \ + `# acls / setcap / selinux isn't going to be reliably copied into the image; skip to make more reproducible` \ + --no-acls \ + --no-xattrs \ + --no-selinux \ + \ + `# we already copied this stuff in to /etc directly, no need to copy it into the store again.` \ + --exclude '${libc-pkg}/etc' \ + \ + `# There are a few components of glibc which have absolutely nothing to do with our goals and present` \ + `# material and trivially avoided hazards just by their presence. Thus, we filter them out here.` \ + `# None of this applies to musl (if we ever decide to ship with musl). That said, these filters will` \ + `# just not do anything in that case. ` \ + \ + `# Anybody even trying to access the glibc audit functionality in our container environment is ` \ + `# 100% up to no good.` \ + `# Intercepting and messing with dynamic library loading is _absolutely_ not on our todo list, and this ` \ + `# stuff has a history of causing security issues (arbitrary code execution). Just disarm this.` \ + `# Go check out this one, it is a classic: ` \ + `# https://www.exploit-db.com/exploits/18105 ` \ + \ + --exclude '${libc-pkg}/lib/audit*' \ + \ + `# The glibc character set conversion code is not only useless to us, is is an increasingly common attack ` \ + `# vector (see CVE-2024-2961 for example). We are 100% unicode only, so all of these legacy character ` \ + `# conversion algorithms can and should be excluded. We wouldn't run on (e.g.) old MAC hardware anyway.` \ + `# More, we have zero need or desire (or meaningful ability) to change glibc locales in the container ` \ + `# and it wouldn't be respected by rust's core/std libs anyway. ` \ + `# This is also how fedora packages glibc, and for the same basic reasons.` \ + `# See https://fedoraproject.org/wiki/Changes/Gconv_package_split_in_glibc` \ + --exclude '${libc-pkg}/lib/gconv*' \ + --exclude '${libc-pkg}/share/i18n*' \ + --exclude '${libc-pkg}/share/locale*' \ + \ + `# getconf isn't even shipped in the container so this is useless. You couldn't change limits in the ` \ + `# container like this anyway. Even if we needed to and could, we wouldn't use setconf et al.` \ + --exclude '${libc-pkg}/libexec*' \ + \ + --verbose \ + --file "$out" \ + \ + . \ + ${libc-tar-input} \ + ${libgcc-tar-input} \ + ${workspace.dataplane} \ + ${workspace.init} \ + ${workspace.cli} \ + ${pkgs.pkgsHostHost.busybox} + ''; + }).overrideAttrs + source-volatile; - containers.dataplane = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane"; - inherit tag; - contents = pkgs.buildEnv { - name = "dataplane-env"; - pathsToLink = [ - "/bin" - "/etc" - "/var" - "/lib" - ]; - paths = [ - pkgs.pkgsHostHost.dockerTools.fakeNss - pkgs.pkgsHostHost.busybox - pkgs.pkgsHostHost.dockerTools.usrBinEnv - workspace.cli - workspace.dataplane - workspace.init - ]; - }; - config.Entrypoint = [ "/bin/dataplane" ]; - }; + containers.dataplane = + (pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane"; + inherit tag; + contents = + (pkgs.buildEnv { + name = "dataplane-env"; + pathsToLink = [ + "/bin" + "/etc" + "/var" + "/lib" + ]; + paths = [ + pkgs.pkgsHostHost.dockerTools.fakeNss + pkgs.pkgsHostHost.busybox + pkgs.pkgsHostHost.dockerTools.usrBinEnv + workspace.cli + workspace.dataplane + workspace.init + ]; + }).overrideAttrs + source-volatile; + config.Entrypoint = [ "/bin/dataplane" ]; + }).overrideAttrs + source-volatile; - containers.dataplane-debugger = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane/debugger"; - inherit tag; - contents = pkgs.buildEnv { - name = "dataplane-debugger-env"; - pathsToLink = [ - "/bin" - "/etc" - "/var" - "/lib" - ]; - paths = [ - pkgs.pkgsBuildHost.gdb - pkgs.pkgsBuildHost.rr - pkgs.pkgsBuildHost.coreutils - pkgs.pkgsBuildHost.bashInteractive - pkgs.pkgsBuildHost.iproute2 - pkgs.pkgsBuildHost.ethtool - pkgs.pkgsHostHost.dockerTools.usrBinEnv + containers.dataplane-debugger = + (pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/debugger"; + inherit tag; + contents = pkgs.buildEnv { + name = "dataplane-debugger-env"; + pathsToLink = [ + "/bin" + "/etc" + "/var" + "/lib" + ]; + paths = [ + pkgs.pkgsBuildHost.gdb + pkgs.pkgsBuildHost.rr + pkgs.pkgsBuildHost.coreutils + pkgs.pkgsBuildHost.bashInteractive + pkgs.pkgsBuildHost.iproute2 + pkgs.pkgsBuildHost.ethtool + pkgs.pkgsHostHost.dockerTools.usrBinEnv - pkgs.pkgsHostHost.libc.debug - workspace.cli.debug - workspace.dataplane.debug - workspace.init.debug - ]; - }; - }; + pkgs.pkgsHostHost.libc.debug + workspace.cli.debug + workspace.dataplane.debug + workspace.init.debug + ]; + }; + }).overrideAttrs + source-volatile; debug-tools = pkgs: @@ -883,149 +1039,155 @@ let pkgs.pkgsHostHost.glibc.libgcc ]; - containers.debug-tools = pkgs.dockerTools.buildLayeredImage { - name = "debug-tools"; - tag = "dev"; # don't push or tag this with anything that might end up in the production repo - contents = pkgs.buildEnv { - name = "debug-tools-env"; - pathsToLink = [ - "/bin" - "/etc" - "/lib" - "/libexec" - "/share" - "/tmp" - "/usr" - "/var" - ]; - paths = debug-tools pkgs; - }; - - fakeRootCommands = '' - #!${pkgs.bash}/bin/bash - set -euo pipefail - mkdir -p /{bin,lib,var,etc,run/dataplane,run/frr/hh,run/netns,home,tmp} - ln -s /run /var/run - # symlinks to help imitate the real image - ln -s /bin/dataplane /dataplane - ln -s /bin/cli /dataplane-cli - ln -s /bin/dataplane-init /dataplane-init - ''; + containers.debug-tools = + (pkgs.dockerTools.buildLayeredImage { + name = "debug-tools"; + tag = "dev"; # don't push or tag this with anything that might end up in the production repo + contents = pkgs.buildEnv { + name = "debug-tools-env"; + pathsToLink = [ + "/bin" + "/etc" + "/lib" + "/libexec" + "/share" + "/tmp" + "/usr" + "/var" + ]; + paths = debug-tools pkgs; + }; - enableFakechroot = true; + fakeRootCommands = '' + #!${pkgs.bash}/bin/bash + set -euo pipefail + mkdir -p /{bin,lib,var,etc,run/dataplane,run/frr/hh,run/netns,home,tmp} + ln -s /run /var/run + # symlinks to help imitate the real image + ln -s /bin/dataplane /dataplane + ln -s /bin/cli /dataplane-cli + ln -s /bin/dataplane-init /dataplane-init + ''; - }; + enableFakechroot = true; - containers.frr.dataplane = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane/frr"; - inherit tag; - contents = pkgs.buildEnv { - name = "dataplane-frr-env"; - pathsToLink = [ - "/bin" - "/etc" - "/lib" - "/libexec" - "/share" - "/usr" - "/var" - ]; - paths = with pkgs; [ - bash - coreutils - dockerTools.usrBinEnv - fancy.dplane-plugin - fancy.dplane-rpc - fancy.frr-agent - fancy.frr-config - fancy.frr.dataplane - findutils - gnugrep - iproute2 - jq - prometheus-frr-exporter - python3Minimal - tini - ]; - }; + }).overrideAttrs + source-volatile; - fakeRootCommands = '' - #!${pkgs.bash}/bin/bash - set -euxo pipefail - mkdir /tmp - mkdir -p /run/frr/hh - chown -R frr:frr /run/frr - mkdir -p /var - ln -s /run /var/run - chown -R frr:frr /var/run/frr - rm /etc/passwd /etc/group - cp ${pkgs.fancy.frr-config}/etc/passwd /etc/passwd - cp ${pkgs.fancy.frr-config}/etc/group /etc/group - ''; + containers.frr.dataplane = + (pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/frr"; + inherit tag; + contents = pkgs.buildEnv { + name = "dataplane-frr-env"; + pathsToLink = [ + "/bin" + "/etc" + "/lib" + "/libexec" + "/share" + "/usr" + "/var" + ]; + paths = with pkgs; [ + bash + coreutils + dockerTools.usrBinEnv + fancy.dplane-plugin + fancy.dplane-rpc + fancy.frr-agent + fancy.frr-config + fancy.frr.dataplane + findutils + gnugrep + iproute2 + jq + prometheus-frr-exporter + python3Minimal + tini + ]; + }; - enableFakechroot = true; + fakeRootCommands = '' + #!${pkgs.bash}/bin/bash + set -euxo pipefail + mkdir /tmp + mkdir -p /run/frr/hh + chown -R frr:frr /run/frr + mkdir -p /var + ln -s /run /var/run + chown -R frr:frr /var/run/frr + rm /etc/passwd /etc/group + cp ${pkgs.fancy.frr-config}/etc/passwd /etc/passwd + cp ${pkgs.fancy.frr-config}/etc/group /etc/group + ''; - config.Entrypoint = [ - "/bin/tini" - "--" - ]; - config.Cmd = [ "/libexec/frr/docker-start" ]; - }; + enableFakechroot = true; - containers.frr.host = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane/frr-host"; - inherit tag; - contents = pkgs.buildEnv { - name = "dataplane-frr-host-env"; - pathsToLink = [ - "/bin" - "/etc" - "/lib" - "/libexec" - "/share" - "/usr" - "/var" + config.Entrypoint = [ + "/bin/tini" + "--" ]; - paths = with pkgs; [ - bash - coreutils - dockerTools.usrBinEnv - # TODO: frr-config's docker-start launches /bin/frr-agent which is not - # present in the host container. A host-specific entrypoint script may - # be needed once this container is actively deployed. - fancy.frr-config - fancy.frr.host - findutils - gnugrep - iproute2 - jq - prometheus-frr-exporter - python3Minimal - tini - ]; - }; - fakeRootCommands = '' - #!${pkgs.bash}/bin/bash - set -euxo pipefail - mkdir /tmp - mkdir -p /run/frr/hh - chown -R frr:frr /run/frr - mkdir -p /var - ln -s /run /var/run - chown -R frr:frr /var/run/frr - rm /etc/passwd /etc/group - cp ${pkgs.fancy.frr-config}/etc/passwd /etc/passwd - cp ${pkgs.fancy.frr-config}/etc/group /etc/group - ''; + config.Cmd = [ "/libexec/frr/docker-start" ]; + }).overrideAttrs + source-volatile; - enableFakechroot = true; + containers.frr.host = + (pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/frr-host"; + inherit tag; + contents = pkgs.buildEnv { + name = "dataplane-frr-host-env"; + pathsToLink = [ + "/bin" + "/etc" + "/lib" + "/libexec" + "/share" + "/usr" + "/var" + ]; + paths = with pkgs; [ + bash + coreutils + dockerTools.usrBinEnv + # TODO: frr-config's docker-start launches /bin/frr-agent which is not + # present in the host container. A host-specific entrypoint script may + # be needed once this container is actively deployed. + fancy.frr-config + fancy.frr.host + findutils + gnugrep + iproute2 + jq + prometheus-frr-exporter + python3Minimal + tini + ]; + }; + fakeRootCommands = '' + #!${pkgs.bash}/bin/bash + set -euxo pipefail + mkdir /tmp + mkdir -p /run/frr/hh + chown -R frr:frr /run/frr + mkdir -p /var + ln -s /run /var/run + chown -R frr:frr /var/run/frr + rm /etc/passwd /etc/group + cp ${pkgs.fancy.frr-config}/etc/passwd /etc/passwd + cp ${pkgs.fancy.frr-config}/etc/group /etc/group + ''; - config.Entrypoint = [ - "/bin/tini" - "--" - ]; - config.Cmd = [ "/libexec/frr/docker-start" ]; - }; + enableFakechroot = true; + + config.Entrypoint = [ + "/bin/tini" + "--" + ]; + config.Cmd = [ "/libexec/frr/docker-start" ]; + }).overrideAttrs + source-volatile; in { @@ -1037,10 +1199,12 @@ in dataplane devenv devroot + doctests docs package-list pkgs sources + src sysroot tests workspace diff --git a/development/code/running-tests.md b/development/code/running-tests.md index a8bac9880d..3c4d81e90c 100644 --- a/development/code/running-tests.md +++ b/development/code/running-tests.md @@ -21,6 +21,26 @@ even if you have not installed [nextest] on your system. cargo nextest run --cargo-profile=release ``` +## Linting (clippy) + +`just clippy` builds clippy through nix, so a developer and CI run the same +thing and the result is cached. Its first argument is a _package_, not a flag: + +```shell +just clippy # the whole workspace +just clippy nat # one package, as with `just test` +``` + +`just clippy -p nat` does not work -- `-p` binds to the package parameter and +the rest is forwarded to `nix build`. It fails rather than silently linting the +wrong thing, but the spelling above is the one that works. + +For a fast inner loop, skip the recipe and use the dev shell directly: + +```shell +cargo clippy --all-targets +``` + ## Code Coverage (llvm-cov) The nix-shell also ships with [cargo llvm-cov] for collecting diff --git a/justfile b/justfile index 93227c9f26..6eed433ff8 100644 --- a/justfile +++ b/justfile @@ -329,26 +329,75 @@ build-container-quick: push-container target="dataplane" *args: (build-container target args) && version {{ _just_debuggable_ }} declare -xr DOCKER_HOST="${DOCKER_HOST:-unix://{{docker_sock}}}" + + # Preserve completed builds across transient registry failures. Skopeo + # retries blobs; this outer loop retries known-safe, idempotent whole pushes + # and announces them so registry degradation remains visible. + retry() { + declare -r what="$1" + shift + declare -ri attempts=4 + declare -i attempt=1 + declare -i delay + declare log + log="$(mktemp)" + declare -r log + while true; do + # Stream multi-gigabyte pushes so they do not appear hung. + if "$@" 2>&1 | tee "${log}"; then + rm -f -- "${log}" + return 0 + fi + if [ "${attempt}" -ge "${attempts}" ]; then + >&2 echo "::error::${what} failed after ${attempts} attempts" + rm -f -- "${log}" + return 1 + fi + # Match registry status and transport vocabulary shared by skopeo + # and oras. Bound 403 so it cannot match inside a digest. + if ! grep -qiE \ + -e 'blob upload (unknown|invalid)|blob transfer' \ + -e '\b(403|429|500|502|503|504)\b' \ + -e 'forbidden|denied|too many requests|rate limit' \ + -e 'internal server error|bad gateway|service unavailable|gateway time-?out' \ + -e 'temporarily unavailable|try again' \ + -e 'unexpected EOF|connection reset|broken pipe|i/o timeout|TLS handshake' \ + "${log}"; then + >&2 echo "::error::${what} failed with a non-retryable error" + rm -f -- "${log}" + return 1 + fi + delay=$(( 5 * 2 ** (attempt - 1) + RANDOM % 5 )) + >&2 echo "::warning::${what} failed (attempt ${attempt}/${attempts}), retrying in ${delay}s" + sleep "${delay}" + attempt=$(( attempt + 1 )) + done + } + + push_image() { + declare -r image="$1" + retry "push of ${image}" \ + skopeo copy --retry-times=3 --src-daemon-host="${DOCKER_HOST}" \ + {{ _skopeo_dest_insecure }} "docker-daemon:${image}" "docker://${image}" + echo "Pushed ${image}" + } + case "{{target}}" in "dataplane") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane }}" "docker://{{ oci_image_dataplane }}" - echo "Pushed {{ oci_image_dataplane }}" + push_image "{{ oci_image_dataplane }}" ;; "dataplane-debugger") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_debugger }}" "docker://{{ oci_image_dataplane_debugger }}" - echo "Pushed {{ oci_image_dataplane_debugger }}" + push_image "{{ oci_image_dataplane_debugger }}" ;; "debug-tools") >&2 echo "do not push the debug tools!" exit 1 ;; "frr.dataplane") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{oci_image_frr_dataplane}}" "docker://{{oci_image_frr_dataplane}}" - echo "Pushed {{ oci_image_frr_dataplane }}" + push_image "{{oci_image_frr_dataplane}}" ;; "frr.host") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{oci_image_frr_host}}" "docker://{{oci_image_frr_host}}" - echo "Pushed {{ oci_image_frr_host }}" + push_image "{{oci_image_frr_host}}" ;; "validator") if [ "{{platform}}" != "wasm32-wasip1" ]; then @@ -356,7 +405,8 @@ push-container target="dataplane" *args: (build-container target args) && versio exit 1 fi pushd ./results/workspace.validator/bin - oras push --annotation version="{{ version }}" "{{ oci_image_dataplane_validator }}" ./validator.wasm + retry "push of {{ oci_image_dataplane_validator }}" \ + oras push --annotation version="{{ version }}" "{{ oci_image_dataplane_validator }}" ./validator.wasm popd echo "Pushed {{ oci_image_dataplane_validator }}" ;; @@ -391,6 +441,94 @@ check-dependencies *args: {{ _just_debuggable_ }} cargo deny {{ _cargo_feature_flags }} check {{ args }} +# Ensure the shared dependency derivations stay reusable across revisions. +# +# Two things break that, and they are different kinds of thing, so they take +# different questions. +# +# The workspace source is a store path, so "the dependency build must not +# depend on it" is a statement about the derivation graph and nix can answer it +# outright: instantiate once and read the inputs. That is exact, it names the +# offending path, and it needs no edit to the working tree. +# +# The git version is a string. It reaches a derivation as an environment +# variable and never as an input path, so no graph walk can see it; the only +# way to ask is to instantiate under two tags and compare. +# +# Both have regressed before, and both surface as a slow cache miss rather than +# a failure, which is why they are checked at all. +[script] +check-deps-reuse: + {{ _just_debuggable_ }} + # Keep Nix stderr; it is the only diagnostic when instantiation fails. + declare src + src="$(nix eval --raw --impure --expr '(import ./default.nix { }).src.outPath')" + declare -r src + if [ -z "${src}" ]; then + >&2 echo "::error::could not resolve the workspace source path" + exit 1 + fi + + deps_drv() { + declare drv + drv="$(nix-instantiate default.nix -A "$1" --argstr tag "$3" | tail -1)" + grep -ao "/nix/store/[a-z0-9]\{32\}-$2[^\"]*\.drv" "${drv}" | sort -u + } + + # Report through the status; command substitution would run this in a + # subshell and discard failure-count updates. + check_reuse() { + declare -r attr="$1" name="$2" + + declare baseline + if ! baseline="$(deps_drv "${attr}" "${name}" dev)" || [ -z "${baseline}" ]; then + >&2 echo "::error::could not resolve ${name} from ${attr}" + return 1 + fi + + # The source question, put to the graph. + declare drv + while IFS= read -r drv; do + [ -z "${drv}" ] && continue + if nix-store -q --requisites "${drv}" | grep -qxF "${src}"; then + >&2 echo "::error::${name} depends on the workspace source" + >&2 echo " ${src}" + >&2 echo " is a build input of ${drv}" + return 1 + fi + done <<<"${baseline}" + + # The version question, put to two instantiations. + declare tagged + if ! tagged="$(deps_drv "${attr}" "${name}" v0.25.2-15-gdeadbee-dirty)"; then + >&2 echo "::error::could not resolve ${name} from ${attr} with a release tag" + return 1 + fi + if [ "${tagged}" != "${baseline}" ]; then + >&2 echo "::error::${name} depends on the git version" + >&2 echo " tag=dev -> ${baseline}" + >&2 echo " tag=v0.. -> ${tagged}" + return 1 + fi + + printf '%s is reusable: %s\n' "${name}" "${baseline}" + } + + # Check production and test flag sets independently: a regression in a + # production-only flag would sail past a test-only guard. + declare -r -A targets=( + [workspace.dataplane]="dataplane-deps" + [tests.all]="dataplane-tests-deps" + ) + declare -i failures=0 + for attr in "${!targets[@]}"; do + check_reuse "${attr}" "${targets[${attr}]}" || failures=$(( failures + 1 )) + done + + if [ "${failures}" -ne 0 ]; then + exit 1 + fi + [script] opengrep: {{ _just_debuggable_ }} @@ -406,16 +544,133 @@ zizmor *args="": {{ _just_debuggable_ }} zizmor --persona=pedantic {{args}} . -[script] -clippy *args: +# Run the CI-equivalent cached lint; direct Cargo remains the fast inner loop. +clippy package="" *args: (build (if package == "" { "clippy.all" } else { "clippy.pkg." + package }) args) {{ _just_debuggable_ }} - cargo clippy --all-targets {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} -- -D warnings [script] actionlint: {{ _just_debuggable_ }} actionlint +# Keep default.nix formatted without adopting legacy files under nix/. +[script] +nixfmt *args="--check": + {{ _just_debuggable_ }} + nixfmt {{ args }} default.nix + +# Keep the lint recipe, workflow steps, and outcome aggregation aligned; drift +# in any of the three silently disables a check. +[script] +check-lint-wiring: + {{ _just_debuggable_ }} + declare -r wf=".github/workflows/dev.yml" + declare -i failures=0 + + # Nix-backed checks run under a `ci::check-` prefix; accept either spelling + # so a recipe stays covered when it moves between workflow jobs. + declare recipe + while read -r recipe; do + grep -qE "recipe: \"(ci::check-)?${recipe}\"" "${wf}" && continue + >&2 echo "::error::\`just lint\` runs ${recipe}, but ${wf} never does" + failures=$(( failures + 1 )) + done < <(just --dump --dump-format json | jq -r '.recipes.lint.dependencies[].recipe') + + # Every lint step is `continue-on-error`, so a step the aggregator does not + # read cannot fail the run. + declare id + while read -r id; do + grep -qF "steps.${id}.outcome" "${wf}" && continue + >&2 echo "::error::${wf} runs ${id} but never reads its outcome" + failures=$(( failures + 1 )) + done < <(yq -r '.jobs.lint.steps[] | select(.id) | .id' "${wf}") + + if [ "${failures}" -ne 0 ]; then + exit 1 + fi + echo "lint wiring agrees" + +# Images are per-revision, so verify that each realized image and its +# dockerTools assembly paths are rejected by the actual Cachix push filter. +[script] +check-push-filter: + {{ _just_debuggable_ }} + declare -r action=".github/actions/nix-shell/action.yml" + declare push_filter + push_filter="$(yq -r '.runs.steps[] | select(.with.pushFilter) | .with.pushFilter' "${action}")" + declare -r push_filter + if [ -z "${push_filter}" ] || [ "${push_filter}" = "null" ]; then + >&2 echo "::error::no pushFilter found in ${action}; nothing is keeping images out of the cache" + exit 1 + fi + + # Walk `containers` and `dataplane` both: the latter holds `dataplane.tar`, + # which the release build selects directly and which is per-revision for the + # same reasons. Both nest, so recurse rather than assume one level. Each line is "attr outPath drvPath". + declare -a images=() + mapfile -t images < <( + nix eval --impure --raw --expr ' + let + d = import ./default.nix { }; + lib = d.pkgs.lib; + flatten = prefix: set: + lib.concatLists (lib.mapAttrsToList (n: v: + let nm = if prefix == "" then n else "${prefix}.${n}"; in + if lib.isDerivation v then [ "${nm} ${v.outPath} ${v.drvPath}" ] + else if builtins.isAttrs v then flatten nm v + else [ ] + ) set); + in lib.concatStringsSep "\n" (flatten "" { inherit (d) containers dataplane; }) + "\n" + ' + ) + if [ "${#images[@]}" -eq 0 ]; then + >&2 echo "::error::found no container images to check; did the attribute move?" + exit 1 + fi + + declare -i failures=0 + declare -i checked=0 + for entry in "${images[@]}"; do + [ -z "${entry}" ] && continue + declare name out drv base + read -r name out drv <<<"${entry}" + # Recover the base name after `source-volatile` has renamed the output. + base="${out##*/}" + base="${base#*-dataplane-volatile-}" + base="${base%.tar.gz}" + + declare -a candidates=( "${out}" ) + # Test dockerTools artifacts; ordinary closure dependencies stay cached. + mapfile -t -O "${#candidates[@]}" candidates < <( + nix-store -q --requisites "${drv}" 2>/dev/null \ + | grep '\.drv$' \ + | xargs -r nix-store -q --outputs 2>/dev/null \ + | sort -u \ + | while IFS= read -r path; do + declare stem="${path##*/}" + case "${stem#*-}" in + "${base}-base.json" | "${base}-conf.json" \ + | "${base}-customisation-layer" | "${base}-env" \ + | "stream-${base}") printf '%s\n' "${path}" ;; + esac + done + ) + + for path in "${candidates[@]}"; do + checked=$(( checked + 1 )) + if ! printf '%s\n' "${path}" | grep -qE "${push_filter}"; then + >&2 echo "::error::${name} would push ${path##*/} to Cachix" + failures=$(( failures + 1 )) + fi + done + done + + if [ "${failures}" -ne 0 ]; then + >&2 echo "::error::extend the pushFilter in ${action}, or mark the image with \`source-volatile\`" + exit 1 + fi + printf 'no cache leak: %d paths across %d artifacts\n' "${checked}" "${#images[@]}" + # Limit linting to tracked Markdown so generated files cannot affect CI. [script] markdownlint *args: @@ -469,14 +724,16 @@ lint: \ (pinact "--fix=false" "--no-api") \ (actionlint) \ (markdownlint) \ + (nixfmt) \ + (check-lint-wiring) \ + (check-push-filter) \ (license-headers) {{ _just_debuggable_ }} -# Run doctests -[script] -doctest *args: +# Cargo cannot archive doctests, so run them inside the Nix sandbox. +doctest package="" *args: (build (if package == "" { "doctests.all" } else { "doctests.pkg." + package }) args) {{ _just_debuggable_ }} - cargo test --doc {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} + # Run instrumented tests and report coverage. Args are forwarded to nextest; for example, # `just coverage -p dataplane-nat` scopes the run to this crate. @@ -538,6 +795,20 @@ coverage-archive package="tests.all" *args: declare src_prefix src_prefix="$(cat "${prefix_file}")" declare -r src_prefix + # llvm-cov resolves relative remaps against the vanished build sandbox. + # Reject absolute prefixes and redirect relative ones to this worktree. + case "${src_prefix}" in + /*) + >&2 echo "::error::source prefix ${src_prefix} is absolute; coverage expects a relative remap" + exit 1 + ;; + esac + + # llvm-cov emits the resolved absolute paths and Codecov wants them relative + # to the repository root, so the root goes into a BRE below. Escape it. + declare root_re + root_re="$(sed -e 's#[].[^$*\\/]#\\&#g' <<<"${root}")" + declare -r root_re # Nextest changes cwd; `%m` also pools compatible profiles across tests. export LLVM_PROFILE_FILE="${profraw}/cov-%m.profraw" @@ -579,32 +850,41 @@ coverage-archive package="tests.all" *args: "${extract}/target/nextest/binaries-metadata.json" ) + # Resolve remapped paths against this worktree and filter out the standard + # library and native dependencies. llvm-cov ignores nonexistent filters, + # so the trailing path must name the real tree. + declare -ra scope=( --compilation-dir="${root}" "${objects[@]}" "${root}" ) + llvm-cov export \ --format=lcov \ --instr-profile="${out}/coverage.profdata" \ - "${objects[@]}" \ - "${src_prefix}" \ - | sed -e "s#^SF:${src_prefix}/#SF:#" > "${out}/lcov.info" + "${scope[@]}" \ + | sed -e "s#^SF:${root_re}/#SF:#" > "${out}/lcov.info" # Codecov needs repository-relative paths; reject failed rewrites. if grep -q '^SF:/' "${out}/lcov.info"; then - >&2 echo "::error::absolute paths survived the ${src_prefix} rewrite:" + >&2 echo "::error::absolute paths survived the ${root} rewrite:" >&2 grep -m5 '^SF:/' "${out}/lcov.info" exit 1 fi + # A filter that matches nothing reports full coverage of an empty set, which + # reads as success everywhere downstream. Insist on some workspace source. + if ! grep -q '^SF:' "${out}/lcov.info"; then + >&2 echo "::error::no workspace sources in the report; the ${root} filter matched nothing" + exit 1 + fi + llvm-cov show \ --format=html \ --output-dir="${out}/html" \ --show-branches=count \ --instr-profile="${out}/coverage.profdata" \ - "${objects[@]}" \ - "${src_prefix}" + "${scope[@]}" llvm-cov report \ --instr-profile="${out}/coverage.profdata" \ - "${objects[@]}" \ - "${src_prefix}" + "${scope[@]}" echo "lcov report: ${out}/lcov.info" echo "html report: ${out}/html/index.html" diff --git a/lifecycle/src/lib.rs b/lifecycle/src/lib.rs index e7519272da..9d589a136b 100644 --- a/lifecycle/src/lib.rs +++ b/lifecycle/src/lib.rs @@ -162,7 +162,7 @@ impl Subsystem { } /// Default drain deadlines. Per-subsystem deadlines bound only the -/// tokio tasks tracked by each [`Subsystem`]; [`TOTAL`] is the absolute +/// tokio tasks tracked by each [`Subsystem`]; [`default_deadlines::TOTAL`] is the absolute /// process-level ceiling enforced by [`spawn_shutdown_watchdog`]. pub mod default_deadlines { use std::time::Duration; diff --git a/miri.just b/miri.just index 495bb1a790..091da10355 100644 --- a/miri.just +++ b/miri.just @@ -43,7 +43,7 @@ test *args="": declare -rx CARGO_INCREMENTAL=0 declare MIRIFLAGS="" # Environment RUSTFLAGS replace the cargo-configured flags. - declare RUSTFLAGS="--cfg=tokio_unstable --check-cfg=cfg(emulated) " + declare RUSTFLAGS="--cfg=tokio_unstable --check-cfg=cfg(emulated) --check-cfg=cfg(instrumented) " MIRIFLAGS+="-Zmiri-compare-exchange-weak-failure-rate=${weak_failure_rate} " MIRIFLAGS+="-Zmiri-disable-isolation " MIRIFLAGS+="-Zmiri-many-seeds=${START_SEED}..${END_SEED} " diff --git a/net/src/headers/embedded_view.rs b/net/src/headers/embedded_view.rs index 85a0d8a984..fe6d5eba0e 100644 --- a/net/src/headers/embedded_view.rs +++ b/net/src/headers/embedded_view.rs @@ -19,7 +19,7 @@ //! [`EmbeddedTransport`] enum, because the inner packet may have been //! truncated by the ICMP source. //! * IPv6 extension-header gap-check semantics carry over unchanged via -//! the embedded variants on [`ExtGapCheck`](super::pat::ExtGapCheck) +//! the embedded variants on [`ExtGapCheck`] //! (`ext_gap_ok_embedded`). //! //! [`EmbeddedHeadersView`] is the type-level qualifier that closes @@ -63,9 +63,9 @@ use super::{EmbeddedHeaders, EmbeddedStart, EmbeddedTransport, Headers, Net, Net /// Declared, checkable shapes for embedded ICMP-error payloads. /// /// Any tuple whose layers chain through the [`Within`] adjacency -/// graph and the [`EmbeddedStep`] trait is a valid embedded shape. +/// graph and the `EmbeddedStep` trait is a valid embedded shape. /// External crates cannot add new shapes (they cannot implement -/// [`EmbeddedStep`]), but they can write any existing shape at the type +/// `EmbeddedStep`), but they can write any existing shape at the type /// level and let the trait bounds do the filtering. pub trait EmbeddedShape: embedded_sealed::Sealed {} @@ -525,7 +525,7 @@ pub trait EmbeddedLook { /// Extract typed references to the matched inner layers. /// /// Compiles to the same sequence of variant reads as - /// [`embedded_sealed::Sealed::matches`], plus `unwrap_unchecked` + /// `embedded_sealed::Sealed::matches`, plus `unwrap_unchecked` /// at each step; the `EmbeddedHeadersView` type invariant /// guarantees success so the `None` branches are pruned by the /// optimizer. diff --git a/net/src/headers/mod.rs b/net/src/headers/mod.rs index bedb460ddf..22fb5237ca 100644 --- a/net/src/headers/mod.rs +++ b/net/src/headers/mod.rs @@ -844,7 +844,7 @@ impl Headers { /// /// # Errors /// - /// Returns [`PushVlanError::TooManyVlans`] if there are already [`MAX_VLANS`] VLANs on the + /// Returns [`PushVlanError::TooManyVlans`] if there are already `MAX_VLANS` VLANs on the /// stack. /// Returns [`PushVlanError::NoEthernetHeader`] if no Ethernet header is present. pub fn push_vlan(&mut self, vid: Vid) -> Result<(), PushVlanError> { diff --git a/net/src/headers/pat.rs b/net/src/headers/pat.rs index 3f24d9c9db..a5f640119d 100644 --- a/net/src/headers/pat.rs +++ b/net/src/headers/pat.rs @@ -215,7 +215,7 @@ impl_strict_ext!(HopByHop, DestOpts, Routing, Fragment, Ipv4Auth, Ipv6Auth); /// /// `Acc` is the tuple of references accumulated so far. /// -/// Runtime cursors track progress into the `vlan` and `net_ext` [`ArrayVec`] +/// Runtime cursors track progress into the `vlan` and `net_ext` `ArrayVec` /// fields so that skipped intermediate layers are detected. #[must_use = "a Matcher does nothing until .done() is called"] pub struct Matcher<'a, Pos, Acc> { diff --git a/net/src/headers/view.rs b/net/src/headers/view.rs index cbde9522e1..516f60ff28 100644 --- a/net/src/headers/view.rs +++ b/net/src/headers/view.rs @@ -23,7 +23,7 @@ //! references to the matched layers via the [`Look`] trait without //! re-validating at each access site. For mutable access, a //! `&mut HeadersView` yields `&mut` references via [`LookMut::look_mut`], -//! which delegates to [`MatcherMut`](super::pat::MatcherMut) so the +//! which delegates to [`MatcherMut`] so the //! multi-`&mut` tuple is built from the same pre-split //! [`Fields`](super::pat::Fields) helper used by the rest of the //! matcher. @@ -32,26 +32,26 @@ //! //! Zero-cost extraction is achieved by informing the optimizer, via //! `Option::unwrap_unchecked`, that the `HeadersView` type invariant rules -//! out the `None` arms of each [`ViewStep::step`] call. The +//! out the `None` arms of each `ViewStep::step` call. The //! `unsafe` required for this is fully contained: //! //! * [`Headers::as_view`] / [`Headers::as_view_mut`] are the only //! ways to obtain a `&HeadersView` / `&mut HeadersView`, and they run -//! the [`sealed::Sealed::matches`] check -- which threads the same -//! cursors and gap checks as [`ViewStep::step`] would -- before +//! the `sealed::Sealed::matches` check -- which threads the same +//! cursors and gap checks as `ViewStep::step` would -- before //! the `#[repr(transparent)]` reference cast. -//! * [`ViewStep`] is crate-private. Its `step` method is a safe +//! * `ViewStep` is crate-private. Its `step` method is a safe //! `Option`-returning function; [`Look::look`] simply invokes it //! and unwraps unchecked, relying on the `HeadersView` newtype //! invariant. -//! * [`ViewStepMut`] is crate-private and mirrors `ViewStep` for +//! * `ViewStepMut` is crate-private and mirrors `ViewStep` for //! the mutable path, dispatching to -//! [`MatcherMut`](super::pat::MatcherMut) so aliasing of the +//! [`MatcherMut`] so aliasing of the //! returned `&mut` tuple is handled by the existing `Fields` //! pre-split. [`LookMut::look_mut`] unwraps the chain's //! `Option` unchecked under the same `HeadersView` invariant. //! * External callers see only [`HeadersView`], [`Look`], and [`LookMut`]. -//! They cannot implement [`ViewStep`] or [`ViewStepMut`] or call +//! They cannot implement `ViewStep` or `ViewStepMut` or call //! them directly, so they cannot forge a `HeadersView` that sidesteps //! `matches`. //! * `HeadersView` has private fields and no owning constructor; @@ -210,7 +210,7 @@ pub struct HeadersView(Headers, PhantomData); /// Declared, checkable shapes for [`HeadersView`]. /// /// Any tuple whose layers chain through the [`Within`] adjacency -/// graph and the [`ViewStep`] trait is a [`Shape`]. External +/// graph and the `ViewStep` trait is a [`Shape`]. External /// crates cannot add new shapes (they cannot implement `ViewStep`), /// but they can write any existing shape at the type level and let the /// trait bounds do the filtering. @@ -355,7 +355,7 @@ pub trait Look { /// Extract typed references to the matched layers. /// /// Compiles to the same sequence of field/variant reads as - /// [`sealed::Sealed::matches`], plus `unwrap_unchecked` at each + /// `sealed::Sealed::matches`, plus `unwrap_unchecked` at each /// step; the `HeadersView` type invariant guarantees success so the /// `None` branches are pruned by the optimizer. fn look<'a>(&'a self) -> Self::Refs<'a> @@ -367,7 +367,7 @@ pub trait Look { /// /// Yields a tuple of `&mut` references to the matched layers. Aliasing /// between the returned references is handled by -/// [`MatcherMut`](super::pat::MatcherMut)'s pre-split +/// [`MatcherMut`]'s pre-split /// [`Fields`](super::pat::Fields) -- `look_mut` delegates to a /// `MatcherMut` chain and unwraps the result unchecked, relying on the /// `HeadersView` shape invariant. @@ -379,7 +379,7 @@ pub trait LookMut { /// Extract typed `&mut` references to the matched layers. /// - /// Compiles to the same [`MatcherMut`](super::pat::MatcherMut) chain + /// Compiles to the same [`MatcherMut`] chain /// as the corresponding `Matcher` chain used by [`Look::look`], but /// mutable. The `HeadersView` type invariant guarantees the chain /// matches, so the final `.done()` is unwrapped unchecked and the diff --git a/net/src/headers/within.rs b/net/src/headers/within.rs index 68df2bddc2..2fc8646360 100644 --- a/net/src/headers/within.rs +++ b/net/src/headers/within.rs @@ -418,7 +418,7 @@ use crate::icmp6::TruncatedIcmp6; use crate::tcp::TruncatedTcp; use crate::udp::TruncatedUdp; -/// Marker type for the starting position of an [`EmbeddedMatcher`]. +/// Marker type for the starting position of an [`crate::headers::pat::EmbeddedMatcher`]. /// /// Embedded headers begin at the network layer (no Eth, no VLAN), so /// `Within` is implemented for Ipv4, Ipv6, and Net. diff --git a/net/src/ip_auth/v4.rs b/net/src/ip_auth/v4.rs index 5aad029619..db168e140a 100644 --- a/net/src/ip_auth/v4.rs +++ b/net/src/ip_auth/v4.rs @@ -3,7 +3,7 @@ //! IPv4-context IP Authentication Header. //! -//! This is a [`repr(transparent)`] newtype over [`IpAuth`] that marks the +//! This is a `repr(transparent)` newtype over [`IpAuth`] that marks the //! header as appearing in an IPv4 extension chain. The builder uses this //! type to restrict `Within` impls so that `Ipv4Auth` can only follow //! IPv4-legal parents (e.g. `Ipv4`), preventing it from being stacked diff --git a/net/src/ip_auth/v6.rs b/net/src/ip_auth/v6.rs index 08aa2ba9a9..a77e029192 100644 --- a/net/src/ip_auth/v6.rs +++ b/net/src/ip_auth/v6.rs @@ -3,7 +3,7 @@ //! IPv6-context IP Authentication Header. //! -//! This is a [`repr(transparent)`] newtype over [`IpAuth`] that marks the +//! This is a `repr(transparent)` newtype over [`IpAuth`] that marks the //! header as appearing in an IPv6 extension chain. The builder uses this //! type to restrict `Within` impls so that `Ipv6Auth` can only follow //! IPv6-legal parents (e.g. `Ipv6`, `Fragment`, `Routing`). diff --git a/net/src/ipv6/hop_by_hop.rs b/net/src/ipv6/hop_by_hop.rs index f52ab90fe4..ea091788db 100644 --- a/net/src/ipv6/hop_by_hop.rs +++ b/net/src/ipv6/hop_by_hop.rs @@ -6,7 +6,7 @@ //! The Hop-by-Hop Options header carries optional information that **must** be //! examined by every node along a packet's delivery path. Per [RFC 8200 section 4.1], //! it **must** immediately follow the IPv6 header when present -- the builder -//! enforces this via [`Within`] bounds. +//! enforces this via `Within` bounds. //! //! [RFC 8200 section 4.1]: https://datatracker.ietf.org/doc/html/rfc8200#section-4.1 //! [RFC 8200 section 4.3]: https://datatracker.ietf.org/doc/html/rfc8200#section-4.3 diff --git a/nix/profiles.nix b/nix/profiles.nix index db0e0b6502..3e0671a531 100644 --- a/nix/profiles.nix +++ b/nix/profiles.nix @@ -45,6 +45,9 @@ let # Register `emulated` so `#[cfg_attr(emulated, ...)]` never trips # `unexpected_cfgs`; only *set* for is-emulated-test and miri. "--check-cfg=cfg(emulated)" + # Only coverage lowers counter-heavy loop counts; sanitizers retain the + # iterations that help expose races. + "--check-cfg=cfg(instrumented)" "-Cdebuginfo=full" "-Cdwarf-version=5" "-Csymbol-mangling-version=v0" @@ -61,6 +64,7 @@ let ] ) ++ (if is-emulated-test then [ "--cfg=emulated" ] else [ ]) + ++ (if instrumentation == "coverage" then [ "--cfg=instrumented" ] else [ ]) ++ (map (flag: "-Clink-arg=${flag}") common.NIX_CFLAGS_LINK); optimize-for.debug.NIX_CFLAGS_COMPILE = [ "-fno-inline" diff --git a/routing/src/fib/test.rs b/routing/src/fib/test.rs index 7070eee4c4..bac97ec8ca 100644 --- a/routing/src/fib/test.rs +++ b/routing/src/fib/test.rs @@ -253,6 +253,9 @@ mod tests { const NUM_WORKERS: u16 = 6; const NUM_PACKETS: u64 = cfg_select! { emulated => 30, + // Coverage instruments every counter; the full count costs ~47s + // and reaches no additional lines. + instrumented => 2_000, _ => 100_000, }; const TENTH: u64 = NUM_PACKETS / 10;