nls: hard-fail when ENABLE_NLS=YES but gettext or libintl missing - #512
Merged
Conversation
Mirror of amule-project#509 / amule-project#511 applied to cmake/nls.cmake. Two soft-fail spots both flipped to FATAL_ERROR with platform-specific install hints: 1. msgfmt / msgmerge missing — fires on any platform without GNU gettext-tools installed. Hint covers Debian/Ubuntu (gettext), Fedora (gettext), macOS Homebrew (gettext), MSYS2 (mingw-w64-x86_64-gettext). 2. libintl missing — fires only on platforms with a separate libintl runtime (macOS, MinGW/MSYS2, *BSD, musl). On glibc, libintl is part of libc itself and find_package(Intl) reports FOUND with empty Intl_LIBRARIES; the branch never fires there. Hint includes a note that hitting it on a glibc system means glibc-devel / libc6-dev is incomplete. Both branches previously did 'set (ENABLE_NLS FALSE)' + STATUS message, masking the missing dep behind a green build with no localization at runtime — same anti-pattern PR amule-project#509 fixed for IP2Country and amule-project#511 fixed for UPnP. Per @Vollstrecker on amule-project#507 (comment): > If a user sets a feature to enable and the deps for it aren't found, > we don't disable it and keep going, we error-out and the feature > will be disabled by the user, or the deps is provided. The same anti-pattern in cmake/boost.cmake will land as a separate small PR (more nuanced — DOWNLOAD_AND_BUILD_DEPS interaction). Test plan: * configure with deps present (-DENABLE_NLS=YES) — succeeds, NLS enabled (verified locally on macOS Homebrew gettext). * configure with msgfmt missing (-DENABLE_NLS=YES) — fails with the new FATAL_ERROR naming gettext per platform. * configure with libintl missing on macOS/MinGW (-DENABLE_NLS=YES) — fails with the new FATAL_ERROR naming the runtime package. * configure with -DENABLE_NLS=NO — succeeds, no probing happens (top-level CMakeLists.txt:79 gates the include behind the option).
3 tasks
mrjimenez
pushed a commit
that referenced
this pull request
May 2, 2026
Mirror of #509 / #511 / #512 applied to cmake/boost.cmake. Two soft-fail spots both flipped to FATAL_ERROR: 1. asio probe failed (boost found but check_include_files for boost/system/error_code.hpp + boost/asio.hpp couldn't compile), no DOWNLOAD_AND_BUILD_DEPS opt-in. This is the realistic case — a user has libboost-dev installed but missing some transitive header / version mismatch / link-step issue tripped the probe. Previously: silently set ENABLE_BOOST=FALSE, falling back to wxWidgets sockets. Now: FATAL_ERROR with hint to install full boost-dev, or pass -DENABLE_BOOST=NO, or pass -DDOWNLOAD_AND_BUILD_DEPS=YES. 2. Boost_FOUND=FALSE — defensive branch, practically unreachable because find_package(Boost CONFIG REQUIRED) at the top of the file already errors out when boost is absent. Replacing the silent disable with FATAL_ERROR makes the assumption explicit: if a future edit ever drops the REQUIRED keyword, this branch surfaces the regression instead of producing a green build with no boost. The DOWNLOAD_AND_BUILD_DEPS escape hatch (where CMake is expected to build Boost itself elsewhere in the configure pass) is preserved for branch 1 — when DAaBD is set, no error fires. Per @Vollstrecker on #507 (comment): > If a user sets a feature to enable and the deps for it aren't found, > we don't disable it and keep going, we error-out and the feature > will be disabled by the user, or the deps is provided. This completes the four-file cmake/<feature>.cmake hard-fail series (ip2country in #509, upnp in #511, nls in #512, boost here). Test plan: * configure with deps present (-DENABLE_BOOST=YES) — succeeds, boost found, ASIO_SOCKETS=TRUE (verified locally on macOS Homebrew boost 1.90.0). * configure with -DENABLE_BOOST=NO — succeeds, no probing happens (top-level CMakeLists.txt:71 gates the include behind the option). * configure with deps missing (-DENABLE_BOOST=YES, no boost) — the REQUIRED keyword on find_package(Boost CONFIG) at line 1 errors out before our changes are reached; the new FATAL_ERROR for the asio-probe-failed case isn't triggerable without artificially installing a broken boost. Mechanically identical FATAL_ERROR shape to #509 / #511 / #512.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
May 2, 2026
…3nks mrjimenez flagged on PR amule-project#514 review that these three contributors were missing from the roster: * Werner Mahr / Vollstrecker — has merge access on amule-project/amule and merges incoming PRs (e.g. amule-project#430, amule-project#385, amule-project#319, amule-project#330, amule-project#286). Goes under Maintainers based on that role. * Pablo Barciela / Sc0w — recent contributor, e.g. wxWidgets 3.2.6 build fixes, GTK version-check cleanup, dropping gtk1/gtk2 support, Chinese translation work. Goes under Developers. * got3nks — Wayland app_id binding + SNI tray (amule-project#508), macOS UX (amule-project#508), libmaxminddb CI fix (amule-project#507), ip2country/upnp/nls/boost hard-fail series (amule-project#509/amule-project#511/amule-project#512/amule-project#513), packaging (amule-project#510), docs modernize (this PR). Goes under Developers. Other recent folks may be missing too — those can be added in follow-up commits as they're identified. mrjimenez explicitly noted this is the priority subset to not block the rename PR.
mrjimenez
pushed a commit
that referenced
this pull request
May 2, 2026
…3nks mrjimenez flagged on PR #514 review that these three contributors were missing from the roster: * Werner Mahr / Vollstrecker — has merge access on amule-project/amule and merges incoming PRs (e.g. #430, #385, #319, #330, #286). Goes under Maintainers based on that role. * Pablo Barciela / Sc0w — recent contributor, e.g. wxWidgets 3.2.6 build fixes, GTK version-check cleanup, dropping gtk1/gtk2 support, Chinese translation work. Goes under Developers. * got3nks — Wayland app_id binding + SNI tray (#508), macOS UX (#508), libmaxminddb CI fix (#507), ip2country/upnp/nls/boost hard-fail series (#509/#511/#512/#513), packaging (#510), docs modernize (this PR). Goes under Developers. Other recent folks may be missing too — those can be added in follow-up commits as they're identified. mrjimenez explicitly noted this is the priority subset to not block the rename PR.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirror of #509 / #511 applied to
cmake/nls.cmake. Two soft-fail spots both flipped toFATAL_ERRORwith platform-specific install hints:gettext), Fedora (gettext), macOS Homebrew (gettext), MSYS2 (mingw-w64-x86_64-gettext).find_package(Intl)reports FOUND with emptyIntl_LIBRARIES; the branch never fires there. Hint includes a note that hitting it on a glibc system meansglibc-devel/libc6-devis incomplete.Both branches previously did
set(ENABLE_NLS FALSE)+ STATUS message, masking the missing dep behind a green build with no localization at runtime — same anti-pattern #509 fixed for IP2Country and #511 fixed for UPnP.Why
Per @Vollstrecker on #507 (comment):
This change applies that policy to NLS. The macOS-specific find_program HINTS at the top of
cmake/nls.cmake(which auto-locate Homebrew's keg-only gettext) stay unchanged — they're a fulfillment path, not a soft-fail.Companion PR
The same anti-pattern in
cmake/boost.cmakewill land as a separate small PR (more nuanced — DOWNLOAD_AND_BUILD_DEPS interaction).Test plan
-DENABLE_NLS=YES— succeeds,Everything is fine. aMule can be localized(verified locally on macOS Homebrew gettext 0.26).-DENABLE_NLS=NO— succeeds, no probing happens (top-levelCMakeLists.txt:79gates the include behind the option).-DENABLE_NLS=YES— not directly testable on macOS without uninstalling Homebrew gettext: thefind_programHINTS atcmake/nls.cmake:27-33deliberately auto-locate the keg-only install, bypassing PATH manipulation. The FATAL_ERROR branch is mechanically identical to upnp: hard-fail when ENABLE_UPNP=YES but libupnp is missing #511's verified pattern (sameif(NOT VAR)→message(FATAL_ERROR ...)shape). Will validate on a fresh Linux runner without gettext if needed at review time.