From 078fbbbae51982e5dcc339a8919a19b820b9ce2f Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 1 Feb 2022 14:26:05 +0100 Subject: [PATCH 01/12] Add unit tests for detecting the OS and its architecture --- jvm-packages/xgboost4j/pom.xml | 5 ++ .../xgboost4j/java/ArchDetectionTest.java | 85 +++++++++++++++++++ .../dmlc/xgboost4j/java/OsDetectionTest.java | 85 +++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java create mode 100644 jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java diff --git a/jvm-packages/xgboost4j/pom.xml b/jvm-packages/xgboost4j/pom.xml index c911eaa6ebc6..06a219be068b 100644 --- a/jvm-packages/xgboost4j/pom.xml +++ b/jvm-packages/xgboost4j/pom.xml @@ -31,6 +31,11 @@ 4.13.1 test + + com.github.stefanbirkner + system-lambda + 1.2.1 + com.typesafe.akka akka-actor_${scala.binary.version} diff --git a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java new file mode 100644 index 000000000000..f610c2b83c39 --- /dev/null +++ b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java @@ -0,0 +1,85 @@ +/* + 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 org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import java.util.Collection; + +import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties; +import static java.util.Arrays.asList; +import static junit.framework.TestCase.assertSame; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.X86_64; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.AARCH64; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.SPARC; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.detectArch; +import static org.junit.Assert.assertThrows; + +/** + * Test cases for {@link NativeLibLoader.Arch}. + */ +@RunWith(Enclosed.class) +public class ArchDetectionTest { + + private static final String OS_ARCH_PROPERTY = "os.arch"; + + @RunWith(Parameterized.class) + public static class ParameterizedArchDetectionTest { + + private final String osArchValue; + private final NativeLibLoader.Arch expectedArch; + + public ParameterizedArchDetectionTest(String osArchValue, NativeLibLoader.Arch expectedArch) { + this.osArchValue = osArchValue; + this.expectedArch = expectedArch; + } + + @Parameters + public static Collection data() { + return asList(new Object[][]{ + {"x86_64", X86_64}, + {"amd64", X86_64}, + {"aarch64", AARCH64}, + {"arm64", AARCH64}, + {"sparc64", SPARC} + }); + } + + @Test + public void testArch() throws Exception { + restoreSystemProperties(() -> { + System.setProperty(OS_ARCH_PROPERTY, osArchValue); + assertSame(detectArch(), expectedArch); + }); + } + } + + public static class UnsupportedArchDetectionTest { + + @Test + public void testUnsupportedArch() throws Exception { + restoreSystemProperties(() -> { + System.setProperty(OS_ARCH_PROPERTY, "unsupported"); + assertThrows(IllegalStateException.class, NativeLibLoader.Arch::detectArch); + }); + } + } + +} 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 new file mode 100644 index 000000000000..4a69787028c0 --- /dev/null +++ b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/OsDetectionTest.java @@ -0,0 +1,85 @@ +/* + 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 org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import java.util.Collection; + +import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties; +import static java.util.Arrays.asList; +import static junit.framework.TestCase.assertSame; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.WINDOWS; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.MACOS; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.LINUX; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.SOLARIS; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.detectOS; +import static org.junit.Assert.assertThrows; + +/** + * Test cases for {@link NativeLibLoader.OS}. + */ +@RunWith(Enclosed.class) +public class OsDetectionTest { + + private static final String OS_NAME_PROPERTY = "os.name"; + + @RunWith(Parameterized.class) + public static class ParameterizedOSDetectionTest { + + private final String osNameValue; + private final NativeLibLoader.OS expectedOS; + + public ParameterizedOSDetectionTest(String osNameValue, NativeLibLoader.OS expectedOS) { + this.osNameValue = osNameValue; + this.expectedOS = expectedOS; + } + + @Parameters + public static Collection data() { + return asList(new Object[][]{ + {"windows", WINDOWS}, + {"mac", MACOS}, + {"darwin", MACOS}, + {"linux", LINUX}, + {"sunos", SOLARIS} + }); + } + + @Test + public void getOS() throws Exception { + restoreSystemProperties(() -> { + System.setProperty(OS_NAME_PROPERTY, osNameValue); + assertSame(detectOS(), expectedOS); + }); + } + } + + public static class UnsupportedOSDetectionTest { + + @Test + public void testUnsupportedOs() throws Exception { + restoreSystemProperties(() -> { + System.setProperty(OS_NAME_PROPERTY, "unsupported"); + assertThrows(IllegalStateException.class, NativeLibLoader.OS::detectOS); + }); + } + } +} From f92c894f7a97e270db585511f212963c7c429b40 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 1 Feb 2022 14:51:57 +0100 Subject: [PATCH 02/12] Remove redundant private modifier for enums --- .../src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 b8d8be8f68dd..a20dbf0236cb 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 @@ -45,7 +45,7 @@ enum OS { final String name; - private OS(String name) { + OS(String name) { this.name = name; } @@ -80,7 +80,7 @@ enum Arch { final String name; - private Arch(String name) { + Arch(String name) { this.name = name; } From eceda142ba450c765b8417aa227dfcb1384cddda Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 1 Feb 2022 14:52:07 +0100 Subject: [PATCH 03/12] Fix typo in Javadoc --- .../src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a20dbf0236cb..7b044a1a248a 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 @@ -115,7 +115,7 @@ static Arch detectArch() { *
  • Supported OS: macOS, Windows, Linux, Solaris.
    • *
  • Supported Architectures: x86_64, aarch64, sparc.
    • * - * Throws UnsatisfiedLinkError if the library failed to load it's dependencies. + * Throws UnsatisfiedLinkError if the library failed to load its dependencies. * @throws IOException If the library could not be extracted from the jar. */ static synchronized void initXGBoost() throws IOException { From 278d968faef0fd9deaf87da178192638b28d7424 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 1 Feb 2022 14:54:23 +0100 Subject: [PATCH 04/12] Add support for detecting musl-based Linux This is done by checking the memory-mapped files in "/proc/self/map_files". If one of the files in there contains "musl", we consider the OS as musl-based Linux. --- .../dmlc/xgboost4j/java/NativeLibLoader.java | 41 ++++++++++++++++- .../dmlc/xgboost4j/java/OsDetectionTest.java | 44 ++++++++++++++----- 2 files changed, 74 insertions(+), 11 deletions(-) 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 7b044a1a248a..45e60ee83632 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,8 +21,13 @@ 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.stream.Stream; +import com.google.common.annotations.VisibleForTesting; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -34,6 +39,8 @@ class NativeLibLoader { private static final Log logger = LogFactory.getLog(NativeLibLoader.class); + private static Path mappedFilesBaseDir = Paths.get("/proc/self/map_files"); + /** * Supported OS enum. */ @@ -41,6 +48,7 @@ enum OS { WINDOWS("windows"), MACOS("macos"), LINUX("linux"), + LINUX_MUSL("linux-musl"), SOLARIS("solaris"); final String name; @@ -49,6 +57,11 @@ enum OS { this.name = name; } + @VisibleForTesting + static void setMappedFilesBaseDir(Path baseDir) { + mappedFilesBaseDir = baseDir; + } + /** * Detects the OS using the system properties. * Throws IllegalStateException if the OS is not recognized. @@ -61,13 +74,39 @@ static OS detectOS() { } else if (os.contains("win")) { return WINDOWS; } else if (os.contains("nux")) { - return LINUX; + return isMuslBased() ? LINUX_MUSL : LINUX; } else if (os.contains("sunos")) { return SOLARIS; } else { throw new IllegalStateException("Unsupported OS:" + os); } } + + /** + * Checks if the Linux OS is musl based. For this, we check the memory-mapped + * files 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)) { + return dirStream + .map(OS::toRealPath) + .anyMatch(s -> s.toLowerCase().contains("musl")); + } catch (IOException ignored) { + // ignored + } + return false; + } + + private static String toRealPath(Path path) { + try { + return path.toRealPath().toString(); + } catch (IOException e) { + return ""; + } + } + } /** 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 4a69787028c0..1de2fcc6f802 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 @@ -15,8 +15,11 @@ */ 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; @@ -26,15 +29,11 @@ import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties; import static java.util.Arrays.asList; import static junit.framework.TestCase.assertSame; -import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.WINDOWS; -import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.MACOS; -import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.LINUX; -import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.SOLARIS; -import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.detectOS; +import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.*; import static org.junit.Assert.assertThrows; /** - * Test cases for {@link NativeLibLoader.OS}. + * Test cases for {@link OS}. */ @RunWith(Enclosed.class) public class OsDetectionTest { @@ -45,9 +44,9 @@ public class OsDetectionTest { public static class ParameterizedOSDetectionTest { private final String osNameValue; - private final NativeLibLoader.OS expectedOS; + private final OS expectedOS; - public ParameterizedOSDetectionTest(String osNameValue, NativeLibLoader.OS expectedOS) { + public ParameterizedOSDetectionTest(String osNameValue, OS expectedOS) { this.osNameValue = osNameValue; this.expectedOS = expectedOS; } @@ -72,13 +71,38 @@ public void getOS() throws Exception { } } - public static class UnsupportedOSDetectionTest { + 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"); + + restoreSystemProperties(() -> { + 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"); + + restoreSystemProperties(() -> { + System.setProperty(OS_NAME_PROPERTY, "linux"); + assertSame(detectOS(), LINUX_MUSL); + }); + } @Test public void testUnsupportedOs() throws Exception { restoreSystemProperties(() -> { System.setProperty(OS_NAME_PROPERTY, "unsupported"); - assertThrows(IllegalStateException.class, NativeLibLoader.OS::detectOS); + assertThrows(IllegalStateException.class, OS::detectOS); }); } } From 317d838b12a89dba603dbcef7088bee30e2cb7e5 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 1 Feb 2022 15:49:03 +0100 Subject: [PATCH 05/12] Specify test scope for system-lambda artifact Co-authored-by: Marc Philipp --- jvm-packages/xgboost4j/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/jvm-packages/xgboost4j/pom.xml b/jvm-packages/xgboost4j/pom.xml index 06a219be068b..37b7a3503153 100644 --- a/jvm-packages/xgboost4j/pom.xml +++ b/jvm-packages/xgboost4j/pom.xml @@ -35,6 +35,7 @@ com.github.stefanbirkner system-lambda 1.2.1 + test     com.typesafe.akka From 5f925c1ae18d3019d538851402195677160d0470 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Wed, 2 Feb 2022 13:25:53 +0100 Subject: [PATCH 06/12] Add dependency for system-lambda to pom.xml generator for tests --- jvm-packages/xgboost4j-tester/generate_pom.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jvm-packages/xgboost4j-tester/generate_pom.py b/jvm-packages/xgboost4j-tester/generate_pom.py index 24a02d3417b5..7d29d1153fc7 100644 --- a/jvm-packages/xgboost4j-tester/generate_pom.py +++ b/jvm-packages/xgboost4j-tester/generate_pom.py @@ -101,6 +101,12 @@ 4.11 test + + com.github.stefanbirkner + system-lambda + 1.2.1 + test + ml.dmlc xgboost4j_${{scala.binary.version}} From 702f92657fe0eb04f63ad714d3ccc26bb20c9ac2 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Wed, 2 Feb 2022 13:28:46 +0100 Subject: [PATCH 07/12] Upgrade JUnit version and sync with generator Python script for tests on CI --- jvm-packages/xgboost4j-tester/generate_pom.py | 2 +- jvm-packages/xgboost4j/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jvm-packages/xgboost4j-tester/generate_pom.py b/jvm-packages/xgboost4j-tester/generate_pom.py index 7d29d1153fc7..16e1039d4868 100644 --- a/jvm-packages/xgboost4j-tester/generate_pom.py +++ b/jvm-packages/xgboost4j-tester/generate_pom.py @@ -98,7 +98,7 @@ junit junit - 4.11 + 4.13.2 test diff --git a/jvm-packages/xgboost4j/pom.xml b/jvm-packages/xgboost4j/pom.xml index 37b7a3503153..d2dc155e7281 100644 --- a/jvm-packages/xgboost4j/pom.xml +++ b/jvm-packages/xgboost4j/pom.xml @@ -28,7 +28,7 @@ junit junit - 4.13.1 + 4.13.2 test From c3cc638fcfbb0ee25ccab66f85e67d0963a02621 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 15 Feb 2022 09:13:27 +0100 Subject: [PATCH 08/12] Change JavaDoc of method isMuslBased to be less ambiguous --- .../src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 45e60ee83632..d618abe8b0b6 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 @@ -84,7 +84,7 @@ static OS detectOS() { /** * Checks if the Linux OS is musl based. For this, we check the memory-mapped - * files and see if one of those contains the string "musl". + * filenames and see if one of those contains the string "musl". * * @return true if the Linux OS is musl based, false otherwise. */ From 303bfcda7e8c29ec8af8b87b4b8f10dd14c9eca5 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 15 Feb 2022 09:24:24 +0100 Subject: [PATCH 09/12] Remove not strictly needed usage of VisibleForTesting annotation --- .../src/main/java/ml/dmlc/xgboost4j/java/NativeLibLoader.java | 2 -- 1 file changed, 2 deletions(-) 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 d618abe8b0b6..9f5740671ae9 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 @@ -27,7 +27,6 @@ import java.util.Locale; import java.util.stream.Stream; -import com.google.common.annotations.VisibleForTesting; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -57,7 +56,6 @@ enum OS { this.name = name; } - @VisibleForTesting static void setMappedFilesBaseDir(Path baseDir) { mappedFilesBaseDir = baseDir; } From cf0e7dc2965a507b15dc1312302d0a88ff1e5206 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Tue, 15 Feb 2022 09:53:11 +0100 Subject: [PATCH 10/12] Use custom solution for restoring system properties This gets rid of the test dependency system-lambda. --- jvm-packages/xgboost4j-tester/generate_pom.py | 6 ---- jvm-packages/xgboost4j/pom.xml | 6 ---- .../xgboost4j/java/ArchDetectionTest.java | 23 +++++++++++---- .../dmlc/xgboost4j/java/OsDetectionTest.java | 28 ++++++++++++++----- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/jvm-packages/xgboost4j-tester/generate_pom.py b/jvm-packages/xgboost4j-tester/generate_pom.py index 16e1039d4868..88f0c57bc47a 100644 --- a/jvm-packages/xgboost4j-tester/generate_pom.py +++ b/jvm-packages/xgboost4j-tester/generate_pom.py @@ -101,12 +101,6 @@ 4.13.2 test - - com.github.stefanbirkner - system-lambda - 1.2.1 - test - ml.dmlc xgboost4j_${{scala.binary.version}} diff --git a/jvm-packages/xgboost4j/pom.xml b/jvm-packages/xgboost4j/pom.xml index d2dc155e7281..d60a83b291b3 100644 --- a/jvm-packages/xgboost4j/pom.xml +++ b/jvm-packages/xgboost4j/pom.xml @@ -31,12 +31,6 @@ 4.13.2 test - - com.github.stefanbirkner - system-lambda - 1.2.1 - test - com.typesafe.akka akka-actor_${scala.binary.version} diff --git a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java index f610c2b83c39..1379992183c6 100644 --- a/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java +++ b/jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/ArchDetectionTest.java @@ -23,7 +23,6 @@ import java.util.Collection; -import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties; import static java.util.Arrays.asList; import static junit.framework.TestCase.assertSame; import static ml.dmlc.xgboost4j.java.NativeLibLoader.Arch.X86_64; @@ -63,8 +62,8 @@ public static Collection data() { } @Test - public void testArch() throws Exception { - restoreSystemProperties(() -> { + public void testArch() { + executeAndRestoreProperty(() -> { System.setProperty(OS_ARCH_PROPERTY, osArchValue); assertSame(detectArch(), expectedArch); }); @@ -74,12 +73,26 @@ public void testArch() throws Exception { public static class UnsupportedArchDetectionTest { @Test - public void testUnsupportedArch() throws Exception { - restoreSystemProperties(() -> { + public void testUnsupportedArch() { + executeAndRestoreProperty(() -> { System.setProperty(OS_ARCH_PROPERTY, "unsupported"); assertThrows(IllegalStateException.class, NativeLibLoader.Arch::detectArch); }); } } + private static void executeAndRestoreProperty(Runnable action) { + String oldValue = System.getProperty(OS_ARCH_PROPERTY); + + try { + action.run(); + } finally { + if (oldValue != null) { + System.setProperty(OS_ARCH_PROPERTY, oldValue); + } else { + System.clearProperty(OS_ARCH_PROPERTY); + } + } + } + } 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 1de2fcc6f802..b8ca3d77247d 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 @@ -26,7 +26,6 @@ import java.util.Collection; -import static com.github.stefanbirkner.systemlambda.SystemLambda.restoreSystemProperties; import static java.util.Arrays.asList; import static junit.framework.TestCase.assertSame; import static ml.dmlc.xgboost4j.java.NativeLibLoader.OS.*; @@ -63,8 +62,8 @@ public static Collection data() { } @Test - public void getOS() throws Exception { - restoreSystemProperties(() -> { + public void getOS() { + executeAndRestoreProperty(() -> { System.setProperty(OS_NAME_PROPERTY, osNameValue); assertSame(detectOS(), expectedOS); }); @@ -81,7 +80,7 @@ public void testForRegularLinux() throws Exception { setMappedFilesBaseDir(folder.getRoot().toPath()); folder.newFile("ld-2.23.so"); - restoreSystemProperties(() -> { + executeAndRestoreProperty(() -> { System.setProperty(OS_NAME_PROPERTY, "linux"); assertSame(detectOS(), LINUX); }); @@ -92,18 +91,33 @@ public void testForMuslBasedLinux() throws Exception { setMappedFilesBaseDir(folder.getRoot().toPath()); folder.newFile("ld-musl-x86_64.so.1"); - restoreSystemProperties(() -> { + executeAndRestoreProperty(() -> { System.setProperty(OS_NAME_PROPERTY, "linux"); assertSame(detectOS(), LINUX_MUSL); }); } @Test - public void testUnsupportedOs() throws Exception { - restoreSystemProperties(() -> { + public void testUnsupportedOs() { + executeAndRestoreProperty(() -> { System.setProperty(OS_NAME_PROPERTY, "unsupported"); assertThrows(IllegalStateException.class, OS::detectOS); }); } } + + private static void executeAndRestoreProperty(Runnable action) { + String oldValue = System.getProperty(OS_NAME_PROPERTY); + + try { + action.run(); + } finally { + if (oldValue != null) { + System.setProperty(OS_NAME_PROPERTY, oldValue); + } else { + System.clearProperty(OS_NAME_PROPERTY); + } + } + } + } From 6afc19fd47c1e71c211f7dff1341cc8e779024a7 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Thu, 10 Mar 2022 09:31:05 +0100 Subject: [PATCH 11/12] Add debug log when musl-related memory mapped file is detected The debug log also shows the filename which causes us to consider the Linux OS as musl-based. --- .../ml/dmlc/xgboost4j/java/NativeLibLoader.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) 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 9f5740671ae9..fd87b3dee72c 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 @@ -25,6 +25,7 @@ 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; @@ -88,9 +89,17 @@ static OS detectOS() { */ static boolean isMuslBased() { try (Stream dirStream = Files.list(mappedFilesBaseDir)) { - return dirStream - .map(OS::toRealPath) - .anyMatch(s -> s.toLowerCase().contains("musl")); + 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 (IOException ignored) { // ignored } From 2e1378887bdd28c5d5d6e906dd378529e1d5ceb4 Mon Sep 17 00:00:00 2001 From: Daniel Clausen Date: Thu, 10 Mar 2022 09:45:51 +0100 Subject: [PATCH 12/12] Enhance logged error in case the shared library cannot be loaded In case of Linux, we now additionally mention that the MUSL-detection might have gone wrong. --- .../dmlc/xgboost4j/java/NativeLibLoader.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) 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 fd87b3dee72c..e6e6542a5288 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 @@ -175,18 +175,37 @@ static synchronized void initXGBoost() throws IOException { platform + "/" + System.mapLibraryName(libName); loadLibraryFromJar(libraryPathInJar); } catch (UnsatisfiedLinkError ule) { - logger.error("Failed to load " + libName + " due to missing native dependencies for " + - "platform " + platform + ", this is likely due to a missing OpenMP dependency"); + String failureMessageIncludingOpenMPHint = "Failed to load " + libName + " " + + "due to missing native dependencies for " + + "platform " + platform + ", " + + "this is likely due to a missing OpenMP dependency"; + switch (os) { case WINDOWS: + logger.error(failureMessageIncludingOpenMPHint); logger.error("You may need to install 'vcomp140.dll' or 'libgomp-1.dll'"); break; case MACOS: - logger.error("You may need to install 'libomp.dylib', via `brew install libomp`" + - " or similar"); + logger.error(failureMessageIncludingOpenMPHint); + logger.error("You may need to install 'libomp.dylib', via `brew install libomp` " + + "or similar"); break; case LINUX: + 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."); + break; case SOLARIS: + logger.error(failureMessageIncludingOpenMPHint); logger.error("You may need to install 'libgomp.so' (or glibc) via your package " + "manager."); break;