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

JPMS support #1957

Open
cowwoc opened this issue Feb 9, 2022 · 29 comments
Open

JPMS support #1957

cowwoc opened this issue Feb 9, 2022 · 29 comments

Comments

@cowwoc
Copy link

cowwoc commented Feb 9, 2022

Looking at testng-team/testng#2727 it sounds like you need to include module-info.java in all your releases that opens <package>. Otherwise, JDK 9+ users will be unable to read the resources directory.

You can use multi-release JARs to support both JDK 8 and JDK 9+ users. Feel free to reach out if you have any questions.

@bowbahdoe
Copy link

Bump?

@jamesward
Copy link
Member

I don't have much experience with JPMS stuff so not sure what to do for this. Can someone contribute what is needed to an existing WebJar?

@cowwoc
Copy link
Author

cowwoc commented Jul 24, 2023

Good question :) I've asked for guidance here: https://stackoverflow.com/q/76756387/14731

@bowbahdoe
Copy link

I'm also not a great resource, but in this video at 28:10 the person says there is a loophole for resources in the encapsulation.

https://www.youtube.com/watch?v=4fevIDAxQAM

So thats why I thought just adding an empty module-info would be enough.

@cowwoc
Copy link
Author

cowwoc commented Jul 25, 2023

I'm also not a great resource, but in this video at 28:10 the person says there is a loophole for resources in the encapsulation.

The loophole is that a module can see its own resources, which isn't our case here.

Regardless, I don't think that webjars has a JPMS support.

Per https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/Module.html#getResourceAsStream(java.lang.String):

If the resource is not in a package in the module then the resource is not encapsulated. [...]
A resource name with the name "META-INF/MANIFEST.MF" is never encapsulated because "META-INF" is not a legal package name.

I'm going to close this issue. Whatever is going on in TestNG sounds like a bug on their end.

@cowwoc cowwoc closed this as completed Jul 25, 2023
@bowbahdoe
Copy link

bowbahdoe commented Jul 25, 2023

@cowwoc wait, i'm confused. The use case I want to support is having the webjars on the module path and having them play well with jlink. Adding empty module descriptors still sounds like it would accomplish that.

@bowbahdoe
Copy link

bowbahdoe commented Jul 25, 2023

Basic example, if I have the jquery webjar on the module path then I can do this

module mod {
    requires jquery;
    exports mod;
}
javac --module-path libs mod/**/*.java -d out

But without an actual module descriptor, jlink won't be happy

jlink --module-path libs:out --add-modules mod --output image
Error: automatic module cannot be used with jlink: jquery from file:///Users/emccue/Development/linky/libs/jquery-3.7.0.jar

Very few people use modules, but it becomes more possible the more "base" libraries do.

@cowwoc
Copy link
Author

cowwoc commented Jul 25, 2023

@bowbahdoe Sorry, you're right.

@jamesward Does any webjar include Java classes? If not, all you'd need to do is add module-info.java to each webjar containing the following:

module org.webjars.<artifact_id>
{}

and just like that you'd have JPMS support.

@cowwoc cowwoc reopened this Jul 25, 2023
@jamesward
Copy link
Member

There are not any java classes in WebJars (with the exception of the utilities like webjars-locator) so we could add that file. The only problem is that we can't add it retroactively so it'd only be for new WebJars. Silly question... where does the module-info.java file go in the jar?

@cowwoc
Copy link
Author

cowwoc commented Jul 25, 2023

@jamesward You have to create this file in the root package, compile it, then stick the class file inside the root directory of the JAR.

@jamesward
Copy link
Member

I'm happy to take a stab at this. Is there a WebJar any of you use that we could test with?

@cowwoc
Copy link
Author

cowwoc commented Jul 25, 2023

@jamesward Not at the moment. Maybe @bowbahdoe ?

@bowbahdoe
Copy link

I'll try and get a minimal example

@bowbahdoe
Copy link

bowbahdoe commented Jul 25, 2023

https://github.com/bowbahdoe/webjar-jpms-example

@jamesward Here is a minimal jlink-ing server app that consumes htmx.

@bowbahdoe
Copy link

@jamesward Any time to get back to this?

@jamesward
Copy link
Member

Thanks for the ping. NPM WebJars currently are created by:
https://github.com/webjars/webjars/blob/main/app/utils/WebJarCreator.scala

I also am working on moving all Classic WebJars to this approach as well. So that is where we will need to somehow create the module-info.java file and compile it, and then put the class in the jar. Is that right? Any ideas on how I should do the compile from something running in the JVM?

@bowbahdoe
Copy link

