Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

build: make a DFNR-less build fail configure instead of passing quietly - #4495

#4495
Open
ten9876 wants to merge 1 commit into
mainaethersdr/AetherSDR:mainfrom
build/dfnr-enforce-defaultaethersdr/AetherSDR:build/dfnr-enforce-defaultCopy head branch name to clipboard
Open

build: make a DFNR-less build fail configure instead of passing quietly#4495
ten9876 wants to merge 1 commit into
mainaethersdr/AetherSDR:mainfrom
build/dfnr-enforce-defaultaethersdr/AetherSDR:build/dfnr-enforce-defaultCopy head branch name to clipboard

Conversation

@ten9876

@ten9876 ten9876 commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

The problem

ENABLE_DFNR defaults to ON. When libdeepfilter is absent, CMake printed one STATUS line, set ENABLE_DFNR back to OFF, and continued — so every build afterwards succeeded cleanly with no further signal, while ENABLE_DFNR:BOOL=ON stayed in CMakeCache.txt the 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 available under DFNR in get_state model=dsp at runtime.

Why documentation wasn't the fix

AGENTS.md already said to run setup-deepfilter.sh before cmake. 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 a FATAL_ERROR naming the fix, the opt-out, and the fact that a plain cmake --build cannot recover (detection is configure-time). Building without DFNR stays supported via -DENABLE_DFNR=OFF; it just has to be explicit rather than accidental.
  • ci.ymlcheck-system-libs-linux never ran the setup step, so it was building DFNR-less by accident. It now passes -DENABLE_DFNR=OFF explicitly, next to the -DENABLE_RADE=OFF already in that invocation. (It's continue-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:

CMake Error at CMakeLists.txt:490 (message):
  DFNR (DeepFilterNet3) was requested but its library is missing:

      .../third_party/deepfilter/lib/darwin-arm64/libdeepfilter.a
    Fix:     run ./scripts/setup/setup-deepfilter.sh (Windows: scripts\setup\setup-deepfilter.ps1), then re-run cmake
    Opt out: configure with -DENABLE_DFNR=OFF

  A plain 'cmake --build' will NOT pick up a newly fetched library — this
  check runs at configure time.

Test plan

  • Local build passes
    • Library present → configures, exit=0, DFNR (DeepFilterNet3) enabled — .../libdeepfilter.a
    • -DENABLE_DFNR=OFF → configures, exit=0, DFNR absent
    • Library stashed away, DFNR requested → exit=1 with the message above
    • Full build with DFNR on: 665 targets, 0 errors, 0 warnings; runtime get_state model=dsp reports DFNR available: true
  • Behavior verified on a real radio if applicable — n/a, build-system change; DFNR availability confirmed on a FLEX-8600 via the automation bridge
  • Existing tests pass (CI)

⚠️ Breaking change

Any 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 the AGENTS.md hunk split out and committed separately. The CMakeLists.txt + ci.yml half is Tier 2 and reviewable normally.

Checklist

  • Commits are signed
  • No new flat-key AppSettings calls (Principle V) — n/a
  • Code is clean-room (Principle IV)
  • All meter UI uses MeterSmoother — n/a
  • Documentation updated if user-visible behavior changed — AGENTS.md updated in this PR
  • Security-sensitive changes reference a GHSA if applicable — n/a

🤖 Generated with Claude Code

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>
@ten9876
ten9876 requested review from a team as code owners July 26, 2026 21:29
@ten9876 ten9876 self-assigned this Jul 26, 2026

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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=OFF is unnecessary (the lib is in-tree) and costs the sid canary its DFNR compile coverage; suggest dropping the hunk.

Non-blocking notes

  • codeql.yml:74 and sanitizers.yml:63 also 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 this FATAL_ERROR and 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.md hunk 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

Comment thread .github/workflows/ci.yml
- 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.

Comment thread AGENTS.md
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)"

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Antares Detector — candidate vulnerable file(s)

  • third_party/zlib/configure
  • tests/run_digital_voice_waveform_no_args.cmake
  • tools/migrate_colours.py

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.