AlessioDP / libby

A runtime dependency management library for plugins running in Java-based Minecraft server platforms.
MIT License
81 stars 20 forks source link

Transitive dependencies support #22

Closed bivashy closed 1 year ago

bivashy commented 1 year ago

I've added support for transitive dependencies using maven-resolver.

I created this as a separate module so that someone can use it only if they really want to. The core module remains unchanged.

Usage

LibraryManager libraryManager = new BukkitLibraryManager(plugin);

TransitiveLibraryManager transitiveLibraryManager = TransitiveLibraryManager.wrap(libraryManager);

Library library = Library.builder()
    .groupId("your{}dependency{}groupId")
    .artifactId("artifactId")
    .version("version")
    .build();

transitiveLibraryManager.loadLibraryTransitively(library);

You can also exclude transitive libraries:

transitiveLibraryManager.loadLibraryTransitively(library, ExcludedLibrary.of("excluded{}dependency{}groupId", "excludedArtifactId"));

Note: The demo snippet that has been used can be found here: Snippet

frengor commented 1 year ago

First of all, good job! This is a very requested feature.

Speaking from an API prospective, I personally don't like how the functionality is provided (i.e. TransitiveLibraryManager). I would have preferred something like adding a method to the Library's builder:

LibraryManager libraryManager = new BukkitLibraryManager(plugin);

Library library = Library.builder()
    .groupId("your{}dependency{}groupId")
    .artifactId("artifactId")
    .version("version")
    .resolveTransitiveDeps(true) // This indicates to the LibraryManager to resolve transitive dependencies
    .excludeTransitiveDep(ExcludedLibrary.of("excluded{}dependency{}groupId", "excludedArtifactId")) // This excludes a transitive library
    .build();

libraryManager.loadLibrary(library); // This will resolve and load transitive dependencies

Also, I would avoid adding another module and instead download (and load) the maven-resolver dependencies at runtime using Libby itself. For reference, Libby already does this in RelocationHelper and URLClassLoaderHelper.

bivashy commented 1 year ago

Thanks for the feedback

About how the functionality is provided, I don't really like that either. But the main problem is that if we download maven-resolver dependencies in runtime, we may have problems with huge use of reflection, which in my personal opinion will degrade the readability of the code, because we would need to load, initialize, call methods for many classes.

If there were a way to avoid or somehow fix this problem, I could implement it into the Library's builder

frengor commented 1 year ago

we may have problems with huge use of reflection

A possible solution to this is to write all the code which handles the dependency resolution in 1 class and then manually define it into the IsolatedClassLoader. Such class won't obviously be used anywhere else in the code (it can be a package-private class so that it isn't exposed in the API).
This solution requires adding the following code to IsolatedClassLoader:

Code ```java /** * Defines and loads a class. * * @param name The expected binary name of the class * @param classBytes An {@link InputStream} which provides the class bytes. It will be automatically closed by this * method after being read. * @return The defined class * @throws IOException If an exception occurs while reading the provided {@link InputStream} * @throws ClassFormatError If the bytes provided by the {@link InputStream} doesn't contain valid class * @see ClassLoader#defineClass(String, byte[], int, int) */ public Class defineClass(String name, InputStream classBytes) throws IOException, ClassFormatError { byte[] bytes = readAllBytes(classBytes); return super.defineClass(name, bytes, 0, bytes.length); } // From: https://stackoverflow.com/a/53347936 private static byte[] readAllBytes(InputStream inputStream) throws IOException { final int bufLen = 4 * 0x400; // 4KB byte[] buf = new byte[bufLen]; int readLen; IOException exception = null; try { try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { while ((readLen = inputStream.read(buf, 0, bufLen)) != -1) outputStream.write(buf, 0, readLen); return outputStream.toByteArray(); } } catch (IOException e) { exception = e; throw e; } finally { if (exception == null) inputStream.close(); else try { inputStream.close(); } catch (IOException e) { exception.addSuppressed(e); } } } ```

