Skip to content

Native libraries from packages are not copied #13101

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

Merged

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented May 15, 2025

Closes #12557

Pull Request Description

Removes the copying of native libraries next to the generated NI binary. Native libraries of packages are loaded from the same location in both JVM and AOT modes.

Old behavior: EnsoLibraryFeature copies all the native libraries within **/polyglot/lib/** next to the generated NI binary. The motivation for that is mentioned in the description of #11874.

New behavior: EnsoLibraryFeature no longer copies native libraries. Instead, once a package is loaded in DefaultPackageRepository.registerPackageInternal, paths to its native libraries are added to the native library search path via NativeLibrarySearchPath. Inspired by https://github.com/Akirathan/native-lib-loader.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@Akirathan Akirathan self-assigned this May 15, 2025
@Akirathan Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels May 15, 2025
@Akirathan
Copy link
Member Author

Setting java.library.path system property does not work

In JVM mode, java.library.path system property is cached during bootstrap. Any subsequent change to the property is not reflected.

In AOT mode, java.library.path system property is cached in the first call to System.loadLibrary. If the property is changed before that call, it is reflected by SVM, but not after that.

Conclusion: Using java.library.path is not a good idea. It would work only if we could set all the path before first call to System.loadLibrary. This is not possible - the first call to System.loadLibrary happens during parsing of enso modules. Changing the java.llibrary.path would happen once a Enso package is loaded.

@Akirathan
Copy link
Member Author

Akirathan commented May 19, 2025

Setting LD_LIBRARY_PATH env var via native calls does not work

Trying to set LD_LIBRARY_PATH via something like:

package org.mayfa.untitled.nativeenv;

import java.util.List;
import org.graalvm.nativeimage.c.CContext;
import org.graalvm.nativeimage.c.function.CFunction;
import org.graalvm.nativeimage.c.type.CCharPointer;
import org.graalvm.nativeimage.c.type.CTypeConversion;

@CContext(value = NativeEnv.Directives.class)
public final class NativeEnv {

  @CFunction
  static native int setenv(CCharPointer name, CCharPointer value, int overwrite);

  @CFunction
  static native CCharPointer getenv(CCharPointer name);

  public static void setEnv(String name, String value) {
    try (var namePtr = CTypeConversion.toCString(name);
        var valuePtr = CTypeConversion.toCString(value)) {
      var ret = setenv(namePtr.get(), valuePtr.get(), 1);
      if (ret != 0) {
        throw new AssertionError("Failed to set environment variable " + name + ", retcode = " + ret);
      }
    }
  }

  public static String getEnv(String name) {
    try (var namePtr = CTypeConversion.toCString(name)) {
      var ret = getenv(namePtr.get());
      if (ret.isNull()) {
        return null;
      }
      return CTypeConversion.toJavaString(ret);
    }
  }

  static final class Directives implements CContext.Directives {
    @Override
    public List<String> getHeaderFiles() {
      return List.of("<stdlib.h>");
    }
  }
}

suffers the same pitfall as setting java.library.path system property, i.e., if an attempt to find a native library is made before LD_LIBRARY_PATH is set via setenv syscall, the subsequent changes to LD_LIBRARY_PATH env var are not reflected. Which is expected according to the POSIX standards.

@Akirathan
Copy link
Member Author

Akirathan commented May 21, 2025

After few discussions with @JaroslavTulach, I have arrived at a working solution demonstrated in https://github.com/Akirathan/native-lib-loader. Discussed also in GraalVM Slack. This solution explicitly changes the com.oracle.svm.core.jdk.NativeLibraries#usrPaths field at runtime.

Now, it is time to integrate this idea into Enso.

GitHub
Simple project demonstrating how to change the native library search path at runtime. - Akirathan/native-lib-loader

This comment was marked as off-topic.

@Akirathan Akirathan marked this pull request as ready for review May 22, 2025 07:38
@Akirathan
Copy link
Member Author

Akirathan commented May 22, 2025

I have downloaded the built-distribution artifact from https://github.com/enso-org/enso/actions/runs/15169686252/job/42689718870#step:12:100 and ensured that there is no relevant native library next to the generated binary:

$ ls -l built-distribution/enso-engine-2025.2.1-dev-linux-amd64/enso-2025.2.1-dev/bin/
total 487M
-rwxr-xr-x 1 pavel pavel 486M May 21 20:32 enso
-rw-r--r-- 1 pavel pavel 1.1M May 21 20:29 libsqlitejdbc.so

(libsqlitejdbc.so is not comming from any std lib native libs)

GitHub
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - Native libraries from packages are not copied · 9d88669

@JaroslavTulach
Copy link
Member

JaroslavTulach commented May 22, 2025

... solution demonstrated in https://github.com/Akirathan/native-lib-loader. ... explicitly changes the com.oracle.svm.core.jdk.NativeLibraries#usrPaths field at runtime.

  • The tough part is to guarantee this trick works in future versions of native image
  • This would be an interesting library to publish at Maven Central
    • assuming the GraalVM guys don't want the functionality into GraalVM itself
    • assuming there is a way to guarantee it would work in the future
  • The project would need nicer API
    • NativeLibLoader.loadLibrary(String library, String... searchPaths) maybe?
  • the main method doesn't need to be public, I think
  • or the main method needn't be there at all
  • better to have some unit tests using standard native image JUnit support

Just an idea for the long term sustainability vision. In any case I am glad you got it working with MethodHandles!

*
* @param path Directory to add to the search path.
*/
public static void addToSearchPath(String path) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not use a real type like Path?

Copy link
Member Author

@Akirathan Akirathan May 22, 2025

Choose a reason for hiding this comment

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

The only callsite uses TruffleFile. Better String than TruffleFile.

NativeLibraryFinder.listAllNativeLibraries(pkg, FileSystem$.MODULE$.defaultFs());
for (var nativeLib : nativeLibs) {
var out = new File(nativeLibDir, nativeLib.getName());
Files.copy(nativeLib.toPath(), out.toPath(), StandardCopyOption.REPLACE_EXISTING);
Copy link
Member

Choose a reason for hiding this comment

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

Heuréka, if the system continues to work even after removing these copies!

@Akirathan Akirathan added the CI: Ready to merge This PR is eligible for automatic merge label May 22, 2025
@mergify mergify bot merged commit f0cbf1a into develop May 22, 2025
81 of 83 checks passed
@mergify mergify bot deleted the wip/akirathan/12557-no-native-lib-duplicate-next-to-bin branch May 22, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove duplicated .so libraries from AppImage & other distributions
2 participants