From 755d9d460924ab0afdf287d6ff3a01ae701a657a Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Thu, 26 May 2022 04:53:15 +0200 Subject: [PATCH] [JVM-Packages] Auto-detection of MUSL is replaced by system properties (#7921) This PR removes auto-detection of MUSL-based Linux systems in favor of system properties the user can set to configure a specific path for a native library. --- .../dmlc/xgboost4j/java/NativeLibLoader.java | 113 ++++++++---------- .../java/LibraryPathProviderTest.java | 61 ++++++++++ .../dmlc/xgboost4j/java/OsDetectionTest.java | 33 +---- 3 files changed, 116 insertions(+), 91 deletions(-) create mode 100644 jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/LibraryPathProviderTest.java diff --git a/jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java b/jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java index f10bab924..322f77001 100644 --- a/jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java +++ b/jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java @@ -21,16 +21,14 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Locale; -import java.util.Optional; -import java.util.stream.Stream; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.LibraryPathProvider.getLibraryPathFor; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.LibraryPathProvider.getPropertyNameForLibrary; + /** * class to load native library * @@ -39,8 +37,6 @@ import org.apache.commons.logging.LogFactory; class NativeLibLoader { private static final Log logger = LogFactory.getLog(NativeLibLoader.class); - private static Path mappedFilesBaseDir = Paths.get("/proc/self/map_files"); - /** * Supported OS enum. */ @@ -48,7 +44,6 @@ class NativeLibLoader { WINDOWS("windows"), MACOS("macos"), LINUX("linux"), - LINUX_MUSL("linux-musl"), SOLARIS("solaris"); final String name; @@ -57,13 +52,10 @@ class NativeLibLoader { this.name = name; } - static void setMappedFilesBaseDir(Path baseDir) { - mappedFilesBaseDir = baseDir; - } - /** * Detects the OS using the system properties. * Throws IllegalStateException if the OS is not recognized. + * * @return The OS. */ static OS detectOS() { @@ -73,7 +65,7 @@ class NativeLibLoader { } else if (os.contains("win")) { return WINDOWS; } else if (os.contains("nux")) { - return isMuslBased() ? LINUX_MUSL : LINUX; + return LINUX; } else if (os.contains("sunos")) { return SOLARIS; } else { @@ -81,39 +73,6 @@ class NativeLibLoader { } } - /** - * Checks if the Linux OS is musl based. For this, we check the memory-mapped - * filenames and see if one of those contains the string "musl". - * - * @return true if the Linux OS is musl based, false otherwise. - */ - static boolean isMuslBased() { - try (Stream dirStream = Files.list(mappedFilesBaseDir)) { - Optional muslRelatedMemoryMappedFilename = dirStream - .map(OS::toRealPath) - .filter(s -> s.toLowerCase().contains("musl")) - .findFirst(); - - muslRelatedMemoryMappedFilename.ifPresent(muslFilename -> { - logger.debug("Assuming that detected Linux OS is musl-based, " - + "because a memory-mapped file '" + muslFilename + "' was found."); - }); - - return muslRelatedMemoryMappedFilename.isPresent(); - } catch (Exception ignored) { - // ignored - } - return false; - } - - private static String toRealPath(Path path) { - try { - return path.toRealPath().toString(); - } catch (IOException e) { - return ""; - } - } - } /** @@ -149,8 +108,43 @@ class NativeLibLoader { } } + /** + * Utility class to determine the path of a native library. + */ + static class LibraryPathProvider { + + private static final String nativeResourcePath = "/lib"; + private static final String customNativeLibraryPathPropertyPrefix = "xgboostruntime.native."; + + static String getPropertyNameForLibrary(String libName) { + return customNativeLibraryPathPropertyPrefix + libName; + } + + /** + * If a library-specific system property is set, this value is + * being used without further processing. + * Otherwise, the library path depends on the OS and architecture. + * + * @return path of the native library + */ + static String getLibraryPathFor(OS os, Arch arch, String libName) { + + String libraryPath = System.getProperty(getPropertyNameForLibrary(libName)); + + if (libraryPath == null) { + libraryPath = nativeResourcePath + "/" + + getPlatformFor(os, arch) + "/" + + System.mapLibraryName(libName); + } + + logger.debug("Using path " + libraryPath + " for library with name " + libName); + + return libraryPath; + } + + } + private static boolean initialized = false; - private static final String nativeResourcePath = "/lib"; private static final String[] libNames = new String[]{"xgboost4j"}; /** @@ -168,16 +162,14 @@ class NativeLibLoader { if (!initialized) { OS os = OS.detectOS(); Arch arch = Arch.detectArch(); - String platform = os.name + "/" + arch.name; for (String libName : libNames) { try { - String libraryPathInJar = nativeResourcePath + "/" + - platform + "/" + System.mapLibraryName(libName); + String libraryPathInJar = getLibraryPathFor(os, arch, libName); loadLibraryFromJar(libraryPathInJar); } catch (UnsatisfiedLinkError ule) { String failureMessageIncludingOpenMPHint = "Failed to load " + libName + " " + "due to missing native dependencies for " + - "platform " + platform + ", " + + "platform " + getPlatformFor(os, arch) + ", " + "this is likely due to a missing OpenMP dependency"; switch (os) { @@ -194,15 +186,9 @@ class NativeLibLoader { logger.error(failureMessageIncludingOpenMPHint); logger.error("You may need to install 'libgomp.so' (or glibc) via your package " + "manager."); - logger.error("Alternatively, your Linux OS is musl-based " + - "but wasn't detected as such."); - break; - case LINUX_MUSL: - logger.error(failureMessageIncludingOpenMPHint); - logger.error("You may need to install 'libgomp.so' (or glibc) via your package " + - "manager."); - logger.error("Alternatively, your Linux OS was wrongly detected as musl-based, " + - "although it is not."); + logger.error("Alternatively, if your Linux OS is musl-based, you should set " + + "the path for the native library " + libName + " " + + "via the system property " + getPropertyNameForLibrary(libName)); break; case SOLARIS: logger.error(failureMessageIncludingOpenMPHint); @@ -212,7 +198,8 @@ class NativeLibLoader { } throw ule; } catch (IOException ioe) { - logger.error("Failed to load " + libName + " library from jar for platform " + platform); + logger.error("Failed to load " + libName + " library from jar for platform " + + getPlatformFor(os, arch)); throw ioe; } } @@ -307,4 +294,8 @@ class NativeLibLoader { return temp.getAbsolutePath(); } + private static String getPlatformFor(OS os, Arch arch) { + return os.name + "/" + arch.name; + } + } diff --git a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/LibraryPathProviderTest.java b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/LibraryPathProviderTest.java new file mode 100644 index 000000000..95051eb6e --- /dev/null +++ b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/LibraryPathProviderTest.java @@ -0,0 +1,61 @@ +/* + Copyright (c) 2014 by Contributors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package ml.dmlc.xgboost4j.java; + +import org.junit.Test; + +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertTrue; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.X86_64; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.LibraryPathProvider.getLibraryPathFor; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.LINUX; + +public class LibraryPathProviderTest { + + @Test + public void testLibraryPathProviderUsesOsAndArchToResolvePath() { + String libraryPath = getLibraryPathFor(LINUX, X86_64, "someLibrary"); + + assertTrue(libraryPath.startsWith("/lib/linux/x86_64/")); + } + + @Test + public void testLibraryPathProviderUsesPropertyValueForPathIfPresent() { + String propertyName = "xgboostruntime.native.library"; + + executeAndRestoreProperty(propertyName, () -> { + System.setProperty(propertyName, "/my/custom/path/to/my/library"); + String libraryPath = getLibraryPathFor(LINUX, X86_64, "library"); + + assertEquals("/my/custom/path/to/my/library", libraryPath); + }); + } + + private static void executeAndRestoreProperty(String propertyName, Runnable action) { + String oldValue = System.getProperty(propertyName); + + try { + action.run(); + } finally { + if (oldValue != null) { + System.setProperty(propertyName, oldValue); + } else { + System.clearProperty(propertyName); + } + } + } + +} diff --git a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java index b8ca3d772..e3aeb3a62 100644 --- a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java +++ b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java @@ -16,10 +16,8 @@ package ml.dmlc.xgboost4j.java; import ml.dmlc.xgboost4j.java.NativeLibLoader.OS; -import org.junit.Rule; import org.junit.Test; import org.junit.experimental.runners.Enclosed; -import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; @@ -40,12 +38,12 @@ public class OsDetectionTest { private static final String OS_NAME_PROPERTY = "os.name"; @RunWith(Parameterized.class) - public static class ParameterizedOSDetectionTest { + public static class SupportedOSDetectionTest { private final String osNameValue; private final OS expectedOS; - public ParameterizedOSDetectionTest(String osNameValue, OS expectedOS) { + public SupportedOSDetectionTest(String osNameValue, OS expectedOS) { this.osNameValue = osNameValue; this.expectedOS = expectedOS; } @@ -70,32 +68,7 @@ public class OsDetectionTest { } } - public static class NonParameterizedOSDetectionTest { - - @Rule - public TemporaryFolder folder = new TemporaryFolder(); - - @Test - public void testForRegularLinux() throws Exception { - setMappedFilesBaseDir(folder.getRoot().toPath()); - folder.newFile("ld-2.23.so"); - - executeAndRestoreProperty(() -> { - System.setProperty(OS_NAME_PROPERTY, "linux"); - assertSame(detectOS(), LINUX); - }); - } - - @Test - public void testForMuslBasedLinux() throws Exception { - setMappedFilesBaseDir(folder.getRoot().toPath()); - folder.newFile("ld-musl-x86_64.so.1"); - - executeAndRestoreProperty(() -> { - System.setProperty(OS_NAME_PROPERTY, "linux"); - assertSame(detectOS(), LINUX_MUSL); - }); - } + public static class UnsupportedOSDetectionTest { @Test public void testUnsupportedOs() {