-
Notifications
You must be signed in to change notification settings - Fork 249
feat: add support to x64 systems using musl #632
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
Conversation
|
Hey @xerial! This is a pull request that gives compatibility with systems using musl. Can you please take a look at this? Thanks! |
|
@samyuh It seems building a native library for Alipine is failing on CI. https://github.com/xerial/snappy-java/actions/runs/12839320244/job/36630155639?pr=632 |
|
Hey @xerial thanks for the feedback! I will try to make time to look into this later today :) |
Pipelines should pass now, I just renamed one file and forgot to change it on |
|
Hey @xerial |
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces support for musl-based x64 systems. The changes include updates to the build system and runtime detection logic in Java. The review focuses on improving maintainability of the build scripts, ensuring the correctness of the OSGi configuration, and enhancing the robustness of the Java code.
build.sbt
Outdated
| "org/xerial/snappy/native/Linux/ppc64/libsnappyjava.so;osname=linux;processor=ppc64le", | ||
| "org/xerial/snappy/native/Linux/s390x/libsnappyjava.so;osname=linux;processor=s390x", | ||
| "org/xerial/snappy/native/Linux/loongarch64/libsnappyjava.so;osname=linux;processor=loongarch64", | ||
| "org/xerial/snappy/native/Linux/x86_64-musl/libsnappyjava.so;osname=Linux;processor=x86-64-musl", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The processor value x86-64-musl in the OSGi Bundle-NativeCode entry is not a standard processor architecture that an OSGi framework would recognize. On an Alpine x86-64 system, the os.arch property is typically x86_64, which the OSGi framework maps to processor aliases like x86-64 or amd64. It's unlikely to match x86-64-musl. This could result in the OSGi framework either not extracting any native library or extracting the glibc-based one, which would fail to load. This could break functionality in OSGi environments on Alpine.
| Linux-x86_64-musl_CXX := $(CROSS_PREFIX)g++ | ||
| Linux-x86_64-musl_STRIP := $(CROSS_PREFIX)strip | ||
| ifeq ($(IBM_JDK_7),) | ||
| Linux-x86_64-musl_CXXFLAGS := -Ilib/inc_linux -I$(JAVA_HOME)/include -O2 -fPIC -fvisibility=hidden -m64 -std=c++11 | ||
| else | ||
| Linux-x86_64-musl_CXXFLAGS := -include $(IBM_JDK_LIB)/jni_md.h -include $(IBM_JDK_LIB)/jniport.h -I$(JAVA_HOME)/include -O2 -fPIC -fvisibility=hidden -m64 -std=c++11 | ||
| endif | ||
| Linux-x86_64-musl_LINKFLAGS := -shared -static-libgcc -static-libstdc++ | ||
| Linux-x86_64-musl_LIBNAME := libsnappyjava.so | ||
| Linux-x86_64-musl_SNAPPY_FLAGS := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build configuration for Linux-x86_64-musl is very similar to the existing Linux-x86_64 configuration. This duplication can make future maintenance more difficult, as changes would need to be applied in two places. Consider refactoring the common settings into a reusable block to reduce redundancy.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Changed processor=x86-64-musl to processor=x86-64 in Bundle-NativeCode as x86-64-musl is not a valid OSGi processor value. The OSGi spec only recognizes x86-64 (and aliases: amd64, em64t, x86_64) for 64-bit x86. Also fixed osname=Linux to osname=linux for consistency with other entries. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed improper indentation of catch statement at line 237 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Problem
When importing a data source of a file that was previously compressed using the snappy compression algorithm, the compression fails with the following exception:
This problem occurs since the dockerimage does not containg the
ld-linux-x86-64.so.2lib, which is from glibc. Alpine images use musl making this incompatible.Having this dependency being linked dynamically can mess things up. Ideally, we would like to have snappy being compiled against a MUSL image.
Snappy removed pure-java mode 1.1.9.0 (https://github.com/xerial/snappy-java/pull/381/files), which could be used to run snappy in non-supported systems with the use of one flag. We could force an older version but it was removed due to some problems (such as data corruption) and older versions than 1.1.9.0 contain critical CVEs (https://mvnrepository.com/artifact/org.xerial.snappy/snappy-java).
Workaround
Alpine images also have some packages that allow us to run binaries precompiled against glibc libraries (they don't allow us to compile against it), such as gcompat. In our case,
libc6-compatprovides/lib64/ld-linux-x86-64.so.2compatible lib. Turns out that/lib64/ld-linux-x86-64.so.2is a symlink to/lib/libc.musl-x86_64.so.1. Adding a/lib/ld-linux-x86-64.so.2linked to/lib/ld-musl-x86_64.so.1solves the issue since that is the place snappy is searching for the dependency.This is the same solution employed by https://stackoverflow.com/a/55568352.
Fix
The library now includes native binaries compiled specifically for musl environments, eliminating the need for workarounds like installing
gcompatorlibc6-compatpackages in Alpine containers.This improvement means:
Closes #616
Closes #239