build: make a DFNR-less build fail configure instead of passing quietly - #4495
#4495build: make a DFNR-less build fail configure instead of passing quietly#4495ten9876 wants to merge 1 commit intomainaethersdr/AetherSDR:mainfrom build/dfnr-enforce-defaultaethersdr/AetherSDR:build/dfnr-enforce-defaultCopy head branch name to clipboard
Conversation
ENABLE_DFNR defaults to ON, so a missing libdeepfilter meant CMake printed one STATUS line, set ENABLE_DFNR back OFF, and carried on. Every build after that succeeded cleanly with no further signal, and ENABLE_DFNR:BOOL=ON stayed in CMakeCache.txt the whole time — so the cache flag looked correct while the feature was absent. Local builds silently diverged from what CI ships, and the only way to notice was to check `available` under DFNR at runtime. AGENTS.md already said to run setup-deepfilter.sh before cmake. It was framed as optional, and a documented instruction is not an enforcement point (Principle XII): the miss survived a full end-to-end read of that file. Make it a FATAL_ERROR naming the fix, the opt-out, and the fact that a plain `cmake --build` cannot recover because detection is configure-time. Building without DFNR stays supported via -DENABLE_DFNR=OFF — it just has to be said out loud rather than happening by omission. check-system-libs-linux never ran the setup step, so it was building DFNR-less by accident; it now passes -DENABLE_DFNR=OFF explicitly, alongside the -DENABLE_RADE=OFF already in that same invocation. AGENTS.md: the setup script moves into the build recipe as step 1 rather than sitting below it as an optional aside, the prose is rewritten for the new hard-fail, and -j$(nproc) is annotated with the macOS equivalent (it does not exist there, and this file is read by agents on all four platforms). Verified: configures with the library present; configures with -DENABLE_DFNR=OFF; fails with exit 1 when DFNR is requested and the library is absent. Note: this is a breaking change for any checkout without third_party/deepfilter/lib/<platform>/ — such a build previously configured quietly and will now stop. That is the intent, but expect reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
The CMake half is the right shape and I like the framing — a STATUS line is not an enforcement point, and ENABLE_DFNR:BOOL=ON in the cache looking correct precisely when the feature is missing is a genuinely nasty failure mode. The error text is unusually good: it names the fix, the opt-out, and the configure-time-not-build-time gotcha, which is the part people actually get stuck on.
The ci.yml hunk, though, rests on a premise that doesn't hold: third_party/deepfilter/lib/linux-x86_64/libdeepfilter.a is tracked in the repo (52 MB, in-tree since 6af248b / #4279), so every x86_64 Linux job already finds the library from a bare checkout. check-system-libs-linux was building with DFNR, not DFNR-less — so there's no hard-fail to pre-empt, and -DENABLE_DFNR=OFF just removes DFNR from the sid canary's compile surface. Details inline. I also checked the rest of CI: nothing else breaks, but only because of that same committed lib.
Would like fixed before merge
ci.yml—-DENABLE_DFNR=OFFis unnecessary (the lib is in-tree) and costs the sid canary its DFNR compile coverage; suggest dropping the hunk.
Non-blocking notes
codeql.yml:74andsanitizers.yml:63also configure with no setup step and survive only on the committed x86_64 lib — worth a comment somewhere, or they become the next silent break if that lib is ever un-vendored (it's the one thing standing between thisFATAL_ERRORand two red required checks).AGENTS.md— "Configure fails if you skip it" isn't true on linux-x86_64; one clause would keep it accurate for the platform most Linux contributors are on.- Breaking-change call-out and the release-note suggestion are right; agreed the Tier 1
AGENTS.mdhunk needs an admin override or a separate commit. Verified the setup script is genuinely idempotent (early exit when lib + model are both present), so "run it whenever you're unsure" is safe advice.
🤖 aethersdr-agent · cost: $4.3224 · model: claude-opus-5
| - name: Configure | ||
| run: | | ||
| cmake -B build -DUSE_SYSTEM_ZLIB=ON -DUSE_SYSTEM_MSPACK=ON -DUSE_SYSTEM_LIBMOSQUITTO=ON -DUSE_SYSTEM_RTMIDI=ON -DUSE_SYSTEM_LIQUID_DSP=ON -DENABLE_RADE=OFF | ||
| cmake -B build -DUSE_SYSTEM_ZLIB=ON -DUSE_SYSTEM_MSPACK=ON -DUSE_SYSTEM_LIBMOSQUITTO=ON -DUSE_SYSTEM_RTMIDI=ON -DUSE_SYSTEM_LIQUID_DSP=ON -DENABLE_RADE=OFF -DENABLE_DFNR=OFF |
There was a problem hiding this comment.
This hunk's rationale doesn't hold up. third_party/deepfilter/lib/linux-x86_64/libdeepfilter.a is tracked in the repo:
$ git ls-files third_party/deepfilter/lib
third_party/deepfilter/lib/linux-x86_64/libdeepfilter.a
So check-system-libs-linux (debian:sid, x86_64) resolves DFNR_LIB to a file that exists straight out of actions/checkout — it has been building with DFNR all along, not "DFNR-less by accident." There is no new hard-fail here to pre-empt.
What the flag does do is take DFNR out of the sid canary's compile surface — and that job's whole purpose is to catch newest-system-library breakage early, on the one configuration where DFNR is free to test. -DENABLE_RADE=OFF next to it is load-bearing (RADE pulls deps sid doesn't have); this one isn't.
| cmake -B build -DUSE_SYSTEM_ZLIB=ON -DUSE_SYSTEM_MSPACK=ON -DUSE_SYSTEM_LIBMOSQUITTO=ON -DUSE_SYSTEM_RTMIDI=ON -DUSE_SYSTEM_LIQUID_DSP=ON -DENABLE_RADE=OFF -DENABLE_DFNR=OFF | |
| cmake -B build -DUSE_SYSTEM_ZLIB=ON -DUSE_SYSTEM_MSPACK=ON -DUSE_SYSTEM_LIBMOSQUITTO=ON -DUSE_SYSTEM_RTMIDI=ON -DUSE_SYSTEM_LIQUID_DSP=ON -DENABLE_RADE=OFF |
Same fact means the PR body's "all three release workflows run the setup step, so shipped binaries have always included DFNR" is true but under-sells it: x86_64 Linux gets DFNR from the checkout regardless. The real exposure is macOS/Windows/linux-aarch64 hand builds — which is exactly your case, and the CMake change still fixes it.
| script is a no-op once the library is present, so run it whenever you're | ||
| unsure. | ||
|
|
||
| **Configure fails if you skip it.** `ENABLE_DFNR` defaults to `ON`, so a |
There was a problem hiding this comment.
Accurate on macOS, Windows, and linux-aarch64; not on linux-x86_64, where third_party/deepfilter/lib/linux-x86_64/libdeepfilter.a is committed to the repo and configure succeeds without the script ever running. A contributor on Ubuntu who reads this, skips the script anyway, and sees a clean configure learns the doc overstates — which is the same erosion the old "Optional" framing caused, pointed the other way.
One clause covers it, e.g. "…defaults to ON, so a missing library is now a FATAL_ERROR. (On linux-x86_64 the library is vendored in-tree, so the script is already satisfied there — every other platform must fetch it.)"
|
Antares Detector — candidate vulnerable file(s)
Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting. 🤖 aethersdr-agent · cost: $4.7136 · model: claude-opus-5 |
The problem
ENABLE_DFNRdefaults toON. Whenlibdeepfilteris absent, CMake printed oneSTATUSline, setENABLE_DFNRback toOFF, and continued — so every build afterwards succeeded cleanly with no further signal, whileENABLE_DFNR:BOOL=ONstayed inCMakeCache.txtthe whole time. The cache flag looks correct precisely when the feature is missing.The result: local builds silently diverge from what CI ships. All three release workflows (
macos-dmg,appimage,windows-installer) run the setup step, so shipped binaries have always included DFNR — this only ever bit hand builds. Which is the bad half, since it means testing a change against a binary users don't run.The only reliable way to notice was checking
availableunderDFNRinget_state model=dspat runtime.Why documentation wasn't the fix
AGENTS.mdalready said to runsetup-deepfilter.shbeforecmake. It was framed as Optional, sat below the recipe block rather than in it, and the miss survived a full end-to-end read of that file — four consecutive DFNR-less builds followed. A documented instruction is not an enforcement point (Principle XII); the check has to live somewhere you cannot read past.The change
CMakeLists.txt— missing library is aFATAL_ERRORnaming the fix, the opt-out, and the fact that a plaincmake --buildcannot recover (detection is configure-time). Building without DFNR stays supported via-DENABLE_DFNR=OFF; it just has to be explicit rather than accidental.ci.yml—check-system-libs-linuxnever ran the setup step, so it was building DFNR-less by accident. It now passes-DENABLE_DFNR=OFFexplicitly, next to the-DENABLE_RADE=OFFalready in that invocation. (It'scontinue-on-error: true, a Debian sid canary, so it was never a gate either way.)AGENTS.md— setup script moves into the recipe as step 1; prose rewritten for the hard-fail;-j$(nproc)annotated with the macOS equivalent, which doesn't exist there and this file is read by agents on all four platforms.The error a contributor now sees:
Test plan
exit=0,DFNR (DeepFilterNet3) enabled — .../libdeepfilter.a-DENABLE_DFNR=OFF→ configures,exit=0, DFNR absentexit=1with the message aboveget_state model=dspreportsDFNR available: trueAny checkout without
third_party/deepfilter/lib/<platform>/previously configured quietly and will now stop. That is the intent, but it will surprise people mid-work — worth a release-note line, and expect a couple of "my build broke" reports. The fix is one command and the error says so.Reviewer note
This touches
AGENTS.md, which is Tier 1 (@aethersdr/maintainers= @ten9876 only) — and @ten9876 authored it, so GitHub will block self-approval and no other CODEOWNER can satisfy that path. It needs an admin override, or theAGENTS.mdhunk split out and committed separately. TheCMakeLists.txt+ci.ymlhalf is Tier 2 and reviewable normally.Checklist
AppSettingscalls (Principle V) — n/aMeterSmoother— n/aAGENTS.mdupdated in this PR🤖 Generated with Claude Code