Skip to content

cmake: work around broken UPNP.cmake in some distro packages - #439

Merged
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:pr/upnp-cmake-fix
Apr 9, 2026
Merged

cmake: work around broken UPNP.cmake in some distro packages#439
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:pr/upnp-cmake-fix

Conversation

@got3nks

@got3nks got3nks commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Some distro packages ship a UPNP.cmake config file that lists non-existent paths in INTERFACE_INCLUDE_DIRECTORIES, causing CMake to abort at generation time. Observed on Ubuntu 25.10 with libupnp-dev 1.14.24:

CMake Error in src/CMakeLists.txt:
  Imported target "UPNP::Shared" includes non-existent path
    "/usr/COMPONENT"
  in its INTERFACE_INCLUDE_DIRECTORIES.

The offending line in the distro's UPNP.cmake:

INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/upnp;${_IMPORT_PREFIX}/COMPONENT;${_IMPORT_PREFIX}/UPNP_Development"

/usr/COMPONENT and /usr/UPNP_Development do not exist — this is a packaging bug unrelated to the callback signature issue discussed in #432.

Fix

Before loading the CMake config, inspect the targets file for the known bad path pattern. If detected, skip find_package(UPNP CONFIG) and fall back to pkg-config, which correctly finds libupnp on all affected systems:

find_file (_upnp_cmake_targets "UPNP.cmake" ...)
if (_upnp_cmake_targets)
    file (READ "${_upnp_cmake_targets}" _upnp_cmake_content)
    if (_upnp_cmake_content MATCHES "INTERFACE_INCLUDE_DIRECTORIES.*COMPONENT")
        message (STATUS "Broken UPNP CMake config detected — using pkg-config instead")
        set (_upnp_skip_config TRUE)
    endif()
endif()

Tested on

  • Ubuntu 25.10, libupnp 1.14.24 — previously failed at CMake configure, now builds cleanly with UPnP enabled
  • Related to the libupnp packaging discussion in Fix build with libupnp 1.18 #432

Some distro packages (observed on Ubuntu 25.10 with libupnp-dev 1.14.24)
ship a UPNP.cmake that lists non-existent paths (/usr/COMPONENT,
/usr/UPNP_Development) in INTERFACE_INCLUDE_DIRECTORIES, causing CMake
to abort at generation time:

  CMake Error: Imported target "UPNP::Shared" includes non-existent
  path "/usr/COMPONENT" in its INTERFACE_INCLUDE_DIRECTORIES.

This is a packaging bug unrelated to the callback signature issue
discussed in amule-project#432. Work around it by inspecting the UPNP targets file
before loading it, and falling back to pkg-config if the broken path
pattern is detected. pkg-config correctly finds libupnp on all affected
systems.
@mrjimenez
mrjimenez merged commit b878055 into amule-project:master Apr 9, 2026
2 checks passed
@Vollstrecker

Copy link
Copy Markdown
Collaborator

I'm not sure if this is really needed as it's fixed upstream and I'm sure it will arrive in Ubuntu before a release.

mrjimenez pushed a commit that referenced this pull request Apr 22, 2026
Exposed by new CI jobs covering macOS and MSYS2 MINGW64; all four fixes
are narrow and guarded so Linux behaviour is unchanged.

* src/utils/cas/CMakeLists.txt: link -framework CoreServices on Apple
  (cas/functions.c calls FSFindFolder/CFURL*, matching the alc/wxcas
  pattern). Also add PRIVATE keyword to the existing gdlib link to
  satisfy CMake's no-plain/keyword-mixing rule.

* cmake/gdlib.cmake: relax if (NOT WIN32) to if (NOT MSVC) so MSYS2
  MinGW, which ships a working gdlib.pc at /mingw64/lib/pkgconfig/,
  can use the pkg-config path. MSVC still skips (no pkg-config there).