@jamesward An easy way is to use a tool provider

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.spi.ToolProvider;

public class Main {
    public static Path makeOpenModule(String moduleName) throws Exception {
        String moduleContents = """
                open module %s {}
                """.formatted(moduleName);

        Path dir = Files.createTempDirectory("module-temp");
        Path file = Path.of(dir.toString(), "module-info.java");

        Files.writeString(file, moduleContents);
        var javac = ToolProvider.findFirst("javac").orElseThrow();

        int exitCode = javac.run(System.out, System.err, "--release", "9", file.toString());
        if (exitCode != 0) {
            throw new Exception("javac exit code: " + exitCode);
        }

        return Path.of(dir.toString(), "module-info.class");
    }

    public static void main(String[] args) throws Exception {
        System.out.println(makeOpenModule("org.example"));
    }
}

@bowbahdoe
Copy link

As to whether we need an open module or not - here are the docs. Obv. should test a little but I think absent any class files it doesn't need to be open (not that I can think of much of a downside)

image

@jamesward
Copy link
Member

Thank you! I'll give this a try.

@bowbahdoe
Copy link

bowbahdoe commented May 2, 2024

Oh, also - putting the class file in the jar @ /module-info.class will probably be fine, but technically /META-INF/versions/9/module-info.class + adding Multi-Release: true to the MANIFEST.MF can deal with some hypothetical issues where people try to parse every class in their dependencies and also haven't updated their java in 10 years

https://openjdk.org/jeps/238

@jamesward
Copy link
Member

Thanks! Question... I'm trying this and there seems to be a constraint on the module names that make them incompatible with the artifact names. For example:

/tmp/module-temp2528621057074025779/module-info.java:1: error: '{' expected
open module org.webjars.npm.react-redux {}

Should I just strip the invalid chars or is there a better way?

@bowbahdoe
Copy link

bowbahdoe commented May 2, 2024

Should I just strip the invalid chars or is there a better way?

I think either stripping them or replacing them with .s is the only way to go.

First thought is that org.webjars.npm.react.redux feels better than org.webjars.npm.reactredux. There is some algorithm that java uses to generate automatic module names from file names...that might be easy to duplicate.

jamesward added a commit that referenced this issue May 2, 2024
@bowbahdoe
Copy link

bowbahdoe commented May 2, 2024

lord forgive me

import java.io.FileOutputStream;
import java.lang.module.ModuleDescriptor;
import java.lang.module.ModuleFinder;
import java.lang.module.ModuleReference;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.zip.ZipOutputStream;

public class Main {
    public static String sanitizeModuleName(String moduleName) throws Exception{
        Path file = Path.of(
                Files.createTempDirectory("sanitize-hack").toString(),
                moduleName + ".jar"
        );

        try (var zos = new ZipOutputStream(new FileOutputStream(file.toFile()))) {}

        return ModuleFinder.of(file).findAll()
                .stream().map(ModuleReference::descriptor)
                .map(ModuleDescriptor::name)
                .findFirst()
                .orElseThrow();
    }

    public static void main(String[] args) throws Exception {
        System.out.println(sanitizeModuleName("org.example"));
        System.out.println(sanitizeModuleName("org.example.react-redux"));
    }
}

Another issue, besides invalid characters, is things like react-native. I just tried and native, being a reserved keyword, doesn't work.

@jamesward
Copy link
Member

Oh fun. I'm working on: #2072

I wonder how hard it is to get to valid names. I think there could also be an issue if a part starts with a number? I'm assuming these names have to follow Java syntax rules?

@bowbahdoe
Copy link

Yeah, especially if we are generating them by running them through the compiler (but thats not a bad thing - to be used they need to work like that anyways).

For reserved keywords, adding a _ at the beginning or end would work. react._native or react.native_. For parts starting with numbers, adding to the beginning would work. react._456

@jamesward
Copy link
Member

jamesward commented May 2, 2024

Might be ugly, but what if we prefix and postfix everything with _ and replace invalid chars with _ ?
i.e. _react_native_
I can also collapse repeated underscores to a single underscore.

@bowbahdoe
Copy link

bowbahdoe commented May 3, 2024

🙉 🙈 🙊

(I am ultimately fine with any heuristic that works)

@jamesward
Copy link
Member

Ok, how's this: 7baa60c

@bowbahdoe
Copy link

bowbahdoe commented May 16, 2024

@jamesward Sorry - I keep getting pulled in different directions. Will be back to this soon

jamesward added a commit that referenced this issue May 28, 2024
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

No branches or pull requests

3 participants