ci(ubuntu): install binutils-dev so the BFD path is exercised - #488
Merged
Merged
Conversation
cmake/bfd.cmake's foreach probe loop only runs when bfd.h is findable on the build host, i.e. when binutils-dev is installed. None of the CI jobs install it today, which is why the bugs fixed in amule-project#487 (PR ae5b70b) survived for years -- the entire buggy block was dead code under CI. Adding binutils-dev to the Ubuntu CI dep list ensures the BFD probe loop actually runs, so any future regression in bfd.cmake fails the build. The mingw-w64 and macOS jobs are intentionally left alone: cmake/bfd.cmake is platform-agnostic, so coverage on one platform is enough and we avoid bloating the other two jobs' install steps. Suggested by @danim7 as a follow-up to amule-project#487.
3 tasks
4 tasks
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 15, 2026
…sing-override (amule-project#488) (amule-project#493) * style(amule): mark CamuleApp wxApp overrides with `override` (amule-project#488) CamuleApp already marks some of its wxApp overrides `override` (e.g. EnableIP2Country, GetIP2Country), so newer clang flags the ones that don't via -Winconsistent-missing-override. That fires 800+ times across the build because amule.h is included in almost every translation unit (issue amule-project#488, and it grows louder on the fussier clang shipping in Ubuntu 26.10). Mark the five remaining wxApp overrides -- OnInit, OnExit, OnFatalException, OnAssertFailure, OnUnhandledException -- with `override` (dropping the now redundant `virtual`), matching the class's existing style. `override` also makes the compiler verify they really do override a base method. Reproduced locally (830 warnings) and confirmed 0 after the change, clean build. Scope is deliberately just these compiler warnings; the broader tree-wide modernize-use-override sweep + enabling the clang-tidy check is left for a separate one-shot PR. Refs amule-project#488 * build: promote -Winconsistent-missing-override to an error on Clang (amule-project#488) Now that amule.h no longer trips it, guard against the whole class of amule-project#488 warning recurring: when a class marks some virtual overrides `override` but not others, Clang emits -Winconsistent-missing-override once per translation unit, so one header can produce hundreds of warnings (amule.h was 800+). Promote it to an error, mirroring the existing -Werror=deprecated* gate. Clang-only via a generator expression: GCC has no "some marked, some not" variant (only the whole-tree -Wsuggest-override), so an unconditional -Werror= would break the GCC/Ubuntu build. The Clang-based CI jobs (macOS, mingw-w64) are enough to catch a regression. The warning is on by default under Clang, so only the promotion to error is added. Verified: build stays green, and un-marking a single override makes it fail as intended. Refs amule-project#488
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
2 participants
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.
Follow-up to #487 (already merged as ae5b70b). Suggested by @danim7 in #487 (comment).
Why
cmake/bfd.cmakehas acheck_include_file (bfd.h HAVE_BFD)gate. Whenbfd.hisn't findable, the entire foreach probe loop below is dead code. None of the CI jobs installbinutils-dev(or its mingw/Homebrew equivalents), so the buggy block fixed in #487 was never exercised in CI — which is exactly why three independent bugs in it survived for years.What
Adds
binutils-devtocmake_ubuntu_deps(one line). The Ubuntu job will then havebfd.hfindable, the foreach probe loop will run, and any future regression incmake/bfd.cmakewill fail the build.Why Ubuntu only
cmake/bfd.cmakeis platform-agnostic — there's no per-platform branching in it. Coverage on one platform is sufficient to catch regressions; adding the dep to mingw-w64 and macOS jobs would just bloat their install steps without adding meaningful coverage.Test plan
bfd.cmakeconfigures cleanly post-cmake/bfd: fix unset syntax error and undefined-variable typos #487.