* cmake/upnp.cmake:
  - drop NO_DEFAULT_PATH and the hard-coded Linux PATHS from the
    broken-UPNP-config detection find_file; switch to PATH_SUFFIXES
    including lib/cmake/UPNP so CMake's default search reaches
    /mingw64/lib/cmake/UPNP too. MSYS2's mingw-w64-pupnp 1.14.x has
    the same /COMPONENT bad-path bug addressed for Ubuntu in #439.
  - find_package(Threads REQUIRED) before find_package(UPNP CONFIG):
    pupnp's UPNPConfig.cmake references Threads::Threads in its link
    interface without loading Threads itself. No-op on platforms
    where Threads was already resolved.
  - pkg-config fallback: use UNKNOWN IMPORTED on MinGW. SHARED IMPORTED
    without IMPORTED_IMPLIB makes Debug-config lookups resolve to
    UPNP::Shared-NOTFOUND on MinGW. Linux/macOS keep SHARED IMPORTED.

* cmake/cryptopp.cmake: when the Windows branch finds only the release
  library (MSYS2 mingw-w64-crypto++ ships libcryptopp.dll.a with no
  *d.dll.a counterpart), also set plain IMPORTED_LOCATION so Debug
  builds fall back to it. Change only runs inside the existing
  if (WIN32) block and only when CRYPTOPP_LIBRARY_DEBUG is unset, so
  MSVC builds with separate debug/release libs are unaffected.