And can be used in the following way:

String helperClassName = "net.byteflux.libby.trantitive.TransitiveHelper"; // Imaginary class which implements the dependency resolution
String helperClassPath = '/' + helper.replace('.', '/') + ".class"; // Don't hardcode the path so that relocation still works
Class<?> helperClass = isolatedClassLoaderInstance.defineClass(helperClassName, getClass().getResourceAsStream(helperClassPath));
// Use reflection to get and call the helper methods, but without needing to write all the code with reflections
bivashy commented 1 year ago

we may have problems with huge use of reflection

A possible solution to this is to write all the code which handles the dependency resolution in 1 class and then manually define it into the IsolatedClassLoader. Such class won't obviously be used anywhere else in the code (it can be a package-private class so that it isn't exposed in the API). This solution requires adding the following code to IsolatedClassLoader: Code

/**
 * Defines and loads a class.
 *
 * @param name The expected binary name of the class
 * @param classBytes An {@link InputStream} which provides the class bytes. It will be automatically closed by this
 *                   method after being read.
 * @return The defined class
 * @throws IOException If an exception occurs while reading the provided {@link InputStream}
 * @throws ClassFormatError If the bytes provided by the {@link InputStream} doesn't contain valid class
 * @see ClassLoader#defineClass(String, byte[], int, int)
 */
public Class <?> defineClass(String name, InputStream classBytes) throws IOException, ClassFormatError {
    byte[] bytes = readAllBytes(classBytes);
    return super.defineClass(name, bytes, 0, bytes.length);
}

// From: https://stackoverflow.com/a/53347936
private static byte[] readAllBytes(InputStream inputStream) throws IOException {
    final int bufLen = 4 * 0x400; // 4KB
    byte[] buf = new byte[bufLen];
    int readLen;
    IOException exception = null;

    try {
        try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
            while ((readLen = inputStream.read(buf, 0, bufLen)) != -1)
                outputStream.write(buf, 0, readLen);

            return outputStream.toByteArray();
        }
    } catch (IOException e) {
        exception = e;
        throw e;
    } finally {
        if (exception == null) inputStream.close();
        else try {
            inputStream.close();
        } catch (IOException e) {
            exception.addSuppressed(e);
        }
    }
}

And can be used in the following way:

String helperClassName = "net.byteflux.libby.trantitive.TransitiveHelper"; // Imaginary class which implements the dependency resolution
String helperClassPath = '/' + helper.replace('.', '/') + ".class"; // Don't hardcode the path so that relocation still works
Class<?> helperClass = isolatedClassLoaderInstance.defineClass(helperClassName, getClass().getResourceAsStream(helperClassPath));
// Use reflection to get and call the helper methods, but without needing to write all the code with reflections

Thank you very much!!! This is literally what I asked, soon I will rewrite all transitive dependency thing into core module.

bivashy commented 1 year ago

It is done.

Usage

Library library = Library.builder()
    .groupId("your{}dependency{}groupId")
    .artifactId("artifactId")
    .version("version")
    .resolveTransitiveDependencies(true)
    .excludeTransitiveDependency("excluded{}dependency{}groupId", "artifactId")
    .build();

Transitive dependencies conflict Currently, I'm unsure how to mark two libraries as "conflicting", if they would have conflicting transitive dependencies. It is required, so i can pass two library into Maven Resolver, so it would "resolve" conflict.

Relocations

Library library = Library.builder()
    .groupId("your{}dependency{}groupId")
    .artifactId("artifactId")
    .version("version")
    .resolveTransitiveDependencies(true)
    .excludeTransitiveDependency("excluded{}dependency{}groupId", "artifactId")
    .relocate("test{}library{}package", "another{}library{}package")
    .relocate("transitive{}dependency{}package", "another{}library{}package{}transitive")
    .build();

So all relocations/repositories will be cloned into transitive dependency.