Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GHA: Integrate Alpine musl job in main workflow #4826

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kinke
Copy link
Member

@kinke kinke commented Jan 18, 2025

Incl. prebuilt package etc.

@kinke kinke force-pushed the gha_musl branch 8 times, most recently from fb64457 to 969402a Compare January 19, 2025 01:33
@kinke kinke changed the title GHA main: Add Alpine musl job GHA: Integrate Alpine musl job in main workflow Jan 19, 2025
@kinke kinke force-pushed the gha_musl branch 4 times, most recently from 3f95f9b to 83cab98 Compare January 19, 2025 03:08
@kinke kinke marked this pull request as ready for review January 19, 2025 04:17
@kinke kinke requested a review from JohanEngelen January 19, 2025 04:17
@kinke kinke force-pushed the gha_musl branch 3 times, most recently from 83539fc to c3f00ed Compare January 19, 2025 16:00
@kinke kinke force-pushed the gha_musl branch 3 times, most recently from c9122cd to 62905ee Compare January 19, 2025 18:11
…t.curl]

Incl. the Phobos test runner, and dub.
@JohanEngelen
Copy link
Member

cool that you are working on this!

@kinke kinke force-pushed the gha_musl branch 2 times, most recently from 0e98a19 to 2df74c9 Compare January 19, 2025 20:12
@kinke
Copy link
Member Author

kinke commented Jan 19, 2025

Okay, should be ready now, after lots of trial & error runs, as usual for these CI MRs. I guess adding a musl AArch64 job as follow-up would be pretty trivial.

Some notes:

  • Preliminary runs with Alpine v3.21 and LLVM 19 exhibited LTO problems IIRC. So sticking with v3.20 / LLVM 17 for now.
  • I couldn't find an Alpine package with static LLD libs. So no LDC_WITH_LLD / -link-internally support.
  • The bundled ldc-prof{data,gen} tools aren't linked with -static, so depend on musl .so. That would require more CMake fiddling.
  • std.net.curl support depends on loading libcurl dynamically, so doesn't work (at runtime) with fully static executables. So bundled dub and reggae depend on musl .so as well (not linked with -Xcc=-static).
    • The bundled ldc-build-runtime tool is fully static, so won't be able to download the source zip automatically. Again, would require more CMake fiddling.

@JohanEngelen
Copy link
Member

Very cool! Let's fix the static linking later.

@@ -58,8 +58,20 @@ void main(string[] args)
auto r = Runtime.unloadLibrary(h);
if (!r)
assert(0);
if (finalizeCounter != 4)
assert(0);
version (darwin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, replace darwin with OSX because it is more common throughout the source code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I know it's deprecated (unfortunately), but it's more convenient than the usual

version (OSX)
version = Darwin;
else version (iOS)
version = Darwin;
else version (TVOS)
version = Darwin;
else version (WatchOS)
version = Darwin;
which would be required for the other Apple platforms too. There are 3 existing version (darwin) usages for this shared druntime integration test, so I added the 4th one, out of laziness of course. ;)

-DCOMPILER_RT_LIBDIR_OS=linux
-DLDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH=x86_64
-DTEST_COMPILER_RT_LIBRARIES="profile;lsan;asan;msan;fuzzer"
-DCMAKE_EXE_LINKER_FLAGS=-static-libstdc++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps it is as simple as -DCMAKE_EXE_LINKER_FLAGS="-static-libstdc++;-static" to get fully statically linked ldc_profgen (cpp-only executables) ? I'm OK to leave as is for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that would include the 2 pure C++ LLVM tools, but unfortunately also the static Phobos test runner, where the std.net.curl unittests then fail...

Copy link
Member Author

@kinke kinke Jan 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And D_LINKER_ARGS apply to all D executables built via build_d_executable() (now for LDC_LINK_MANUALLY=OFF too, where it was previously ignored). And that unfortunately also includes the ldc-build-runtime tool. Edit: But no druntime/Phobos test runners, they are pure C executables (in CMake terms).

So I think we need a dedicated CMake option for fully-static linking (on Posix), adding -static for suited executables only (no ldc-build-runtime, but the PGO LLVM tools).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I've pushed a commit to that effect, and now all but dub/reggae/ldc-build-runtime executables are fully static.

@kinke
Copy link
Member Author

kinke commented Jan 19, 2025

Btw, I was surprised that I could link & run a Phobos-hello-world on my Ubuntu 24 box - with the bundled compiler, druntime and Phobos targeting a musl triple (edit: and my system cc for linking targeting glibc). No idea if that's just lucky or whether musl is a strict compatible subset of glibc API-wise (edit: well, the libc API required from druntime and Phobos at least).

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.

2 participants