@got3nks got3nks mentioned this pull request Apr 29, 2026
@got3nks
got3nks deleted the pr/upnp-cmake-fix branch May 3, 2026 15:19
ngosang pushed a commit to ngosang/amule that referenced this pull request Jul 13, 2026
… + REST, and add remote GeoIP config (amule-project#439, amule-project#440)

Resolves amule-project#439 (peer country) and amule-project#440 (server country).

**Core refactor.** Move the GeoIP resolver (`CIP2Country`) out of the GUI and into the core (`CamuleApp`), headless, so `amuled` resolves country codes too. Flag rendering becomes a separate GUI concern (`CCountryFlags`, embedded PNGs) — no libmaxminddb needed to display.

**amule-project#439 / amule-project#440.** Resolve each peer's and server's country core-side and expose the ISO code: over EC (`EC_TAG_CLIENT_COUNTRY`, `EC_TAG_SERVER_COUNTRY`); over REST as `country_code` on `/clients` and `/servers` (always present, empty when off/unresolved); and in amulecmd's `show servers` (e.g. `[de]`). The peer list renders the flag; unresolved peers show none.

**Remote GeoIP config (amulegui).** amulegui has no resolver — it configures the daemon's GeoIP over EC: settings sync, live status line, "Update now", auto-download on source change, and the page is hidden when the connected core lacks GeoIP (built without it, or a pre-3.1 daemon).

**Decoupling.** `ENABLE_IP2COUNTRY` now gates only the resolver (libmaxminddb); a new `GEOIP_GUI` gate (`ENABLE_IP2COUNTRY || CLIENT_GUI`) covers display + config, so amulegui shows flags and edits GeoIP config with no libmaxminddb linked.

**REST `/preferences`.** New `ip2country` object (GET + PATCH: enabled, source, custom URL, MaxMind license, auto-update, `update_now`; plus read-only status + `supported`).

Mixed builds (core off / GUI on and vice-versa) are link- and crash-safe. Tests: `RefresherTest`, curl (`05`, `10`), plus live macOS validation (monolithic + amulegui + amuled + amuleapi). Docs: `docs/api/REFERENCE.md`.
ngosang added a commit to ngosang/amule that referenced this pull request Jul 29, 2026
… columns

Surface `country_code` (amule-project#439/amule-project#440, added core-side in 5b7cdf1) in the Web UI as
a plain 2-letter ISO 3166-1 alpha-2 code, no flag: a sortable country column in
the peer table (Clients page + the per-file Clients tab of both detail panels)
and in the ED2K server table. Its header is an abbreviation ("CC" in English,
"CP" in Spanish) because a spelled-out "Country" would force ~80px for a 2-char
cell.

The column always renders. The daemon omits the EC tag entirely when GeoIP is off
or the build lacks it, which reaches the REST layer as `country_code: ""` and
shows as "—" like any other empty cell -- so nothing has to probe for GeoIP
support (/status carries no GeoIP flag, and the Web UI has no preferences store
to read `ip2country.supported` from).

Also fills three gaps in the peer table -- fields /clients has always returned
but nothing painted: Address (`ip:port`), OS (`os_info`) and User hash. All three
start hidden in every consumer, so they only widen the table once picked from the
column picker. Peer columns now run CC, Address, Name, User hash, Software, OS,
File, so each identity field sits next to the one it qualifies. The four transfer
totals are relabelled to the compact DL/UL total and DL/UL session, which also
lets all four share one width.

The server table is reordered to match (CC, Address, Name, Description, Users,
Files, Version, Ping, Priority, Actions) and starts with Address, Version and
Ping hidden, keeping the default view to what identifies a server and how busy it
is. Description also drops its fixed 180px and splits the leftover width with
Name, since descriptions are long (forum URLs, blurbs). Both tables' Name columns
lose their `always` flag, so the picker offers them like any other column -- with
Address and CC available, the name is no longer the only way to tell rows apart.
Downloads / Shared / Search keep theirs pinned: there the name column anchors the
row-selection checkbox.

Two pre-existing bugs this surfaced, both fixed for every list view:

  * A header label wider than its column spilled over the neighbouring header
    instead of being clipped (`white-space: nowrap`, and no overflow rule of its
    own) -- which is why a "Country" label looked like it fit 70px. Headers now
    ellipsize and carry the full label as a title tooltip, so a translation that
    outgrows its column stays readable on hover.
  * The server Address column sorted its `ip:port` strings lexically, putting
    10.x before 9.x and .182 before .87. Both tables now sort it by IP value
    through one shared `ipNum` helper in table.js, next to the other sort/filter
    helpers.

Peer detail-only fields (GET /clients/{ecid}) are untouched: they need a peer
detail panel, not a column.
ngosang added a commit to ngosang/amule that referenced this pull request Jul 29, 2026
… columns (amule-project#690)

Surface `country_code` (amule-project#439/amule-project#440, added core-side in 5b7cdf1) in the Web UI as
a plain 2-letter ISO 3166-1 alpha-2 code, no flag: a sortable country column in
the peer table (Clients page + the per-file Clients tab of both detail panels)
and in the ED2K server table. Its header is an abbreviation ("CC" in English,
"CP" in Spanish) because a spelled-out "Country" would force ~80px for a 2-char
cell.

The column always renders. The daemon omits the EC tag entirely when GeoIP is off
or the build lacks it, which reaches the REST layer as `country_code: ""` and
shows as "—" like any other empty cell -- so nothing has to probe for GeoIP
support (/status carries no GeoIP flag, and the Web UI has no preferences store
to read `ip2country.supported` from).

Also fills three gaps in the peer table -- fields /clients has always returned
but nothing painted: Address (`ip:port`), OS (`os_info`) and User hash. All three
start hidden in every consumer, so they only widen the table once picked from the
column picker. Peer columns now run CC, Address, Name, User hash, Software, OS,
File, so each identity field sits next to the one it qualifies. The four transfer
totals are relabelled to the compact DL/UL total and DL/UL session, which also
lets all four share one width.

The server table is reordered to match (CC, Address, Name, Description, Users,
Files, Version, Ping, Priority, Actions) and starts with Address, Version and
Ping hidden, keeping the default view to what identifies a server and how busy it
is. Description also drops its fixed 180px and splits the leftover width with
Name, since descriptions are long (forum URLs, blurbs). Both tables' Name columns
lose their `always` flag, so the picker offers them like any other column -- with
Address and CC available, the name is no longer the only way to tell rows apart.
Downloads / Shared / Search keep theirs pinned: there the name column anchors the
row-selection checkbox.

Two pre-existing bugs this surfaced, both fixed for every list view:

  * A header label wider than its column spilled over the neighbouring header
    instead of being clipped (`white-space: nowrap`, and no overflow rule of its
    own) -- which is why a "Country" label looked like it fit 70px. Headers now
    ellipsize and carry the full label as a title tooltip, so a translation that
    outgrows its column stays readable on hover.
  * The server Address column sorted its `ip:port` strings lexically, putting
    10.x before 9.x and .182 before .87. Both tables now sort it by IP value
    through one shared `ipNum` helper in table.js, next to the other sort/filter
    helpers.

Peer detail-only fields (GET /clients/{ecid}) are untouched: they need a peer
detail panel, not a column.
ngosang added a commit to ngosang/amule that referenced this pull request Jul 30, 2026
…mule-project#687) (amule-project#719)

1bc9d6e (amule-project#694) added GET /flags/{code}.png, but nothing consumed it: the
country column of the peer list and of the ed2k server list still painted only
the bare uppercase code, duplicating the same one-liner cell in two views.

Both now render through a single new CountryCell in components.js: the 16x11
famfamfam flag from the route followed by the code, the layout the desktop
lists already use. The image URL is built relative to window.location.pathname
like BASE in api.js, so it survives a reverse-proxy subpath, and the route's
one-day Cache-Control means a peer list full of <img> tags doesn't re-fetch a
flag per country on reload.

country_code is an empty string when the daemon's GeoIP is off or the IP
doesn't resolve (amule-project#439, amule-project#440), so an empty code short-circuits to the dash the
cell already showed -- no <img>, and in particular no request for the
"/flags/.png" the route would 404. A well-formed code the flag set has no
artwork for (zz, GeoIP pseudo-codes like ap/eu) also 404s, so onError hides
the image and leaves the code readable instead of a broken-image icon.

The cell gains a title with the localized country name via Intl.DisplayNames
({ type: "region" }) in the UI language, as docs/api/REFERENCE.md prescribes:
no endpoint and no new translation keys, since the browser already has the
data. The formatter is built once per module load, not once per row.

Column width goes 52px -> 70px to fit flag + code + sort arrow; sortVal is
unchanged, so the column still sorts by code.

Verified against a live daemon: flags render in the Networks server list and
the Clients list, an empty country_code shows the dash alone, /flags/zz.png
falls back to the hidden image, the tooltip follows the EN/ES switch
(France/Francia), sorting works both directions, and the console stays clean.
got3nks added a commit to got3nks/amule that referenced this pull request Aug 2, 2026
…poll

amule-project#439 moved GeoIP resolution out of GUI paint and into the core. Paint hid
the cost: it ran only for visible rows, only while the client list was on
screen, only with a user watching. The EC client tag now resolves EVERY
peer on EVERY poll -- roughly every 3 s, headless, forever -- to produce a
value that cannot change while the peer's IP does not.

Each call formats the IP into a string (Uint32toStringIP allocates), then
MMDB_lookup_string parses it back into a sockaddr via getaddrinfo and
walks the database. Isolated on a live daemon by toggling IP2Country off
and on with no code change, so the two figures differ only by the lookup:
20.2us vs 12.6us per peer per poll for the client tag build. About 37% of
it, on the phase that scales with peer count.

Cache it in CIP2Country, keyed on the numeric IP the callers already
hold, so the string form is built only on a miss.

Invalidated on Enable and Disable, which between them also cover a
database refresh since DownloadFinished routes through both. That part is
load-bearing rather than defensive: GeoIP can be toggled at runtime, and
on a headless daemon an entry resolved while it was off would otherwise
stay empty forever -- there is no repaint to force a re-read and nobody
watching to notice.

Bounded at 8192 entries. Peers churn, so an unbounded map on a long-lived
daemon grows without limit; on overflow the whole map is dropped rather
than evicting by recency, which would slow the hit path -- the one that
matters -- to optimise a rare event. A rebuild costs one lookup per live
peer, exactly what a single poll cost before this existed.

The GUI paths take the cache too: GetDisplayCountryCode now takes the
numeric IP, so the monolithic per-paint lookups in GenericClientListCtrl
and ServerListCtrl stop going straight to the database. The client
accessor is GetFullIPNumeric rather than GetFullUserIP: the resolver
overloads on argument type, so a name one word from GetFullIP would let a
caller silently take the uncached path.

Also fixes an unrelated build break found while testing the stub path:
CServerListCtrl::FlagImage reaches theApp->GetCountryFlags(), which only
exists under GEOIP_GUI, but was compiled unconditionally -- so monolithic
aMule failed to build at -DENABLE_IP2COUNTRY=NO, in violation of the
feature-gating policy. Reproduced on unmodified master. Only the
definition is gated, not the declaration: GEOIP_GUI is #defined in
amule.h, which ServerListCtrl.cpp includes but ServerListCtrl.h does not,
so gating the header would compile the declaration out from under the
definition.
got3nks added a commit to got3nks/amule that referenced this pull request Aug 2, 2026
…poll (amule-project#771)

amule-project#439 moved GeoIP resolution out of GUI paint and into the core. Paint hid
the cost: it ran only for visible rows, only while the client list was on
screen, only with a user watching. The EC client tag now resolves EVERY
peer on EVERY poll -- roughly every 3 s, headless, forever -- to produce a
value that cannot change while the peer's IP does not.

Each call formats the IP into a string (Uint32toStringIP allocates), then
MMDB_lookup_string parses it back into a sockaddr via getaddrinfo and
walks the database. Isolated on a live daemon by toggling IP2Country off
and on with no code change, so the two figures differ only by the lookup:
20.2us vs 12.6us per peer per poll for the client tag build. About 37% of
it, on the phase that scales with peer count.

Cache it in CIP2Country, keyed on the numeric IP the callers already
hold, so the string form is built only on a miss.

Invalidated on Enable and Disable, which between them also cover a
database refresh since DownloadFinished routes through both. That part is
load-bearing rather than defensive: GeoIP can be toggled at runtime, and
on a headless daemon an entry resolved while it was off would otherwise
stay empty forever -- there is no repaint to force a re-read and nobody
watching to notice.

Bounded at 8192 entries. Peers churn, so an unbounded map on a long-lived
daemon grows without limit; on overflow the whole map is dropped rather
than evicting by recency, which would slow the hit path -- the one that
matters -- to optimise a rare event. A rebuild costs one lookup per live
peer, exactly what a single poll cost before this existed.

The GUI paths take the cache too: GetDisplayCountryCode now takes the
numeric IP, so the monolithic per-paint lookups in GenericClientListCtrl
and ServerListCtrl stop going straight to the database. The client
accessor is GetFullIPNumeric rather than GetFullUserIP: the resolver
overloads on argument type, so a name one word from GetFullIP would let a
caller silently take the uncached path.

Also fixes an unrelated build break found while testing the stub path:
CServerListCtrl::FlagImage reaches theApp->GetCountryFlags(), which only
exists under GEOIP_GUI, but was compiled unconditionally -- so monolithic
aMule failed to build at -DENABLE_IP2COUNTRY=NO, in violation of the
feature-gating policy. Reproduced on unmodified master. Only the
definition is gated, not the declaration: GEOIP_GUI is #defined in
amule.h, which ServerListCtrl.cpp includes but ServerListCtrl.h does not,
so gating the header would compile the declaration out from under the
definition.
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.

3 participants