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.

Sign up for GitHub

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

Jump to bottom

Add support for finding libraries with multi component versions on Linux #76

Merged
merged 1 commit into from
Sep 21, 2016
Merged

Add support for finding libraries with multi component versions on Linux #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 74 additions & 33 deletions src/main/java/jnr/ffi/Platform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@

import java.io.File;
import java.io.FilenameFilter;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public abstract class Platform {
Expand Down Expand Up @@ -448,22 +449,14 @@ public String getName() {
/**
* A {@link Platform} subclass representing the Linux operating system.
*/
private static final class Linux extends Supported {
static final class Linux extends Supported {

Copy link
Contributor

Choose a reason for hiding this comment

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

package private is for testing I guess ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed.

public Linux() {
super(OS.LINUX);
}

@Override
public String locateLibrary(final String libName, List<String> libraryPath) {
FilenameFilter filter = new FilenameFilter() {
Pattern p = Pattern.compile("lib" + libName + "\\.so\\.[0-9]+$");
String exact = "lib" + libName + ".so";
public boolean accept(File dir, String name) {
return p.matcher(name).matches() || exact.equals(name);
}
};

public String locateLibrary(final String libName, List<String> libraryPaths) {
Pattern exclude;
// there are /libx32 directories in wild on ubuntu 14.04 and the
// oracle-java8-installer package
Expand All @@ -474,43 +467,91 @@ public boolean accept(File dir, String name) {
exclude = Pattern.compile(".*(lib[a-z]*64|amd64|x86_64).*");
}

List<File> matches = new LinkedList<File>();
for (String path : libraryPath) {
final Pattern versionedLibPattern = Pattern.compile("lib" + libName + "\\.so((?:\\.[0-9]+)*)$");

FilenameFilter filter = new FilenameFilter() {
public boolean accept(File dir, String name) {
return versionedLibPattern.matcher(name).matches();
}
};

Map<String, int[]> matches = new HashMap<String, int[]>();
for (String path : libraryPaths) {
if (exclude.matcher(path).matches()) {
continue;
}
File[] files = new File(path).listFiles(filter);
if (files != null && files.length > 0) {
matches.addAll(Arrays.asList(files));

File libraryPath = new File(path);
File[] files = libraryPath.listFiles(filter);
if (files == null) {
continue;
}

for (File file : files) {
Matcher matcher = versionedLibPattern.matcher(file.getName());
String versionString = matcher.matches() ? matcher.group(1) : "";
int[] version;
if (versionString == null || versionString.isEmpty()) {
version = new int[0];
} else {
String[] parts = versionString.split("\\.");
version = new int[parts.length - 1];
for (int i = 1; i < parts.length; i++) {
version[i - 1] = Integer.parseInt(parts[i]);
}
}
matches.put(file.getAbsolutePath(), version);
Copy link
Contributor

@nirvdrum nirvdrum Nov 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This loses the ordering of the library paths and is proving to be problematic for me. I have both /lib64/librt.so.1 and /lib/librt.so.1, with /lib64 before /lib. I'm going to look at stripping out the /lib part, but it's coming from the "java.library.path" system property so it's likely to be there for others, too. In any event, since the HashMap here loses the ordering, /lib/librt.so1 actually comes before /lib64/librt.so.1 during the best match scan. Since they have the same version number, there's no way for the /lib64 version to win and I wind up with an UnsatisfiedLinkError: /lib/librt.so.1: wrong ELF class: ELFCLASS32.

Copy link
Contributor Author

@pepijnve pepijnve Nov 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Simple fix is to use a LinkedHashMap instead of a HashMap for the matches Map. That way insertion order will be retained and the original behaviour should be restored. The only caveat is that, continuing on your example, if lib contains a more precies version than lib64 that the lib version will get picked.
Ideally the library loading code should filter out libraries that are not using the correct ABI I guess. That's new functionality that the old code did not do either though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I'd be happy just getting the sort order back. It appears the java.library.path changes were made with JDK 7 to include both 32-bit and 64-bit paths. While I'm not convinced that was a worthwhile change, it's something we'll have to cope with.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated in 9143ba2.

}
}

//
// Search through the results and return the highest numbered version
// i.e. libc.so.6 is preferred over libc.so.5
//
int bestVersion = -1;
int[] bestVersion = null;
String bestMatch = null;
for (File file : matches) {
String path = file.getAbsolutePath();
int fileVersion;
if (path.endsWith(".so")) {
fileVersion = 0;
} else {
String num = path.substring(path.lastIndexOf(".so.") + 4);
try {
fileVersion = Integer.parseInt(num);
} catch (NumberFormatException e) {
continue; // Just skip if not a number
}
}
if (fileVersion > bestVersion) {
bestMatch = path;
for (Map.Entry<String,int[]> entry : matches.entrySet()) {
String file = entry.getKey();
int[] fileVersion = entry.getValue();

if (compareVersions(fileVersion, bestVersion) > 0) {
bestMatch = file;
bestVersion = fileVersion;
}
}

return bestMatch != null ? bestMatch : mapLibraryName(libName);
}

private static int compareVersions(int[] version1, int[] version2) {
// Null is always smallest
if (version1 == null) {
return version2 == null ? 0 : -1;
}
if (version2 == null) {
return 1;
}

// Compare component by component
int commonLength = Math.min(version1.length, version2.length);
for (int i = 0; i < commonLength; i++) {
if (version1[i] < version2[i]) {
return -1;
} else if (version1[i] > version2[i]) {
return 1;
}
}

// If all components are equal, version with fewest components is smallest
if (version1.length < version2.length) {
return -1;
} else if (version1.length > version2.length) {
return 1;
} else {
return 0;
}
}

@Override
public String mapLibraryName(String libName) {
// Older JDK on linux map 'c' to 'libc.so' which doesn't work
Expand Down
94 changes: 94 additions & 0 deletions src/test/java/jnr/ffi/PlatformTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package jnr.ffi;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import java.io.File;
import java.io.IOException;
import java.util.Collections;

public class PlatformTest {
private File tmpDir;

@Before
public void createTempDir() throws IOException {
tmpDir = mkTmpDir();
}

@After
public void deleteTempDir() throws IOException {
rmDir(tmpDir);
}

private void rmDir(File dir) throws IOException {
File[] children = dir.listFiles();
if (children != null) {
for (File child : children) {
if (child.isDirectory()) {
rmDir(child);
} else {
rm(child);
}
}
}
rm(dir);
}

private void rm(File file) throws IOException {
if (!file.delete()) {
throw new IOException("Could not delete " + file);
}
}

private File mkTmpDir() throws IOException {
File tmp = new File(System.getProperty("java.io.tmpdir"));
String suffix = System.currentTimeMillis() + "-";
for (int counter = 0; counter < 100; counter++) {
File tempDir = new File(tmp, suffix + counter);
if (tempDir.mkdir()) {
return tempDir;
}
}
throw new IOException("Could not create temp dir");
}

private void testVersionComparison(String expected, String... versions) throws Exception {
Platform.Linux linux = new Platform.Linux();
String libName = "test";
String mappedLibName = "lib" + libName + ".so";
for (String version : versions) {
String name = versionedLibName(mappedLibName, version);
if (!new File(tmpDir, name).createNewFile()) {
throw new IOException("Could not create file " + name);
}
}

String locatedLibrary = linux.locateLibrary(libName, Collections.singletonList(tmpDir.getAbsolutePath()));
Assert.assertEquals(new File(tmpDir, versionedLibName(mappedLibName, expected)).getAbsolutePath(), locatedLibrary);
}

private String versionedLibName(String mappedLibName, String version) {
String name = mappedLibName;
if (!version.isEmpty()) {
name += "." + version;
}
return name;
}

@Test
public void testNoVersionComparison() throws Exception {
testVersionComparison("", "");
}

@Test
public void testSingleComponentVersionComparison() throws Exception {
testVersionComparison("42", "", "5", "6", "42");
}

@Test
public void testMultiComponentVersionComparison() throws Exception {
testVersionComparison("42.1.3.4", "", "5", "6.1", "42", "42.1", "42.0.5", "42.1.3.4");
}
}