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.

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

Search for lib in java.library.path instead of hardcoded path #81

Merged
merged 4 commits into from
Sep 28, 2023
Merged

Search for lib in java.library.path instead of hardcoded path #81

merged 4 commits into from
Sep 28, 2023

Conversation

nicomem
Copy link
Contributor

@nicomem nicomem commented Sep 26, 2023

Resolve #72

@CLAassistant
Copy link

CLAassistant commented Sep 26, 2023

CLA assistant check
All committers have signed the CLA.

@overheadhunter
Copy link
Member

Both System.mapLibraryName and looking in java.library.path is what Java does per default (via System.loadLibrary), however what happens here is that it simply adds "lib" + name + ".so" on Linux, which fails if the file name is e.g. Debian's libfuse3.so.3.

Hence we had to ditch this default approach.

What we discussed internally: Adding a separate system property, however some Cryptomator distributions (like the AppImage) don't allow to manually override those. Our best option currently is to look in some well-known locations and fallback to System.loadLibrary and hope that there is no additional suffix after .so.

@infeo
Copy link
Member

infeo commented Sep 27, 2023

fallback to System.loadLibrary and hope that there is no additional suffix after .so.

Some systems create a symlink to the newest version of a library, some not. If we add this last ditch attempt, we need to ensure to use the correct fuse version (by calling fuse_version())

@overheadhunter
Copy link
Member

fallback to System.loadLibrary and hope that there is no additional suffix after .so.

Some systems create a symlink to the newest version of a library, some not. If we add this last ditch attempt, we need to ensure to use the correct fuse version (by calling fuse_version())

The major version (which ensures API-compatibility) is part of the lib name (fuse3 instead of fuse)

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

I just opened cryptomator/jfuse#37 to fallback to the System.loadLibrary mechanism.

fuse-nio-adapter should not duplicate what Java is already doing. Instead builder.setLibraryPath(...) is intended to be used to attempt loading from a verified path to an existing .so file.

If no such file could be found in those places, do not set the library path, which will then cause jfuse to fallback to System.loadLibrary(...).

In other words: After updating jfuse, we need something like this:

Arrays.stream(LIB_PATHS).map(Path::of).filter(Files::exists).map(Path::toString).findAny().ifPresent(builder::setLibraryPath);

@nicomem
Copy link
Contributor Author

nicomem commented Sep 27, 2023

Since there will now be a fallback mecanism in case no element in LIB_PATHS is present, the isSupported() method will also need to remove the first test and simply become return isFusermount3Installed();, right?

@overheadhunter
Copy link
Member

Since there will now be a fallback mecanism in case no element in LIB_PATHS is present, the isSupported() method will also need to remove the first test and simply become return isFusermount3Installed();, right?

Right!

Furthermore we need to remove /ust/lib/libfuse3.so from our array, as this is now handled via the fallback mechanism (assuming /usr/lib is in java.library.path on any sane system). Otherwise it would take precedence over e.g. /usr/lib64, which is what causes cryptomator/cryptomator#3127.

@overheadhunter

This comment was marked as resolved.

@infeo

This comment was marked as resolved.

@overheadhunter overheadhunter changed the title Search for lib in java.library.path instead of hardcoded path Search for lib in java.library.path instead of hardcoded path Sep 28, 2023
@sonarcloud
Copy link

sonarcloud bot commented Sep 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@overheadhunter overheadhunter merged commit 3f6d8a3 into cryptomator:develop Sep 28, 2023
7 checks passed
@overheadhunter
Copy link
Member

Thank you @nicomem 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use native system PATH to search for fuse-lib
4 participants