Windows/MinGW build support: cmake fixes, LLP64 pointer safety, ASIO on Windows - #457
Conversation
Four pre-existing bugs in untested Windows CMake paths, plus a new install-time helper to produce a portable Windows bundle: - cmake/wx.cmake: the Windows branch assumed the MSVC-with-prebuilt- subfolder layout from platforms/Windows/MSVC12/README. MinGW installs wx via standard wx-config, so gate the MSVC-specific path with AND NOT MINGW and let MinGW fall through to the generic find_package/wx-config detection. - cmake/wx.cmake: on MinGW, wx-config points at the unicode wx build but does not emit -DUNICODE/-D_UNICODE, so wx/msw/winundef.h gets into an ANSI/UNICODE-mixed state with LPCTSTR/LPCSTR type mismatches. Propagate the defines via wxWidgets_DEFINITIONS in the Unix-style branch, mirroring how the MSVC branch already adds _UNICODE via CHECK_CXX_SYMBOL_EXISTS. - src/CMakeLists.txt: amuled's Win32 target_link_libraries for shlwapi.lib was missing the PRIVATE keyword, which can conflict with modern keyword-signature uses on the same target. - src/utils/cas/CMakeLists.txt: target_sources(cas ...) was missing the required PRIVATE/PUBLIC/INTERFACE specifier, which is a configure-time error in modern CMake. - src/CMakeLists.txt (new block): during cmake --install on MinGW, walk the installed executables' PE import tables via file(GET_RUNTIME_DEPENDENCIES) and copy every non-system DLL into the install prefix's bin/ directory. Produces a self-contained portable bundle without a hardcoded DLL list. Lives in src/CMakeLists.txt rather than the root so it runs after the executable install commands (CMake appends subdir install scripts at the end of the root cmake_install.cmake).
…okup
aMule has never been built on Windows x64 with a modern compiler, so a class
of pointer-truncation bugs that only matters on LLP64 platforms (Windows 64)
has accumulated. Linux and macOS are LP64 (long is 64-bit, same as a
pointer), so long-typed variables holding pointers work there by accident.
On Windows 64, long is 32-bit while pointers are 64-bit, and every site that
stored a pointer in a long silently truncated its upper half.
Symptom that triggered the investigation: sorting any CMuleListCtrl column
(search results, downloads, shared files, etc.) segfaults on Windows x64 as
soon as the comparator dereferences its sortData argument. Once fixed, the
same class of bug was also present in the "browse shared files of a remote
user" result-lookup path — it didn't crash, but it hashed clients by the
lower 32 bits of their pointer and would swap tabs if two clients happened
to collide.
Changes, all replacing long with the portable pointer-sized type
(wxUIntPtr for unsigned pointer-holding, wxIntPtr for the wx-style sortData
parameter) per the typedefs wx already defines for exactly this purpose:
- Sort callback type: MuleListCtrlCompare typedef and SortItems overloads
in the vendored wx listctrl fork (src/extern/wxWidgets/listctrl.{h,cpp}),
plus every CMuleListCtrl subclass's static SortProc (9 pairs of .h/.cpp
files: CommentDialogLst, DownloadListCtrl, FileDetailListCtrl,
GenericClientListCtrl, SearchListCtrl, ServerListCtrl, SharedFilePeersListCtrl,
SharedFilesCtrl, SourceListCtrl) — change the third parameter from
"long sortData" to "wxIntPtr sortData".
- Sort-data storage: the module-level list_ctrl_compare_data in
extern/wxWidgets/listctrl.cpp was "long" — this was the actual crash site,
because CMuleListCtrl::SortList() passes &MuleSortData as the sortData
argument and the 64-bit pointer was being truncated on the way in.
Change to wxIntPtr.
- Sort-iteration locals: MuleListCtrl::SortList()'s lastItemdata /
nextItemdata were "long" but are assigned from GetItemData(), which
returns wxUIntPtr. Change to wxUIntPtr.
- Search result map: CSearchList and CSearchListRem keyed their ResultMap
by "long" even though CSearchFile::m_searchID is already wxUIntPtr, and
the browse-shared-files flow casts a CUpDownClient* to wxUIntPtr and
then narrows it to long to use as the key. Change the map key,
the GetSearchResults/RemoveResults signatures, and the local searchID
variable to wxUIntPtr. Mirror in amule-remote-gui.{h,cpp}.
- CSearchListCtrl::ShowResults and its header declaration take "long
ResultsId" but m_nResultsID is already wxUIntPtr. Change to wxUIntPtr.
- ClientList.cpp: the "Not deleted client %x ..." debug log lines cast
pCurClient to "long int" and printed with %x, which truncated the
printed address on Windows x64. Use static_cast<const void*> with %p
so the full pointer prints on every platform. Cosmetic only — the lines
are inside #ifdef __DEBUG__.
Before: on MinGW, check_include_files("boost/asio.hpp" ...) ran a compile
AND link step. The link failed with "undefined reference to
WSAStartup/WSACleanup" because boost::asio on Windows pulls in WinSock2
and CMAKE_REQUIRED_LIBRARIES wasn't reliably honoured in this CMake +
CheckIncludeFiles combination. The failure was silent
(check_include_files just returns "not found"), so the build set
ENABLE_BOOST=FALSE and fell back to the legacy wxSocket backend.
Fix: find_package(Boost CONFIG REQUIRED) already verified boost is
usable, so the link-check is redundant on MinGW. Set ASIO_SOCKETS
directly and carry the WinSock libs (ws2_32, mswsock) through
Boost_LIBRARIES so the final targets link cleanly. MSVC keeps the
original check — auto-linking via #pragma comment(lib, ...) in the
Windows SDK headers means MSVC was never affected and doesn't need
ws2_32 in Boost_LIBRARIES. Linux/Mac also unchanged.
Only affects MinGW builds — no change for MSVC or non-Windows.
|
k, maybe you should slow down a bit, as things are mixing up. We really appreciate your motivation, but reviewing everything in that many PRs in that many different sections is hell.
|
|
Fair point on the pacing though for what it's worth, these open PRs have no cross-dependencies and each applies cleanly against master independently. I just wanted aMule to compile successfully on all platforms for now. On (1): the MinGW UNICODE line sits inside the legacy cmake/wx.cmake wx-config branch — MSYS2 wx-config currently doesn't emit -DUNICODE/-D_UNICODE. CMake's stock find_package(wxWidgets) does emit them correctly. So when On (2): check_include_files does compile + link. Linux/Mac links fine (asio's header init uses cross-platform symbols); MinGW needs WSAStartup from ws2_32 and CMAKE_REQUIRED_LIBRARIES isn't honoured in this combo → link silently fails. You're right that if find_package(Boost CONFIG REQUIRED) already passed, the redundant check can go on all platforms. I kept the Linux/Mac/MSVC path unchanged to minimize the scope of this PR, but happy to simplify. My next planned work is to bump |
Yep, but the more important question is, will asio still be needed or is wxSocket working as we need it in wx-3.2?
I guess this would be the right starting point, as everything would be revisited when this is done. |
Good question. Architecturally, wx-3.2's wxSocket is still single-threaded and select()-based, same design as wx-2.x. On the other hand, boost::asio uses native async primitives per platform (IOCP on Windows, epoll-ET on Linux, kqueue on BSD/Mac) and runs a 4-thread I/O pool. For a P2P client with many concurrent peers, that throughput/scalability gap is architectural, not a wx version issue. wx-3.3's new APIs (wxWebRequest) improved HTTP but didn't touch the general-purpose socket class. Happy to benchmark if that helps, but given wxSocket's design hasn't changed I expect asio is still needed. |
|
Would you mind adding a MacOS and a Windows build to ccpp.yml? I was in the middle of doing this, but our patches will colide, and you seem to be ahead of me. |
|
Here it is some inspiration to get started, if you want: The relevant branch is ci_macos. It is work in progress, but I see no point in continuing. |
Every current-stable Linux distro ships wx 3.2: Ubuntu 24.04 LTS
3.2.4, Debian 13 3.2.8, Fedora 42/43 3.2.8, Arch 3.2.10, openSUSE
Tumbleweed 3.2.8. configure.ac has also enforced wx >= 3.2 for a
while (the AS_IF on "$WX_VERSION_MAJOR""$WX_VERSION_MINOR" -lt 32
next to WX_CONFIG_CHECK), so the 2.8.12 floor in CMakeLists.txt and
in the WX_CONFIG_CHECK line has been nominal. Raising it to 3.2.0
unblocks retiring the custom cmake/wx.cmake wrapper.
* CMakeLists.txt: MIN_WX_VERSION 2.8.12 -> 3.2.0.
* configure.ac: WX_CONFIG_CHECK and the error-message comment bumped
to 3.2.0.
* cmake/wx.cmake: 311-line custom wrapper replaced with an ~80-line
thin shim over stock find_package(wxWidgets). The shim still
exposes wxWidgets::{BASE,CORE,NET,ADV} as INTERFACE IMPORTED
targets so existing target_link_libraries(... wxWidgets::CORE)
call sites across src/ don't change. What's gone:
- the WIN32 AND NOT MINGW MSVC-prebuilt-subfolder detection
(WX_BASE / WX_CORE / WX_ADV / WX_NET user vars);
- the manual wxWidgets_LIBRARIES -l parser that hand-rolled what
stock CMake's FindwxWidgets module already does;
- the MinGW -DUNICODE / -D_UNICODE propagation workaround from
PR amule-project#457 -- stock FindwxWidgets on MinGW calls wx-config which
already emits these as part of wxWidgets_DEFINITIONS;
- the wx 3.1.2 ADV-merged-into-CORE detection: since the minimum
is now 3.2.0, ADV is always merged, so 'adv' is not requested
from find_package (but wxWidgets::ADV is still created when
wx_NEED_ADV is set so existing generator expressions in
src/CMakeLists.txt keep resolving).
* src/MuleTextCtrl.cpp:100: data.GetTextLength() > 0 becomes
!data.GetText().IsEmpty(). wxTextDataObject::GetTextLength() is
[[deprecated]] in wx 3.3 ("Don't call nor override this
function"); GetText().IsEmpty() has the same semantics and is
supported on every wx version we care about.
* src/utils/fileview/Print.h: drop the now-always-true commented
#if wxCHECK_VERSION(2, 8, 4) dead block.
Every current-stable Linux distro ships wx 3.2: Ubuntu 24.04 LTS
3.2.4, Debian 13 3.2.8, Fedora 42/43 3.2.8, Arch 3.2.10, openSUSE
Tumbleweed 3.2.8. configure.ac has also enforced wx >= 3.2 for a
while (the AS_IF on "$WX_VERSION_MAJOR""$WX_VERSION_MINOR" -lt 32
next to WX_CONFIG_CHECK), so the 2.8.12 floor in CMakeLists.txt and
in the WX_CONFIG_CHECK line has been nominal. Raising it to 3.2.0
unblocks retiring the custom cmake/wx.cmake wrapper.
* CMakeLists.txt: MIN_WX_VERSION 2.8.12 -> 3.2.0.
* configure.ac: WX_CONFIG_CHECK and the error-message comment bumped
to 3.2.0.
* cmake/wx.cmake: 311-line custom wrapper replaced with an ~80-line
thin shim over stock find_package(wxWidgets). The shim still
exposes wxWidgets::{BASE,CORE,NET,ADV} as INTERFACE IMPORTED
targets so existing target_link_libraries(... wxWidgets::CORE)
call sites across src/ don't change. What's gone:
- the WIN32 AND NOT MINGW MSVC-prebuilt-subfolder detection
(WX_BASE / WX_CORE / WX_ADV / WX_NET user vars);
- the manual wxWidgets_LIBRARIES -l parser that hand-rolled what
stock CMake's FindwxWidgets module already does;
- the MinGW -DUNICODE / -D_UNICODE propagation workaround from
PR amule-project#457 -- stock FindwxWidgets on MinGW calls wx-config which
already emits these as part of wxWidgets_DEFINITIONS;
- the wx 3.1.2 ADV-merged-into-CORE detection: since the minimum
is now 3.2.0, ADV is always merged, so 'adv' is not requested
from find_package (but wxWidgets::ADV is still created when
wx_NEED_ADV is set so existing generator expressions in
src/CMakeLists.txt keep resolving).
* src/MuleTextCtrl.cpp:100: data.GetTextLength() > 0 becomes
!data.GetText().IsEmpty(). wxTextDataObject::GetTextLength() is
[[deprecated]] in wx 3.3 ("Don't call nor override this
function"); GetText().IsEmpty() has the same semantics and is
supported on every wx version we care about.
* src/utils/fileview/Print.h: drop the now-always-true commented
#if wxCHECK_VERSION(2, 8, 4) dead block.
|
@mrjimenez follow-up on your CI request: opened #458 adding macOS and Windows MinGW64 cmake build jobs to ccpp.yml (same BUILD_* matrix + ctest as the existing Ubuntu job). Built on your ci_macos branch as the starting point — thanks for the head start. |
Every current-stable Linux distro ships wx 3.2: Ubuntu 24.04 LTS
3.2.4, Debian 13 3.2.8, Fedora 42/43 3.2.8, Arch 3.2.10, openSUSE
Tumbleweed 3.2.8. configure.ac has also enforced wx >= 3.2 for a
while (the AS_IF on "$WX_VERSION_MAJOR""$WX_VERSION_MINOR" -lt 32
next to WX_CONFIG_CHECK), so the 2.8.12 floor in CMakeLists.txt and
in the WX_CONFIG_CHECK line has been nominal. Raising it to 3.2.0
unblocks retiring the custom cmake/wx.cmake wrapper.
* CMakeLists.txt: MIN_WX_VERSION 2.8.12 -> 3.2.0.
* configure.ac: WX_CONFIG_CHECK and the error-message comment bumped
to 3.2.0.
* cmake/wx.cmake: 311-line custom wrapper replaced with an ~90-line
thin shim over stock find_package(wxWidgets). The shim still
exposes wxWidgets::{BASE,CORE,NET,ADV} as INTERFACE IMPORTED
targets so existing target_link_libraries(... wxWidgets::CORE)
call sites across src/ don't change. What's gone:
- the WIN32 AND NOT MINGW MSVC-prebuilt-subfolder detection
(WX_BASE / WX_CORE / WX_ADV / WX_NET user vars);
- the manual wxWidgets_LIBRARIES -l parser that hand-rolled what
stock CMake's FindwxWidgets module already does;
- the wx 3.1.2 ADV-merged-into-CORE detection: since the minimum
is now 3.2.0, ADV is always merged, so 'adv' is not requested
from find_package (but wxWidgets::ADV is still created when
wx_NEED_ADV is set so existing generator expressions in
src/CMakeLists.txt keep resolving);
- the wxUSE_UNICODE CHECK_CXX_SYMBOL_EXISTS probe: wx 3.0+ is
unicode-only, so the probe is always true.
What's kept: the MinGW -DUNICODE / -D_UNICODE propagation
originally added in PR amule-project#457. MSYS2's wx-config points at the
unicode wx build but does not emit those defines, and stock
FindwxWidgets just forwards wx-config's output, so the shim
still has to set them explicitly on MinGW; without this the
MinGW build fails with LoadBitmapA/LPCSTR/LPCTSTR mismatches.
* src/MuleTextCtrl.cpp:100: data.GetTextLength() > 0 becomes
!data.GetText().IsEmpty(). wxTextDataObject::GetTextLength() is
[[deprecated]] in wx 3.3 ("Don't call nor override this
function"); GetText().IsEmpty() has the same semantics and is
supported on every wx version we care about.
* src/utils/fileview/Print.h: drop the now-always-true commented
#if wxCHECK_VERSION(2, 8, 4) dead block.
Excelent! Much cleaner than I would have done. Thanks! |
Every current-stable Linux distro ships wx 3.2: Ubuntu 24.04 LTS
3.2.4, Debian 13 3.2.8, Fedora 42/43 3.2.8, Arch 3.2.10, openSUSE
Tumbleweed 3.2.8. configure.ac has also enforced wx >= 3.2 for a
while (the AS_IF on "$WX_VERSION_MAJOR""$WX_VERSION_MINOR" -lt 32
next to WX_CONFIG_CHECK), so the 2.8.12 floor in CMakeLists.txt and
in the WX_CONFIG_CHECK line has been nominal. Raising it to 3.2.0
unblocks retiring the custom cmake/wx.cmake wrapper.
* CMakeLists.txt: MIN_WX_VERSION 2.8.12 -> 3.2.0.
* configure.ac: WX_CONFIG_CHECK and the error-message comment bumped
to 3.2.0.
* cmake/wx.cmake: 311-line custom wrapper replaced with an ~90-line
thin shim over stock find_package(wxWidgets). The shim still
exposes wxWidgets::{BASE,CORE,NET,ADV} as INTERFACE IMPORTED
targets so existing target_link_libraries(... wxWidgets::CORE)
call sites across src/ don't change. What's gone:
- the WIN32 AND NOT MINGW MSVC-prebuilt-subfolder detection
(WX_BASE / WX_CORE / WX_ADV / WX_NET user vars);
- the manual wxWidgets_LIBRARIES -l parser that hand-rolled what
stock CMake's FindwxWidgets module already does;
- the wx 3.1.2 ADV-merged-into-CORE detection: since the minimum
is now 3.2.0, ADV is always merged, so 'adv' is not requested
from find_package (but wxWidgets::ADV is still created when
wx_NEED_ADV is set so existing generator expressions in
src/CMakeLists.txt keep resolving);
- the wxUSE_UNICODE CHECK_CXX_SYMBOL_EXISTS probe: wx 3.0+ is
unicode-only, so the probe is always true.
What's kept: the MinGW -DUNICODE / -D_UNICODE propagation
originally added in PR #457. MSYS2's wx-config points at the
unicode wx build but does not emit those defines, and stock
FindwxWidgets just forwards wx-config's output, so the shim
still has to set them explicitly on MinGW; without this the
MinGW build fails with LoadBitmapA/LPCSTR/LPCTSTR mismatches.
* src/MuleTextCtrl.cpp:100: data.GetTextLength() > 0 becomes
!data.GetText().IsEmpty(). wxTextDataObject::GetTextLength() is
[[deprecated]] in wx 3.3 ("Don't call nor override this
function"); GetText().IsEmpty() has the same semantics and is
supported on every wx version we care about.
* src/utils/fileview/Print.h: drop the now-always-true commented
#if wxCHECK_VERSION(2, 8, 4) dead block.
3 participants
Summary
This PR makes aMule compile cleanly on Windows for both x86_64 and ARM64, via MSYS2 MinGW-w64 (GCC) and CLANGARM64 (Clang) toolchains respectively.
No effect on Linux/Mac —
ubuntu-latestCI stays green.Commits
cmake/windows: fix MinGW build + bundle MinGW runtime DLLs on install— 5 sub-fixes: MinGW wx path, UNICODE defines (inwx.cmake, mirroring the existing MSVC_UNICODElogic),shlwapi.libPRIVATE keyword,target_sources(cas)specifier, install-time DLL bundling viaGET_RUNTIME_DEPENDENCIES.windows: fix LLP64 pointer truncation in list sort + search result lookup— 27 files,long→wxUIntPtr/wxIntPtracross everyCMuleListCtrlsubclass, sort-data storage in the vendored wx listctrl, and theResultMapkey. All no-ops on LP64 (Linux/Mac).cmake/windows: enable ASIO_SOCKETS on MinGW— skip the brokenCheckIncludeFileslink step on MinGW, setASIO_SOCKETSdirectly, propagatews2_32/mswsockviaBoost_LIBRARIES. MSVC keeps the original check (auto-linksws2_32via#pragma comment). Linux/Mac unchanged.Build instructions (MinGW-w64 via MSYS2)
Tested on Windows 11 x86_64 and Windows 11 ARM64.
1. Install MSYS2 (one-time)
Install from https://www.msys2.org/ or
winget install MSYS2.MSYS2. Default path isC:\msys64. Open "MSYS2 MSYS" from the Start menu (white-themed terminal) and update the base system:2. x86_64 build (MinGW64)
Open "MSYS2 MINGW64" (blue-themed). Install toolchain + deps:
pacman -S --needed \ base-devel git \ mingw-w64-x86_64-toolchain \ mingw-w64-x86_64-cmake \ mingw-w64-x86_64-ninja \ mingw-w64-x86_64-ccache \ mingw-w64-x86_64-wxwidgets3.2-msw \ mingw-w64-x86_64-boost \ mingw-w64-x86_64-crypto++ \ mingw-w64-x86_64-zlib \ mingw-w64-x86_64-curl \ mingw-w64-x86_64-libpng \ mingw-w64-x86_64-readline \ mingw-w64-x86_64-gettextConfigure + build + install:
Check the configure summary for
ASIO_SOCKETSenabled (visible as theboostline in the final block) — without it the build silently falls back to the slow legacy wxSocket backend.Install output:
C:\amule-portable\bin\withamule.exe,amuled.exe,amulecmd.exe,ed2k.exe,alcc.exe, plus ~36 runtime DLLs (bundled automatically at install time). Fully portable — zip it or copy to another machine, noPATHsetup needed.Verify:
3. Native ARM64 build (CLANGARM64)
Only relevant on Windows 11 ARM64 hardware. Avoids Microsoft Prism x64→ARM64 emulation — native ARM64 binaries run ~2–3× faster on the same hardware. MSYS2's ARM64 environment uses Clang + LLD (no GCC for this target yet).
From any MSYS2 shell, install the toolchain + deps:
pacman -S --needed \ mingw-w64-clang-aarch64-toolchain \ mingw-w64-clang-aarch64-cmake \ mingw-w64-clang-aarch64-ninja \ mingw-w64-clang-aarch64-ccache \ mingw-w64-clang-aarch64-wxwidgets3.2-msw \ mingw-w64-clang-aarch64-boost \ mingw-w64-clang-aarch64-crypto++ \ mingw-w64-clang-aarch64-zlib \ mingw-w64-clang-aarch64-curl \ mingw-w64-clang-aarch64-libpng \ mingw-w64-clang-aarch64-readline \ mingw-w64-clang-aarch64-gettextOpen "MSYS2 CLANGARM64" (purple-themed terminal). Verify:
Same configure + build + install commands as the x86_64 step, just in a fresh
build-arm64/directory and installing to a different prefix:Verify:
file /c/amule-portable-arm64/bin/amule.exe # → PE32+ executable for MS Windows (GUI), ARM64The ARM64
amule.exeis ~40 % smaller (~4.6 MB vs ~7.8 MB on x86_64) — different instruction encoding + different codegen between Clang and GCC.Known exclusions
libupnpisn't currently packaged in MSYS2 for either target. Follow-up is either packaging it upstream or wiring it into CmDaB.Scripting the build (optional)
For CI or fresh-VM scripting, set
MSYSTEMexplicitly instead of relying on the Start-menu shortcuts:Replace
MSYSTEM=MINGW64withMSYSTEM=CLANGARM64for the ARM64 variant./usr/bin/bash -lis important — it sources/etc/profilewhich applies the env-specificPATH.Notes
platforms/Windows/are untouched — those targeted 32-bit Windows + wx 2.8, different scope.boost::asiobackend as Linux/Mac.