Add option to make the binary's name lower case#3138
Conversation
There was a problem hiding this comment.
Thanks for the contribution, @dawkagaming — lowercase binary names are very much the Linux convention and a sensible packaging option to expose. A few issues to address before this is mergeable:
1. Desktop file is unconditionally broken when the option is OFF.
The option defaults to OFF, so by default the binary is still installed as AetherSDR, but AetherSDR.desktop now points at Exec=aethersdr. That means desktop-menu launching will silently fail for every existing Linux user the moment this lands. The desktop file's Exec= must match whatever the build actually produced.
The cleanest fix is to template the desktop file:
if(UNIX AND NOT APPLE AND LINUX_LOWER_CASE_BINARY_NAME)
set(AETHERSDR_BINARY_NAME aethersdr)
else()
set(AETHERSDR_BINARY_NAME AetherSDR)
endif()
configure_file(
${CMAKE_SOURCE_DIR}/packaging/linux/AetherSDR.desktop.in
${CMAKE_BINARY_DIR}/packaging/linux/AetherSDR.desktop @ONLY)…and change the checked-in file to AetherSDR.desktop.in with Exec=@AETHERSDR_BINARY_NAME@. Same set_target_properties block can then key off ${AETHERSDR_BINARY_NAME} too, which keeps the two in sync by construction.
2. if(LINUX ...) is not portable to this project's CMake floor.
The LINUX variable was added in CMake 3.25, but CMakeLists.txt:1 declares cmake_minimum_required(VERSION 3.20). On CMake 3.20–3.24 your conditional silently evaluates to false, so the rename never happens and the user just gets a confusing "option did nothing" experience. The existing code already uses the portable idiom — see CMakeLists.txt:67 (if(UNIX AND NOT APPLE)). Please match that.
3. option() is missing its help string.
option(LINUX_LOWER_CASE_BINARY_NAME OFF)With no help text, CMake treats "OFF" as the docstring and the default value falls back to OFF — it works by accident. Every other option in this file (CMakeLists.txt:45, :120, :215, etc.) uses the proper 3-arg form. Please align:
option(LINUX_LOWER_CASE_BINARY_NAME "Install the Linux binary as 'aethersdr' instead of 'AetherSDR' (matches distro packaging convention)" OFF)No other concerns — the change itself is small and the intent is good. Once the desktop file and the LINUX check are sorted, this should be ready.
🤖 aethersdr-agent · cost: $6.9300 · model: claude-opus-4-7
ac98a20 to
7dfe723
Compare
|
Hi @dawkagaming — thanks for the PR! This will be helpful for distro packagers. The CI 1. CMake minimum version bump is too aggressive (the actual failure)In -cmake_minimum_required(VERSION 3.20)
+cmake_minimum_required(VERSION 3.31)The CI container is built on 2. Typo in the
|
7dfe723 to
60ca87a
Compare
|
Claude here. The core idea here is solid — lowercase binary names are a long-standing Linux convention ( There are a few mechanical issues blocking the merge though: 1. The CMake version bump is the immediate CI failureThe diff bumps Fix: revert that line. Keep 2. The
|
b0a52c9 to
78357e2
Compare
78357e2 to
3d1566c
Compare
|
Claude here. Merged at 64eedbc. Clean revision — took every piece of feedback and the diff stays minimal. AetherSDR is now packageable with a lowercased binary name via Thanks Dawid — this is the third of your packaging-readiness PRs to land (after #3074 and #3143), and the rebuilt iteration here was textbook: CMake-version-aware platform check, build-tree output for generated files, install rule consuming the configured artifact. Exactly how this kind of multi-distro plumbing should look. A small downstream note for whoever's doing the actual packaging: this bumps 73, Jeremy KK7GWY & Claude (AI dev partner) |
…TT placement Conflict resolutions in CMakeLists.txt: * Option block conflict (top of file) — combined Dawid's centralised USE_SYSTEM_* + build-feature option block with main's LOWER_CASE_BINARY_NAME option (PR aethersdr#3138). * Link-libraries block conflict (~line 875) — kept both the new USE_SYSTEM_* target_link_libraries calls and main's new LOWER_CASE_BINARY_NAME OUTPUT_NAME logic. Independent additions, both retained. Additional fixes on top: * Dropped cmake_minimum_required from 3.25 back to 3.20. After the switch from cmake_pkg_config to pkg_check_modules(IMPORTED_TARGET ...) nothing in the new code needs 3.25. Restoring the original 3.20 keeps Debian 12 (cmake 3.25) friendly and matches what the rest of the file was written against. * HAVE_MQTT / HAVE_MQTT_TLS placement — the previous form nested `target_compile_definitions(... HAVE_MQTT)` inside `if (NOT USE_SYSTEM_LIBMOSQUITTO)`, so USE_SYSTEM_LIBMOSQUITTO=ON would link against the system package but leave HAVE_MQTT undefined, meaning every `#ifdef HAVE_MQTT` block in the rest of the codebase compiled out. Moved HAVE_MQTT to the outer ENABLE_MQTT scope, and route HAVE_MQTT_TLS on the system-mosquitto branch through the existing MQTT_TLS option (distro mosquitto packages are conventionally TLS-enabled). Local build clean against the bundled defaults (USE_SYSTEM_* all OFF). Co-Authored-By: dawkagaming <147207983+dawkagaming@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hello,
I think it might be useful to add an option for lower case binary naming, as usually the binaries in Linux distributions looks like that.
Thanks,
Dawid SP9SKA