fix: Update LibNames to load candidates in order by chriswk · Pull Request #96 · Unleash/yggdrasil-bindings

chriswk · 2026-03-11T11:08:48Z

this makes it possible to build static graal musl images, which failed in #95

fixes: #95

Copilot

Pull request overview

Updates native library selection on Linux to better detect musl-based environments (notably statically linked GraalVM + Alpine), preventing the wrong .so from being loaded (fixes #95).

Changes:

Refactors LibNames to support libc overrides (system property/env), filesystem musl markers, and /proc/self/maps as a final fallback.
Adds focused unit tests for libc override and marker/fallback behavior.
Bumps yggdrasilCoreVersion and the Java engine version in gradle.properties.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
java-engine/src/main/java/io/getunleash/engine/LibNames.java	Implements ordered musl detection via overrides → marker paths → `/proc/self/maps` fallback.
java-engine/src/test/java/io/getunleash/engine/LibNamesTest.java	Adds unit tests validating selection for musl/glibc and marker/fallback behavior.
java-engine/gradle.properties	Updates core/native version and published Java engine version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T11:21:04Z

+    if (value == null) {
+      return null;
+    }
+
+    String normalized = value.trim().toLowerCase(Locale.ROOT);


parseLibcOverride writes directly to stderr for unsupported values. This library already uses SLF4J elsewhere, and writing to stderr can be noisy/uncontrollable for consumers (especially in serverless/Graal native images). Consider using the project logger (e.g., LOGGER.warn(...)) or silently ignoring unsupported values (or guarding logging behind a debug flag).

Copilot · 2026-03-11T11:21:04Z

+  }
+
+  private static boolean fileExists(String path) {


mapsContainMusl logs to stderr on any IOException reading /proc/self/maps. This can generate unexpected console output in restricted/containerized environments; prefer using the project logger at an appropriate level, or suppressing the warning (since falling back to false is already handled).

Copilot · 2026-03-11T11:21:05Z

+  @Test
+  void propertyOverrideWinsOverMuslMarkers() {
+    String libName = LibNames.pickFor("Linux", "amd64", "glibc", null, path -> true, () -> true);
+
+    assertEquals("libyggdrasilffi_x86_64.so", libName);
+  }


LibNames.pickFor now supports UNLEASH_ENGINE_LIBC (env override) and a defined precedence (system property first, then env). There isn’t a test covering the env override path (including that env is ignored when the property is set). Adding a couple of assertions here would help prevent regressions.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

java-engine/src/main/java/io/getunleash/engine/LibNames.java:156

mapsContainMusl() only catches IOException, but opening/reading /proc/self/maps can also throw SecurityException in restricted environments. That would propagate and fail native library selection/loading. Consider catching SecurityException (or RuntimeException) here as well and treating it the same as the IOException path.

  private static boolean mapsContainMusl() {
    try (BufferedReader reader = new BufferedReader(new FileReader("/proc/self/maps"))) {
      String line;
      while ((line = reader.readLine()) != null) {
        if (line.contains("musl")) {
          return true;
        }
      }
    } catch (IOException e) {
      System.err.println(
          "Warning: Failed to read /proc/self/maps, assuming this is not a musl system: "
              + e.getMessage());
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T13:39:30Z

+      System.err.println(
+          "Warning: Security manager blocked access to '"
+              + path
+              + "' while detecting libc. Assuming the file does not exist.");
+      return false;


fileExists prints a warning to System.err on SecurityException. Elsewhere in this module, warnings go through SLF4J (LOGGER.warn, e.g. FlatInterface.java:29). Please use the shared logging approach (or avoid printing from a utility method) to prevent unexpected stderr output in consumer applications.

Copilot · 2026-03-11T13:39:30Z

+   * <p>Supported values:
+   *
+   * <ul>
+   *   <li>{@code "musl"} – force musl libc selection
+   *   <li>{@code "glibc"} – force glibc selection
+   * </ul>
+   *
+   * Any other value (including {@code null}) will fall back to automatic detection.


The Javadoc for the libc override says only "musl" and "glibc" are supported, but parseLibcOverride also treats "gnu" and "gnu libc" as aliases for glibc. Please either document these aliases here (and in the env var Javadoc) or remove the alias handling so the docs and behavior match.

gastonfournier · 2026-03-11T14:27:49Z

+    "/usr/lib/ld-musl.so.1"
+  };
+
+  private static Boolean parseLibcOverride(String value) {


Can we simplify and return a boolean? Instead of null, we can return false?

Although this is coupled with isMusl... so this makes the whole thing a bit uneasy to read or understand...

I'd design parseLibcOverride to return null or a valid libc version (as an enum) or something else, cause now we're using a ternary result true, false, or null, which in the context of parseLibcOverride is not very clear what each response means without spending some time reading the code

I can have a look here. this is the least interesting thing, it's the extra bit AI added because it insisted it was better to have and not need it. Since we're running a library it isn't really needed, I might be able to just delete it.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T08:44:53Z

+
+  private static Optional<LibcVersion> parseLibcOverride(String value) {
+    if (value == null) {
+      return Optional.empty();
+    }
+
+    String normalized = value.trim().toLowerCase(Locale.ROOT);
+    switch (normalized) {
+      case "":
+      case "glibc":
+      case "gnu":
+      case "gnu libc":
+        return Optional.of(LibcVersion.Glibc);
+      case "musl":
+        return Optional.of(LibcVersion.Musl);
+    }


parseLibcOverride treats an empty/whitespace-only override value ("") as an explicit glibc override, but the Javadoc above states that any other value (including null) falls back to auto-detection. This can also cause surprising behavior if the property/env is set but empty. Consider treating empty/blank as "no override" (return Optional.empty()), or update the Javadoc to match the intended behavior.

gastonfournier

Looks fine, maybe https://github.com/Unleash/yggdrasil-bindings/pull/96/changes#r2923105429 is intentional, and https://github.com/Unleash/yggdrasil-bindings/pull/96/changes#r2917705839 is cosmetic, but I remember @sighphyre wanted to do some testing, let's make sure we're aligned.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T13:50:59Z

+  /**
+   * Environment variable used to override libc detection.
+   *
+   * <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"}


In the LIBC_ENV Javadoc, the supported-values list is missing punctuation between "glibc" and "gnu", which makes the sentence read oddly. Consider adding a comma/"and" so the list is unambiguous.

Suggested change

* <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"}

* <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"},

this makes it possible to build static graal musl images, which failed in #95 fixes: #95

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T14:15:03Z

+  static LibcVersion libcVersion(
+      String libcProperty,
+      String libcEnv,
+      Predicate<String> fileExists,
+      BooleanSupplier mapsContainMusl) {


libcVersion(...) is declared with package-private visibility but returns the private nested type LibcVersion. This makes the method effectively unusable outside LibNames and is confusing API surface; consider making libcVersion private (or widening LibcVersion visibility if it’s intentionally part of the package API).

Copilot · 2026-03-12T14:15:03Z

    } else if (os.contains("win")) {
-      if (arch.contains("arm64")) return "yggdrasilffi_arm64.dll";
-      if (arch.contains("64")) return "yggdrasilffi_x86_64.dll";
+      if (arch.contains("arm64")) {
+        return "yggdrasilffi_arm64.dll";
+      }
+      if (arch.contains("64")) {
+        return "yggdrasilffi_x86_64.dll";
+      }


Windows ARM64 detection only checks for "arm64", but os.arch is commonly "aarch64" on Windows ARM. In that case this will fall through to the arch.contains("64") branch and incorrectly pick the x86_64 DLL. Consider treating aarch64 as ARM64 here (consistent with the macOS/Linux branches).

Copilot · 2026-03-12T14:15:04Z

+   * <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"}
+   * {@code "gnu"}, {@code "gnu libc"}. Any other value (including unset) will fall back to
+   * automatic detection.


The Javadoc list of supported values for the env override is hard to read because the inline {@code ...} values are split across lines without clear separators. Consider converting it to a bullet list (like the property Javadoc above) or adding clear punctuation so it renders unambiguously.

Suggested change

* <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"}

* {@code "gnu"}, {@code "gnu libc"}. Any other value (including unset) will fall back to

* automatic detection.

* <p>Supported values are the same as for {@link #LIBC_PROPERTY}:

*

* <ul>

* <li>{@code "musl"} – force musl libc selection

* <li>{@code "glibc"} – force glibc selection

* <li>{@code "gnu"} - alias for glibc

* <li>{@code "gnu libc"} - alias for glibc

* </ul>

*

* Any other value (including unset) will fall back to automatic detection.

Copilot · 2026-03-12T14:15:04Z

+  void propertyOverrideWorksWithAliases() {
+    String libcProperty = LibNames.pickFor("Linux", "amd64", "gnu", null, path -> true, () -> true);
+    assertEquals("libyggdrasilffi_x86_64.so", libcProperty);
+    String libcEnv = LibNames.pickFor("Linux", "amd64", null, "gnu", path -> true, () -> true);
+    assertEquals("libyggdrasilffi_x86_64.so", libcEnv);


This test file isn’t formatted according to the repo’s Spotless/Google Java Format settings (several long statements remain on a single line). Running ./gradlew :java-engine:spotlessApply (or reformatting) should wrap these lines and avoid spotlessCheck failures in CI.

Copilot · 2026-03-12T14:15:04Z

+    String libcProperty = LibNames.pickFor("Linux", "amd64", "gnu", null, path -> true, () -> true);
+    assertEquals("libyggdrasilffi_x86_64.so", libcProperty);
+    String libcEnv = LibNames.pickFor("Linux", "amd64", null, "gnu", path -> true, () -> true);
+    assertEquals("libyggdrasilffi_x86_64.so", libcEnv);
+  }
+
+  @Test
+  void gnuLibcIsAlsoAValidAlias() {
+    String libcProperty =
+        LibNames.pickFor("Linux", "amd64", "gnu libc", null, path -> true, () -> true);
+    assertEquals("libyggdrasilffi_x86_64.so", libcProperty);
+    String libcEnv = LibNames.pickFor("Linux", "amd64", null, "gnu libc", path -> true, () -> true);


Additional long lines here are also likely to violate the project’s Google Java Format enforcement (Spotless). Reformatting the file should wrap these calls consistently with the rest of the codebase.

Suggested change

String libcProperty = LibNames.pickFor("Linux", "amd64", "gnu", null, path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcProperty);

String libcEnv = LibNames.pickFor("Linux", "amd64", null, "gnu", path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcEnv);

}

@Test

void gnuLibcIsAlsoAValidAlias() {

String libcProperty =

LibNames.pickFor("Linux", "amd64", "gnu libc", null, path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcProperty);

String libcEnv = LibNames.pickFor("Linux", "amd64", null, "gnu libc", path -> true, () -> true);

String libcProperty =

LibNames.pickFor(

"Linux", "amd64", "gnu", null, path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcProperty);

String libcEnv =

LibNames.pickFor(

"Linux", "amd64", null, "gnu", path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcEnv);

}

@Test

void gnuLibcIsAlsoAValidAlias() {

String libcProperty =

LibNames.pickFor(

"Linux", "amd64", "gnu libc", null, path -> true, () -> true);

assertEquals("libyggdrasilffi_x86_64.so", libcProperty);

String libcEnv =

LibNames.pickFor(

"Linux", "amd64", null, "gnu libc", path -> true, () -> true);

Copilot AI review requested due to automatic review settings March 11, 2026 11:08

thomasheartman added this to Issues and PRs Mar 11, 2026

github-project-automation Bot moved this to New in Issues and PRs Mar 11, 2026

Copilot started reviewing on behalf of chriswk March 11, 2026 11:09 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Comment thread java-engine/src/main/java/io/getunleash/engine/LibNames.java Outdated

Comment thread java-engine/src/main/java/io/getunleash/engine/LibNames.java

Comment thread java-engine/src/main/java/io/getunleash/engine/LibNames.java Outdated

chriswk requested a review from sighphyre March 11, 2026 11:17

chriswk self-assigned this Mar 11, 2026

Copilot AI review requested due to automatic review settings March 11, 2026 11:17

Copilot started reviewing on behalf of chriswk March 11, 2026 11:18 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 11, 2026 13:06

Copilot started reviewing on behalf of chriswk March 11, 2026 13:07 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Comment thread java-engine/src/main/java/io/getunleash/engine/LibNames.java

Copilot AI review requested due to automatic review settings March 11, 2026 13:32

Copilot started reviewing on behalf of chriswk March 11, 2026 13:34 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

gastonfournier reviewed Mar 11, 2026

View reviewed changes

gastonfournier moved this from New to In Progress in Issues and PRs Mar 12, 2026

chriswk force-pushed the graal-musl-static-load branch from d826c13 to a380b24 Compare March 12, 2026 08:37

Copilot AI review requested due to automatic review settings March 12, 2026 08:42

Copilot started reviewing on behalf of chriswk March 12, 2026 08:42 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

gastonfournier approved these changes Mar 12, 2026

View reviewed changes

github-project-automation Bot moved this from In Progress to Approved PRs in Issues and PRs Mar 12, 2026

chriswk mentioned this pull request Mar 12, 2026

task: Added separate build validator #97

Open

Copilot AI review requested due to automatic review settings March 12, 2026 13:47

Copilot started reviewing on behalf of chriswk March 12, 2026 13:47 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

chriswk and others added 2 commits March 12, 2026 15:08

fix: Update LibNames to load candidates in order

b9c88cc

this makes it possible to build static graal musl images, which failed in #95 fixes: #95

Update java-engine/src/main/java/io/getunleash/engine/LibNames.java

3b0f425

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

chriswk and others added 6 commits March 12, 2026 15:08

Update java-engine/src/main/java/io/getunleash/engine/LibNames.java

3f0effe

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

chore: spotless apply

c1b0c6a

commit: Add test verifying environment override vs property

5105f05

make libcVersion use Optional and enum to simplify reading

aea5535

added tests for property override using alias

0fda2c6

chore: fmt

991f13a

chriswk force-pushed the graal-musl-static-load branch from 996adbb to 991f13a Compare March 12, 2026 14:08

spotless

4608f84

Copilot AI review requested due to automatic review settings March 12, 2026 14:08

Copilot started reviewing on behalf of chriswk March 12, 2026 14:11 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

chriswk merged commit 0a326e7 into main Mar 12, 2026
7 checks passed

chriswk deleted the graal-musl-static-load branch March 12, 2026 14:29

github-project-automation Bot moved this from Approved PRs to Done in Issues and PRs Mar 12, 2026

sighphyre mentioned this pull request Mar 17, 2026

fix: revert extended musl detection #99

Merged

	* <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"}
	* <p>Supported values are the same as for {@link #LIBC_PROPERTY}: {@code "musl"}, {@code "glibc"},

Conversation

chriswk commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

gastonfournier Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

chriswk Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

gastonfournier left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026