Support non-default libc #1727

Flamefire · 2025-01-01T17:49:30Z

E.g. the musl libc doesn't support all features of glibc so our dev-tools don't work there.
--> Exclude them by default and adapt readme

Also the backtrace function might be in a separate library outside libc
--> Use a CMake find module

Finally configure a header for the used method as the header defining backtrace might be changed.

I also updated the GHA CI configs to use latest version of actions and less steps to view them more easily.

Requires:

Allow to more easily override the library locations dev-tools#7

Fixes #1725

Tested with CMake 3.25 locally and 3.31 on CI. Avoids: > CMake Deprecation Warning at CMakeLists.txt:5 (cmake_minimum_required): Compatibility with CMake < 3.10 will be removed from a future version of CMake.

Set CMAKE_SYSTEM_* instead of e.g. CMAKE_PREFIX_PATH as those will be searched later. Appending will search ours AFTER the users location(s).

Configure a header for determining which "backtrace" functionality is to be used. For `backtrace` it might be required to link libexecinfo, e.g. with musl libc. Use the CMake find module to search for that.

Use most recent actions Remove use of TRAVIS env variable

- Set all env vars in a single step - Do a single `apt install` - Remove outdated clang-16 setup step

5.4 is not supported by kaguya yet an older versions are not available.

stefson · 2025-01-02T20:21:53Z

I can test the branch on the weekend, ping me if you need to know anything specific.

Flamefire · 2025-01-03T09:00:45Z

I think this is ready now. So please leave a comment/review once tested.

.github/workflows/static-analysis.yml

CMakeLists.txt

libs/s25main/CMakeLists.txt

libs/s25main/Debug.cpp

Failure needs to be handled on non-MSVC Windows too so make the signatures consistent.

Flow86 · 2025-01-04T08:54:54Z

libs/s25main/Debug.cpp

