ci(appimage): build and bundle qtkeychain for SmartLink credential persistence (#3639)#3640
Conversation
There was a problem hiding this comment.
Clean, well-scoped packaging fix — thanks @ten9876. This is the correct Linux counterpart to #3634, and the approach is sound: build qtkeychain from source against the active Qt for ABI match, LIBSECRET_SUPPORT=OFF for the pure Qt-D-Bus Secret Service backend (no extra native deps to bundle), and REQUIRE_KEYCHAIN mirroring the existing REQUIRE_SERIALPORT guard so a release artifact can't silently ship without persistence.
A few things I verified that look correct:
- CMake change is macOS/system-safe — the
list(PREPEND CMAKE_PREFIX_PATH ...)is gated onEXISTS .../third_party/qtkeychain, so macOS (brew) and distro-package builds are untouched; only staged builds pick up the prepend. LD_LIBRARY_PATHlib dir matches the script'sCMAKE_INSTALL_LIBDIR=lib, and Qt6 DBus is correctly expected to come in transitively vialibqt6keychain.so'sNEEDEDentry once linuxdeploy resolves it.- Setup script uses
set -euo pipefail, the bare0.16.0tag matches qtkeychain's tag scheme, and the config-file existence check is a clean cache/re-run guard. - Logging category (
aether.smartlink) defaults toQtWarningMsg, so theqCWarningdisabled-path diagnostic surfaces by default while theqCInfoenabled line stays quiet — the right asymmetry for the "password not remembered" case.
One thing worth flagging before dispatch:
REQUIRE_KEYCHAIN=ON applies to both matrix legs, but validation only covered x86_64. The Configure step sets it unconditionally, so it also hard-fails the aarch64 leg (ubuntu-24.04-arm, system Qt via apt, qtkeychain built against that Qt through the qmake6 fallback). If the source build hiccups on the system-Qt path, the entire aarch64 AppImage release fails rather than degrading. Since the workflow only runs on tags/dispatch, a bad aarch64 build wouldn't surface until release time. Recommend the maintainer workflow_dispatch validation explicitly confirms both arches produce an AppImage containing libqt6keychain.so, not just the x86_64/KDE box in the PR notes.
Minor (optional, non-blocking): the script comment says the config-file check "lets CI cache third_party/qtkeychain," but no actions/cache step is added here — though that's consistent with the existing DeepFilterNet3 setup (also uncached), so each tagged build rebuilds from source. Fine as-is given the trigger cadence; just noting the comment is slightly aspirational.
Nothing blocking from my side — the diagnosable-logging addition and the hard-fail guard are both good defensive touches.
🤖 aethersdr-agent · cost: $6.3128 · model: claude-opus-4-8
…rsistence (#3639) Linux AppImage counterpart to the Windows fix (#3634): the AppImage shipped without Qt6Keychain, so SmartLink/MQTT credential persistence silently compiled out (no Auth0 refresh token stored, no log line explaining why). - scripts/setup/setup-qtkeychain.sh — builds qtkeychain 0.16.0 from source against the active Qt (ABI match with the aqt Qt the AppImage links). LIBSECRET_SUPPORT=OFF selects the pure Qt-DBus Secret Service backend (KDE Wallet / GNOME Keyring), so only Qt6 DBus is bundled — no extra native deps. - .github/workflows/appimage.yml — setup before configure, -DREQUIRE_KEYCHAIN=ON, and stage the lib dir on LD_LIBRARY_PATH so linuxdeploy bundles libqt6keychain.so. - CMakeLists.txt — new REQUIRE_KEYCHAIN option (mirrors REQUIRE_SERIALPORT): a missing Qt6Keychain becomes a hard build failure for release/packaged builds instead of a silent compile-out. The EXISTS-guarded third_party/qtkeychain prepend supersedes #3634's WIN32-only one (covers both platforms). - windows-installer.yml — pass -DREQUIRE_KEYCHAIN=ON too, now that the guard exists and #3634 stages qtkeychain there (its own follow-up note). - SmartLinkClient.cpp — log credential-persistence availability at construction (qCWarning under aether.smartlink when disabled) so the compiled-out case is diagnosable instead of silent. Rebased onto main after #3634 merged; CMakeLists conflict resolved by collapsing the two prepends into the general EXISTS-guarded form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
535b09e to
66f2cdb
Compare
|
Hi @ten9876 — thanks for tackling #3639, shipping credential persistence in the AppImage is a genuinely useful fix. 🙏 Quick triage on the red CI so you're not hunting in the dark. Where it failedThe failure is isolated to the Everything before packaging was green, which is the important clue:
It breaks at the Honesty noteThe triage tooling I have surfaces the failing step, but not the raw Most likely causes (check these against the real log)
Suggested next stepGrab the exact bash scripts/setup/setup-qtkeychain.sh
export LD_LIBRARY_PATH="$PWD/third_party/qtkeychain/lib:$LD_LIBRARY_PATH"
./linuxdeploy-x86_64.AppImage --appdir AppDir --plugin qt --output appimageRelevant files for reference: No reviewer/Copilot comments are on the PR yet, so nothing else to fold in. Happy to take another look once you've pushed a fix — you're very close, it's just the bundling handoff. 🚀 🤖 aethersdr-agent · cost: $4.2647 · model: claude-opus-4-8 |
A workflow_dispatch on a branch (e.g. ci/appimage-qtkeychain) puts a '/' in
github.ref_name, which became AetherSDR-ci/appimage-qtkeychain-x86_64.AppImage
and broke mksquashfs ("Could not create destination file: No such file or
directory"). Tag pushes (v26.6.x) are slash-free so releases were unaffected,
but dispatch-based validation of this very PR couldn't produce an AppImage.
Replace unsafe chars with '_' before building LDAI_OUTPUT, mirroring the
sanitization windows-installer.yml already applies for the same reason.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Problem
The Linux AppImage is built without Qt6Keychain, so
HAVE_KEYCHAINis undefined and SmartLink (and MQTT) credential persistence silently compiles out. The Auth0 refresh token is never stored, the email is remembered (it lives inAppSettings, not the keychain) but the password/login is not, and there's no log line explaining why.The reporter confirmed a from-source build on the same Manjaro/KDE box stores and retrieves the credential through KDE Wallet — so the system wallet is fine. The gap is purely AppImage packaging. Fixes #3639.
This is the Linux counterpart to the Windows packaging fix in #3634.
Changes
scripts/setup/setup-qtkeychain.sh(new) — builds qtkeychain 0.16.0 from source against the active Qt and stages it inthird_party/qtkeychain. Building from source (vsapt install qtkeychain-qt6-dev) guarantees an ABI match with the aqt Qt 6.8.3 the AppImage links.LIBSECRET_SUPPORT=OFFselects qtkeychain's pure Qt-D-Bus Secret Service backend, which talks to KDE Wallet (kwalletd) and GNOME Keyring over the session bus with no extra native runtime deps to bundle — only Qt6 DBus..github/workflows/appimage.yml— runs the setup before configure, passes-DREQUIRE_KEYCHAIN=ON, and adds the staged lib dir toLD_LIBRARY_PATHsolinuxdeployresolves and bundleslibqt6keychain.so(Qt6 DBus comes in transitively).CMakeLists.txt— addsREQUIRE_KEYCHAIN(mirrors the existingREQUIRE_SERIALPORTguard) so a missing keychain is a hard build failure for release/packaged builds instead of a silent compile-out; prepends thethird_party/qtkeychainstaging dir toCMAKE_PREFIX_PATH.src/core/SmartLinkClient.cpp— logs credential-persistence availability at construction (aqCWarningunderaether.smartlink/ Help → Support → SmartLink when disabled), so the compiled-out case is diagnosable instead of failing silently.Validation note
tags: ['v*']andworkflow_dispatch— it will not run automatically on this PR. A maintainer should dispatch it onci/appimage-qtkeychain(Actions → AppImage → Run workflow) and confirm:Qt6Keychain found — SmartLink credential persistence enabled-DREQUIRE_KEYCHAIN=ONwould now fail the build if keychain were missinglibqt6keychain.so(--appimage-extractthenfind squashfs-root -iname '*keychain*')Follow-up (out of scope)
Windows (#3634) could also pass
-DREQUIRE_KEYCHAIN=ONnow that the guard exists, to get the same release-time protection.🤖 Generated with Claude Code