From 53b172c101ee9be5a047a07e7476a831c7ca3a6d Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 19 Aug 2026 20:28:02 -0600 Subject: [PATCH 01/20] perf(nix): wrap the outputs that will be kept out of the shared cache Cachix pays for itself only on outputs that are slow to build and reusable across revisions. Per-revision workspace and image outputs are neither, so later commits mark them volatile and skip their substitute lookups. The wrapping is separated from the decisions that use it because nixfmt reindents an entire binding when its expression is parenthesised, so folding these eight sites into the commits that need them buries roughly 630 lines of pure reindentation in four otherwise small diffs. `source-volatile` is introduced and applied here in one place; every commit that follows is legible without `git show -w`. Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 626 +++++++++++++++++++++++++++------------------------- 1 file changed, 323 insertions(+), 303 deletions(-) diff --git a/default.nix b/default.nix index 19dffb5037..6a8e13a784 100644 --- a/default.nix +++ b/default.nix @@ -259,6 +259,10 @@ let "sha256-XwEKLdc2Y7fteSKKOERgjKTdxELy7K/wOVuB/SSj3ng="; }; }; + 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 +281,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 +298,8 @@ let done | $JQ --sort-keys --slurp 'add' > $out '' ) + ).overrideAttrs + source-volatile ) ); version = (craneLib.crateNameFromCargoToml { inherit src; }).version; @@ -663,178 +669,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 +897,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 { From d5487329832ef395532b289b26b07f94b4898acf Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 22:20:52 -0600 Subject: [PATCH 02/20] ci: reserve Cachix for reusable build inputs Per-revision workspace and image outputs consume cache transfer and storage while rarely substituting, crowding out slower native dependencies that are reusable. Mark source-volatile outputs consistently, skip their substitute lookups, and exclude OCI assembly paths that could pull them back through a closure. Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/actions/nix-shell/action.yml | 2 + default.nix | 78 +++++++++++++++------------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/.github/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 2336aabfca..7d761eb9ac 100644 --- a/.github/actions/nix-shell/action.yml +++ b/.github/actions/nix-shell/action.yml @@ -32,6 +32,8 @@ runs: signingKey: '${{ inputs.cachix_signing_key }}' # prettier-ignore authToken: '${{ inputs.cachix_auth_token }}' + # Exclude source-volatile builds and image paths that can reintroduce them through a closure. + pushFilter: '(-dataplane-volatile-|-dataplane-(customisation-layer|conf\.json)$|-stream-(dataplane|frr)$|-(frr-conf\.json|layers\.json|excludePaths)$)' - name: "use nix shell" uses: "rrbutani/use-nix-shell-action@59a52b2b9bbfe3cc0e7deb8f9059abe37a439edf" # v1.1.0 diff --git a/default.nix b/default.nix index 6a8e13a784..d1b39a9927 100644 --- a/default.nix +++ b/default.nix @@ -415,44 +415,48 @@ let } // args )).overrideAttrs - (orig: { - separateDebugInfo = true; + ( + orig: + source-volatile orig + // { + 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 - '' - 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 - ''; - }); + # 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 + ''; + } + ); workspace-builder = { pname ? null, From 3b67b2acf6b930dcad138e1353487fcaccecb9c4 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 08:30:14 -0600 Subject: [PATCH 03/20] ci: keep workspace outputs substitutable Workspace derivations remain identical across workflow-only edits, reruns, merge-queue runs, and post-merge pushes. Marking them source-volatile forced those runs to rebuild the workspace despite unchanged inputs. Keep the marker on per-revision image assembly, but let identical workspace builds substitute. Different flags and sysroots still produce distinct store paths. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 81 ++++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/default.nix b/default.nix index d1b39a9927..715d1b86f0 100644 --- a/default.nix +++ b/default.nix @@ -259,6 +259,9 @@ 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; @@ -415,48 +418,44 @@ let } // args )).overrideAttrs - ( - orig: - source-volatile orig - // { - separateDebugInfo = true; + (orig: { + 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 - '' - 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 - ''; - } - ); + # 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 + ''; + }); workspace-builder = { pname ? null, From 6b7c3a1da1b8a419f9249a64b7b4933448588731 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 08:56:40 -0600 Subject: [PATCH 04/20] fix(nix): keep `results` out of the build source `just build` creates gitignored out-links under `results`, but `lib.cleanSource` does not honor gitignore. Their store-path targets gave local builds a source hash different from CI. Exclude the directory so a previous local build cannot prevent reuse of otherwise identical cached outputs. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index 715d1b86f0..7ea4bee7b1 100644 --- a/default.nix +++ b/default.nix @@ -216,7 +216,11 @@ let 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"); + # `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"); src = pkgs.lib.cleanSourceWith { filter = full-path: t: From 7666f924624ba6a4c4544065a46dbb14f4820c12 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 09:13:03 -0600 Subject: [PATCH 05/20] perf(nix): share one dependency build per flag-set Every crane derivation opted out of dependency artifacts, so each package rebuilt hundreds of third-party crates and the standard library whenever workspace source changed. Build shared production and test dependency artifacts instead. Separate flag sets preserve Cargo fingerprints, while the platform-aware package list avoids pulling excluded, WASI-incompatible dependencies into cross builds. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 152 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 110 insertions(+), 42 deletions(-) diff --git a/default.nix b/default.nix index 7ea4bee7b1..a22b300986 100644 --- a/default.nix +++ b/default.nix @@ -351,6 +351,10 @@ let pname = null; cargoArtifacts = null; }, + # A deps-only build produces cargo artifacts rather than binaries, so it + # skips the debug-info split and keeps the `target.tar.zst` that the + # package path strips. + for-deps ? false, profile, cargo-nextest, hwloc, @@ -422,48 +426,112 @@ let } // args )).overrideAttrs - (orig: { - separateDebugInfo = true; + ( + orig: + if for-deps then + { + 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; @@ -496,7 +564,7 @@ let workspace-check = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -529,7 +597,7 @@ let test-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -572,7 +640,7 @@ let bench-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -615,7 +683,7 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" @@ -651,7 +719,7 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts; RUSTDOCFLAGS = "-D warnings"; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ From 7424efd85ebdb7ace8fe21f357c58bab0ada37cc Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 12:18:52 -0600 Subject: [PATCH 06/20] perf(nix): remap sources to a relative prefix Embedding the source store path in RUSTFLAGS changed the compilation identity of every dependency whenever workspace source changed, defeating the shared dependency build. Use a stable relative prefix instead. The Bolero fix makes an absolute path unnecessary and avoids the global source symlink that raced between worktrees. Resolve that prefix against the worktree for coverage and filter reports to workspace sources; otherwise llvm-cov silently includes the standard library and native dependencies. Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 21 ++++++++------------- justfile | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/default.nix b/default.nix index a22b300986..ab6f863fdf 100644 --- a/default.nix +++ b/default.nix @@ -235,6 +235,10 @@ let 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 @@ -406,18 +410,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 @@ -622,7 +617,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 ""); }; }; diff --git a/justfile b/justfile index 93227c9f26..34bec9022a 100644 --- a/justfile +++ b/justfile @@ -538,6 +538,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 +593,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" From 21604fb67497752979a586a24da865781c605d93 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 11:47:58 -0600 Subject: [PATCH 07/20] ci: widen matrix parallelism for deep runs A queued merge blocks everything behind it, so deep runs should finish faster even when that temporarily uses more of the shared lab pool. Run merge-queue and push matrices four entries at a time while pull requests remain serial and cannot crowd the queue out. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 0afab3591d..4cced53540 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,8 +197,8 @@ 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) }}" steps: @@ -365,7 +379,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 +413,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: sanitizer: - thread @@ -444,7 +458,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 +646,7 @@ jobs: JUST_VARS: "" strategy: fail-fast: false + # Keep cross serial so one pull request cannot occupy the lab. max-parallel: 1 matrix: platform: From 376472ecca02639e0e9f4cc38e581b8cb4cc8aad Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 13:50:26 -0600 Subject: [PATCH 08/20] ci: honour ci:-vlab again The ci:-vlab label remained in use after VLAB became opt-in, but the workflow no longer read it. That left no way to request all deep checks except the lab matrix. Honor the subtractive label for the entire VLAB matrix, including merge-ready runs, just as ci:-upgrade is honored. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 4cced53540..5606e3ca6f 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -714,6 +714,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. @@ -721,7 +722,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') From 5d693b072169876ef8346b476ea40a660b095408 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 13:12:49 -0600 Subject: [PATCH 09/20] ci: retry container pushes Transient registry errors occasionally discard a push after the expensive build has completed. Let skopeo retry individual blobs, then retry whole idempotent skopeo and oras pushes only for known recoverable transport and status errors. Stream output and announce retries so degradation remains visible. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- justfile | 68 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/justfile b/justfile index 34bec9022a..cb302ce526 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 }}" ;; From 21ed20ae0105648d71aa8888a49ac1936c7180c9 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:04:38 -0600 Subject: [PATCH 10/20] perf(nix): keep the git version out of the dependency build The git-derived VERSION changed the shared dependency derivation on every commit even though third-party crates and the standard library do not consume it. Use a constant version for dependency builds while preserving the real value for workspace consumers, keeping artifacts reusable across revisions. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index ab6f863fdf..5cecb9d316 100644 --- a/default.nix +++ b/default.nix @@ -393,7 +393,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"; From 76fce622056f0d2cdd15b70d08d0c9d4bba22539 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:07:42 -0600 Subject: [PATCH 11/20] ci: check that the dependency build stays reusable The shared dependency build is only worth having if it is actually reused. Twice now something per-commit has leaked into it -- once the workspace source path, once the git version string -- and both times the build still succeeded. The only symptom was a slow cache miss, which nobody notices. Add a CI check for each leak, using the cheapest method that can see it. The workspace source is a store path, so "does the dependency build depend on it?" is a question about the derivation graph. Instantiate the derivation once and look at its inputs. That answer is exact, and it names the offending path and derivation rather than reporting only that some hash moved. It also avoids the alternative, which is to edit a tracked file, run the build, and restore the file from a shell trap. The git version is a different shape of problem: it reaches the derivation as an environment variable and never as an input path, so no graph walk can see it. That one needs two instantiations and a comparison. Both flag sets, production and test, are checked independently. `src` is exported so the graph question can be asked from outside. The check reports through the function's exit status, so a detected mismatch cannot be swallowed by a command-substitution subshell. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 9 ++++ default.nix | 1 + justfile | 88 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 5606e3ca6f..3bd0da2563 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -333,6 +333,14 @@ jobs: with: recipe: "markdownlint" + # 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 @@ -351,6 +359,7 @@ jobs: pinact=${{ steps.pinact.outcome }} actionlint=${{ steps.actionlint.outcome }} markdownlint=${{ steps.markdownlint.outcome }} + check-deps-reuse=${{ steps.check-deps-reuse.outcome }} license-headers=${{ steps.license-headers.outcome }} run: | set -euo pipefail diff --git a/default.nix b/default.nix index 5cecb9d316..b708093367 100644 --- a/default.nix +++ b/default.nix @@ -1133,6 +1133,7 @@ in package-list pkgs sources + src sysroot tests workspace diff --git a/justfile b/justfile index cb302ce526..144328ed01 100644 --- a/justfile +++ b/justfile @@ -441,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_ }} From 528c9dfd2e0e07b1a89df87be22671fcc9ed3db2 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:56:12 -0600 Subject: [PATCH 12/20] ci: build clippy, doctests, and docs through nix Clippy and doctests bypassed Nix and rebuilt dependencies in empty runner workspaces, while the existing documentation derivation was unused and had silently rotted. Run all three through Nix so they share dependency artifacts and match local CI entry points. Keep clippy on all targets, execute doctests in the sandbox, and supply rustdoc with the same cfg declarations as rustc. Activating those paths exposed broken documentation links, an unpatched test-runner shebang, and source filters whose escaped regexes matched unrelated files; repair those prerequisites as part of making the checks real. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 7 +++ acl/src/dpdk/install.rs | 2 +- acl/src/lib.rs | 4 +- ci.just | 3 ++ concurrency/src/slot.rs | 2 +- concurrency/src/stress.rs | 2 +- concurrency/src/thread/mod.rs | 2 +- config/src/external/overlay/acl.rs | 2 +- default.nix | 79 +++++++++++++++++++++++++----- development/code/running-tests.md | 20 ++++++++ justfile | 12 ++--- lifecycle/src/lib.rs | 2 +- net/src/headers/embedded_view.rs | 8 +-- net/src/headers/mod.rs | 2 +- net/src/headers/pat.rs | 2 +- net/src/headers/view.rs | 24 ++++----- net/src/headers/within.rs | 2 +- net/src/ip_auth/v4.rs | 2 +- net/src/ip_auth/v6.rs | 2 +- net/src/ipv6/hop_by_hop.rs | 2 +- 20 files changed, 132 insertions(+), 49 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 3bd0da2563..9d492eca7a 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -233,6 +233,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: | 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..64ab331b59 100644 --- a/ci.just +++ b/ci.just @@ -37,6 +37,9 @@ 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 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 b708093367..931b60fbcb 100644 --- a/default.nix +++ b/default.nix @@ -212,10 +212,13 @@ 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; + # Nix escaping made the old regexes match unrelated .sh and .patch files. + justfileFilter = p: _type: lib.hasSuffix ".justfile" p; + markdownFilter = p: _type: lib.hasSuffix ".md" p; + jsonFilter = p: _type: lib.hasSuffix ".json" p; + cHeaderFilter = p: _type: lib.hasSuffix ".h" p; + # `.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 = @@ -231,6 +234,7 @@ let || (markdownFilter p t) || (jsonFilter p t) || (cHeaderFilter p t) + || (shellFilter p t) || ((outputsFilter p t) && (craneLib.filterCargoSources full-path t)); src = lib.cleanSource ./.; name = "source"; @@ -355,10 +359,10 @@ let pname = null; cargoArtifacts = null; }, - # A deps-only build produces cargo artifacts rather than binaries, so it - # skips the debug-info split and keeps the `target.tar.zst` that the - # package path strips. + # 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, @@ -425,8 +429,10 @@ let )).overrideAttrs ( orig: - if for-deps then + if for-deps || no-bins then { + # Only dependency builds should retain crane's target archive. + doInstallCargoArtifacts = for-deps; postBuild = (orig.postBuild or "") + '' unset RUSTFLAGS; ''; @@ -671,24 +677,32 @@ let benches = bench-builder { }; + # `--all-targets` so tests, benches, and examples are linted too. That code + # is as load bearing as the rest and deserves the same static analysis, and + # the bare `cargo clippy` this replaces already covered it. + # + # Linting test targets means compiling them, so this takes the unwind flavour + # of `-Zbuild-std` and the test profile, matching how the tests themselves + # are built. It shares `cargo-artifacts-tests` for the same reason. clippy-builder = { pname ? null, }: pkgs.callPackage invoke { builder = craneLib.mkCargoDerivation; - profile = profile'; + profile = profile-tests'; args = { inherit pname; - cargoArtifacts = cargo-artifacts; + cargoArtifacts = cargo-artifacts-tests; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" "clippy" + "--all-targets" "--profile=${cargo-profile}" "--package=${pname}" ] - ++ cargo-cmd-prefix + ++ cargo-cmd-prefix-tests ++ [ "--" "-D warnings" @@ -704,6 +718,45 @@ let } ) 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 = { package ? null, @@ -717,7 +770,8 @@ let args = { inherit pname; cargoArtifacts = cargo-artifacts; - RUSTDOCFLAGS = "-D warnings"; + # Rustdoc does not inherit rustc's registered cfg declarations. + RUSTDOCFLAGS = "-D warnings --check-cfg=cfg(emulated) --check-cfg=cfg(instrumented)"; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" @@ -1129,6 +1183,7 @@ in dataplane devenv devroot + doctests docs package-list pkgs 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 144328ed01..f748351405 100644 --- a/justfile +++ b/justfile @@ -544,10 +544,9 @@ 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" } else { "clippy." + package }) args) {{ _just_debuggable_ }} - cargo clippy --all-targets {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} -- -D warnings [script] actionlint: @@ -610,11 +609,10 @@ lint: \ (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. 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/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 From c74f7fb1876c9317a2649a25a017cb3784a71306 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 15:16:24 -0600 Subject: [PATCH 13/20] perf(nix): lint the workspace in one derivation Forty-one per-package clippy derivations all invalidated together because they shared the workspace source, yet each paid the fixed cost of unpacking dependency artifacts. Lint the platform-aware workspace package list in one derivation while retaining per-package targets for focused local use. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 34 +++++++++++++++++++--------------- justfile | 2 +- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/default.nix b/default.nix index 931b60fbcb..a9dc93b9d9 100644 --- a/default.nix +++ b/default.nix @@ -677,17 +677,15 @@ let benches = bench-builder { }; - # `--all-targets` so tests, benches, and examples are linted too. That code - # is as load bearing as the rest and deserves the same static analysis, and - # the bare `cargo clippy` this replaces already covered it. - # - # Linting test targets means compiling them, so this takes the unwind flavour - # of `-Zbuild-std` and the test profile, matching how the tests themselves - # are built. It shares `cargo-artifacts-tests` for the same reason. + # 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-tests'; @@ -700,8 +698,14 @@ let "clippy" "--all-targets" "--profile=${cargo-profile}" - "--package=${pname}" ] + # 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 ++ [ "--" @@ -711,12 +715,12 @@ 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. diff --git a/justfile b/justfile index f748351405..18d6ae2a96 100644 --- a/justfile +++ b/justfile @@ -545,7 +545,7 @@ zizmor *args="": zizmor --persona=pedantic {{args}} . # Run the CI-equivalent cached lint; direct Cargo remains the fast inner loop. -clippy package="" *args: (build (if package == "" { "clippy" } else { "clippy." + package }) args) +clippy package="" *args: (build (if package == "" { "clippy.all" } else { "clippy.pkg." + package }) args) {{ _just_debuggable_ }} [script] From c37b3e311aab2e357c424c2eb2cc84c64bb397f6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 17:31:48 -0600 Subject: [PATCH 14/20] ci: give nix the whole core budget, and split it for test_each The lab cgroup provides ten cores, but jobs used only eight because containers cannot discover that limit reliably. Give large derivations all ten cores. Split test_each into two five-core jobs because its many small package derivations cannot saturate the budget serially. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- ci.just | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ci.just b/ci.just index 64ab331b59..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 + _lab-common [private] -_lab := "jobs=" + jobs + " cores=" + cores + " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile +_lab-test-each := "jobs=" + test_each_jobs + " cores=" + test_each_cores + _lab-common [default] [private] @@ -44,7 +56,7 @@ 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 From 8323c5905de78e2f12257a8ec2d099717967f3c1 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 17:38:59 -0600 Subject: [PATCH 15/20] ci: cut the coverage test floor and stop checking the fuzz profile twice Coverage instrumentation made one counter-heavy concurrency test dominate the suite even though more iterations reached no additional lines. The fuzz check also repeated optimized compilation already covered elsewhere. Reduce that test only under coverage instrumentation and drop fuzz from the ordinary check matrix; sanitizers and fuzz-specific jobs retain the heavier exercise. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .cargo/config.toml | 4 ++-- .github/workflows/dev.yml | 4 ++++ miri.just | 2 +- nix/profiles.nix | 4 ++++ routing/src/fib/test.rs | 3 +++ 5 files changed, 14 insertions(+), 3 deletions(-) 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/workflows/dev.yml b/.github/workflows/dev.yml index 9d492eca7a..397d499b38 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -201,6 +201,10 @@ jobs: 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 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/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; From 23eeee12ca775949c78cd6cbde3652b1d3442ed6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 18:18:40 -0600 Subject: [PATCH 16/20] docs(ci): document the ci:-vlab label The workflow honors ci:-vlab, but its README listed ci:-upgrade as the only subtractive label. Document the available opt-out. Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 From 9e1c7954a1f6e692091819a49ac03b4c25235a4a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 16 Aug 2026 21:19:46 -0600 Subject: [PATCH 17/20] ci: check that default.nix stays formatted The root formatting recipe covers Rust but not Nix, allowing default.nix to drift unnoticed during this stack. Format it and add a focused nixfmt check. Older unformatted files under nix remain outside the check to avoid unrelated churn. Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 1 + justfile | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/default.nix b/default.nix index a9dc93b9d9..ba7df759a8 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 diff --git a/justfile b/justfile index 18d6ae2a96..a7e473e1dc 100644 --- a/justfile +++ b/justfile @@ -553,6 +553,12 @@ actionlint: {{ _just_debuggable_ }} actionlint +# Keep default.nix formatted without adopting legacy files under nix/. +[script] +nixfmt *args="--check": + {{ _just_debuggable_ }} + nixfmt {{ args }} default.nix + # Limit linting to tracked Markdown so generated files cannot affect CI. [script] markdownlint *args: @@ -606,6 +612,7 @@ lint: \ (pinact "--fix=false" "--no-api") \ (actionlint) \ (markdownlint) \ + (nixfmt) \ (license-headers) {{ _just_debuggable_ }} From 23ea7cd8fa891b6a010d2dc7dab525445adc5b76 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 16 Aug 2026 21:56:27 -0600 Subject: [PATCH 18/20] perf(nix): keep prose and dev config out of the build source The source filter admitted all Markdown and JSON, so editing prose or a gitignored editor configuration changed every workspace derivation even though builds read none of it. Allow only Markdown included by crate documentation and remove unused JSON and Just filters. Omitting a real include now fails loudly at compile time, while unrelated files no longer defeat substitution. Co-authored-by: Codex Signed-off-by: Daniel Noland --- default.nix | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/default.nix b/default.nix index ba7df759a8..301dba5408 100644 --- a/default.nix +++ b/default.nix @@ -214,10 +214,19 @@ let }; }; # Nix escaping made the old regexes match unrelated .sh and .patch files. - justfileFilter = p: _type: lib.hasSuffix ".justfile" p; - markdownFilter = p: _type: lib.hasSuffix ".md" p; - jsonFilter = p: _type: lib.hasSuffix ".json" p; 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 @@ -225,15 +234,17 @@ let 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)); From 368146d83eb7493087bf3a4904785b0ec6547ef3 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 16 Aug 2026 22:15:55 -0600 Subject: [PATCH 19/20] ci: actually run nixfmt, and check that the lint lists agree Adding nixfmt to `just lint` did not add it to the workflow, whose lint steps and failure aggregation are maintained separately. Either list can drift silently and leave a check unenforced. Run nixfmt in CI and add a guard that keeps the recipe dependencies, workflow steps, and aggregated outcomes aligned. Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 16 ++++++++++++++++ justfile | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 397d499b38..1038981bb4 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -344,6 +344,20 @@ 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" + # Cache misses still pass, so guard dependency reuse explicitly. - name: "check-deps-reuse" id: "check-deps-reuse" @@ -370,6 +384,8 @@ 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-deps-reuse=${{ steps.check-deps-reuse.outcome }} license-headers=${{ steps.license-headers.outcome }} run: | diff --git a/justfile b/justfile index a7e473e1dc..75e01b9fd8 100644 --- a/justfile +++ b/justfile @@ -559,6 +559,37 @@ 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" + # Limit linting to tracked Markdown so generated files cannot affect CI. [script] markdownlint *args: @@ -613,6 +644,7 @@ lint: \ (actionlint) \ (markdownlint) \ (nixfmt) \ + (check-lint-wiring) \ (license-headers) {{ _just_debuggable_ }} From 96561e19dcf2bd1eb0731e87421ab4e1c2af9c74 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 16 Aug 2026 22:20:07 -0600 Subject: [PATCH 20/20] ci: keep every container image out of the shared cache Several container derivations lacked the source-volatile marker, allowing per-revision images and dockerTools assembly paths to reach Cachix. Mark every image and check their realized closures against the actual push filter. Match dockerTools artifacts by shape so newly added or nested images are covered without another name list. Keep the current denylist self-checking; converting cache uploads to a stricter allowlist remains a separate behavioral change. Co-authored-by: Codex Signed-off-by: Daniel Noland --- .github/actions/nix-shell/action.yml | 8 ++- .github/workflows/dev.yml | 8 +++ justfile | 82 ++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/.github/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 7d761eb9ac..6dee11c64a 100644 --- a/.github/actions/nix-shell/action.yml +++ b/.github/actions/nix-shell/action.yml @@ -32,8 +32,12 @@ runs: signingKey: '${{ inputs.cachix_signing_key }}' # prettier-ignore authToken: '${{ inputs.cachix_auth_token }}' - # Exclude source-volatile builds and image paths that can reintroduce them through a closure. - pushFilter: '(-dataplane-volatile-|-dataplane-(customisation-layer|conf\.json)$|-stream-(dataplane|frr)$|-(frr-conf\.json|layers\.json|excludePaths)$)' + # 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/dev.yml b/.github/workflows/dev.yml index 1038981bb4..5cb2a33d5c 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -358,6 +358,13 @@ jobs: 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" @@ -386,6 +393,7 @@ jobs: 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: | diff --git a/justfile b/justfile index 75e01b9fd8..6eed433ff8 100644 --- a/justfile +++ b/justfile @@ -590,6 +590,87 @@ check-lint-wiring: 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: @@ -645,6 +726,7 @@ lint: \ (markdownlint) \ (nixfmt) \ (check-lint-wiring) \ + (check-push-filter) \ (license-headers) {{ _just_debuggable_ }}