@@ -193,14 +184,12 @@ DebugInfo::~DebugInfo()

 std::vector<void*> DebugInfo::GetStackTrace(void* ctx) noexcept
 {
+#ifndef RTTR_CONTEXT_PTR_TYPE
+    using RTTR_CONTEXT_PTR_TYPE = void*;
+#endif
    std::vector<void*> stacktrace(256);


hmm, what I recall is we probably should change this to a compile time storage (e.g std::array) since std::vector may not be able to allocate due to corruption/crash.

True but it did work so far and we would likely run into the same issue in other places, e.g. with std::stringstream

I did convert this vector to a boost::static_vector but I haven't found a static_allocator or so in Boost so that change would be larger and not suitable for this PR.

Avoid a dynamic memory allocation which may fail

stefson · 2025-01-05T06:19:10Z

it seems to compile just fine, both with gcc-13 and clang-19:

ldd /usr/bin/s25client

	/lib/ld-musl-armhf.so.1 (0xf7932000)
	libboost_program_options.so.1.85.0 => /usr/lib/libboost_program_options.so.1.85.0 (0xf731d000)
	libminiupnpc.so.18 => /usr/lib/libminiupnpc.so.18 (0xf730f000)
	libc.so => /lib/ld-musl-armhf.so.1 (0xf7932000)
	libboost_nowide.so.1.85.0 => /usr/lib/libboost_nowide.so.1.85.0 (0xf730a000)
	liblua5.3.so.0 => /usr/lib/liblua5.3.so.0 (0xf72d8000)
	libboost_filesystem.so.1.85.0 => /usr/lib/libboost_filesystem.so.1.85.0 (0xf72b7000)
	libbz2.so.1 => /usr/lib/libbz2.so.1 (0xf72a7000)
	libboost_iostreams.so.1.85.0 => /usr/lib/libboost_iostreams.so.1.85.0 (0xf7296000)
	libsamplerate.so.0 => /usr/lib/libsamplerate.so.0 (0xf7129000)
	libstdc++.so.6 => /usr/lib/gcc/armv7a-unknown-linux-musleabihf/13/libstdc++.so.6 (0xf6f12000)
	libgcc_s.so.1 => /usr/lib/gcc/armv7a-unknown-linux-musleabihf/13/libgcc_s.so.1 (0xf6f07000)
	libboost_atomic.so.1.85.0 => /usr/lib/libboost_atomic.so.1.85.0 (0xf6eff000)
	libz.so.1 => /usr/lib/libz.so.1 (0xf6eea000)

I will do some additional runtime testing, because musl can create some unforeseen issues with localization. Results propably tomorrow.

stefson · 2025-01-08T17:18:50Z

so I did some runtime testing on a musl system: the system language is not honored and it drops back to the english default (as expected), but you can correct that by setting it to a language of your choice in the menue.

I found one bug: in the Campaigns menue, there is an error about the world campaign being unloadable, so there is only the roman campaign available.

the log states:

Lade "/usr/share/s25rttr/RTTR/MUSIC/S2_Standard.pll"
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/EUROPE.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/NAMERICA.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/SAMERICA.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/GREEN.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/AFRICA.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/NASIA.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/SASIA.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/JAPAN.lua" does not exist.
Campaign map lua file "/usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/AUSTRA.lua" does not exist.

the files do exist on the disk, but the paths are all in non-capital letters. So the real path is:
/usr/share/s25rttr/RTTR/campaigns/world/EUROPE.lua

this is no issue if I compile the same musl branch on my linux x86_64 system with glibc.

Flamefire · 2025-01-09T09:31:33Z

so I did some runtime testing on a musl system: the system language is not honored and it drops back to the english default (as expected)

Why is that? How is the system language set there? Care to open a PR with an enhancement?

I found one bug: in the Campaigns menue, there is an error about the world campaign being unloadable, so there is only the roman campaign available.

That is an unrelated issue caused by a wrong spelling of the path in the lua file. Fix: #1730

this is no issue if I compile the same musl branch on my linux x86_64 system with glibc.

I can't see how. The lua file sets the path to /usr/share/s25rttr/RTTR/CAMPAIGNS/WORLD/ which due to Linux being case-sensitive should not exist

Flamefire force-pushed the musl branch 4 times, most recently from ec81598 to 537ed90 Compare January 1, 2025 18:44

Flamefire added 2 commits January 2, 2025 11:40

Add max tested CMake version

1411365

Tested with CMake 3.25 locally and 3.31 on CI. Avoids: > CMake Deprecation Warning at CMakeLists.txt:5 (cmake_minimum_required): Compatibility with CMake < 3.10 will be removed from a future version of CMake.

Update updater

cb3dccc

Flamefire force-pushed the musl branch 8 times, most recently from 23ec351 to d06402f Compare January 2, 2025 16:07

Flamefire added 6 commits January 2, 2025 18:10

Update libutil module

a655e70

Do not include dev-tools by default

afb51c6

Allow to more easily override the dev-tools library locations

986aa17

Set CMAKE_SYSTEM_* instead of e.g. CMAKE_PREFIX_PATH as those will be searched later. Appending will search ours AFTER the users location(s).

Link libexecinfo required for backtrace function if required

076daff

Configure a header for determining which "backtrace" functionality is to be used. For `backtrace` it might be required to link libexecinfo, e.g. with musl libc. Use the CMake find module to search for that.

Use dev-tools in Jenkins and Windows CI

2659d64

Update GHA CI

4282b9b

Use most recent actions Remove use of TRAVIS env variable

Flamefire force-pushed the musl branch 3 times, most recently from 98e727c to e13ba4f Compare January 2, 2025 17:39

Flamefire mentioned this pull request Jan 2, 2025

Replace (semi-)absolute includes to implementation headers by relative ones boostorg/locale#253

Merged

Flamefire added 5 commits January 2, 2025 19:49

Cleanup GHA configs

6dd3938

- Set all env vars in a single step - Do a single `apt install` - Remove outdated clang-16 setup step

GHA: Combine system package installation for OSX & Linux

cf3c7df

GHA: Install lua via homebrew on OSX

facc722

GHA: Update for homebrew changes on OSX

53fd4f3

GHA: Don't use HomeBrew Lua on OSX

f7d0e51

5.4 is not supported by kaguya yet an older versions are not available.

Flamefire force-pushed the musl branch from e13ba4f to f7d0e51 Compare January 2, 2025 18:49

Flamefire requested a review from Flow86 January 3, 2025 09:00

Flow86 requested changes Jan 3, 2025

View reviewed changes

.github/workflows/static-analysis.yml Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

libs/s25main/CMakeLists.txt Show resolved Hide resolved

libs/s25main/Debug.cpp Outdated Show resolved Hide resolved

stefson mentioned this pull request Jan 3, 2025

s25rttr fails to compile with: ../include/kaguya/detail/lua_function_def.hpp:165:18: error: no matching function for call to 'lua_resume' #1729

Closed

Fix return value of captureBacktrace

fd1d563

Failure needs to be handled on non-MSVC Windows too so make the signatures consistent.

Flamefire force-pushed the musl branch from ebc01d5 to fd1d563 Compare January 3, 2025 19:07

Flow86 reviewed Jan 4, 2025

View reviewed changes

Use a vector with static storage for the backtrace

9e47c42

Avoid a dynamic memory allocation which may fail

Flamefire force-pushed the musl branch from a70e8d2 to 9e47c42 Compare January 4, 2025 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support non-default libc #1727

Support non-default libc #1727

Flamefire commented Jan 1, 2025 •

edited

Loading

stefson commented Jan 2, 2025

Flamefire commented Jan 3, 2025

Flow86 Jan 4, 2025

Flamefire Jan 4, 2025

stefson commented Jan 5, 2025

stefson commented Jan 8, 2025

Flamefire commented Jan 9, 2025

Support non-default libc #1727

Are you sure you want to change the base?

Support non-default libc #1727

Conversation

Flamefire commented Jan 1, 2025 • edited Loading

stefson commented Jan 2, 2025

Flamefire commented Jan 3, 2025

Flow86 Jan 4, 2025

Choose a reason for hiding this comment

Flamefire Jan 4, 2025

Choose a reason for hiding this comment

stefson commented Jan 5, 2025

stefson commented Jan 8, 2025

Flamefire commented Jan 9, 2025

Flamefire commented Jan 1, 2025 •

edited

Loading