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

Should layrry-launcher also have a module-info.java? #12

Open
AugustNagro opened this issue May 18, 2020 · 11 comments
Open

Should layrry-launcher also have a module-info.java? #12

AugustNagro opened this issue May 18, 2020 · 11 comments

Comments

@AugustNagro
Copy link
Contributor

You could then have the benefit of using the module path for the launcher as well. The exec plugin supports this (see bottom of page):

https://www.mojohaus.org/exec-maven-plugin/examples/example-exec-for-java-programs.html

@gunnarmorling
Copy link
Member

Yes, eventually it should. I was refraining from doing it so far, as I wasn't sure how well the Maven resolver stuff used by the launcher will work on the module path.

@aalmiray
Copy link
Contributor

If we keep shipping layrry-launcher as a shaded JAR then we either

  1. Skip adding a full module definition (module-info.java)
  2. Figure out a way to combine all of Layrry and its dependencies into a single module.

@sormuras
Copy link

Yes, layrry-launcher should be a Java module.

And it should provide a ToolProvider implementation (named layrry?) within its module declaration. This way Bach would support it "out-of-the-box" to launch modular applications.

@aalmiray
Copy link
Contributor

@sormuras then we'd have to rethink if layrry-launcher-<version>-all.jar (shaded artifact) should be published or not. If only there was a tool that would take a set of module descriptors and output a merged descriptor 😏

@sormuras
Copy link

sormuras commented Oct 28, 2020

Split 'em?

  1. layrry.launcher a 100% Java module - that requires other Layrry modules and behaves on the module path.
  2. An "all-in-happy-puppy-shaded-uber" JAR (that also contains the layrry.launcher module) with all deps included.

Use 1.) in other modules and 2.) on the console via java -jar layrry-all.jar ARGS

@aalmiray
Copy link
Contributor

aalmiray commented Oct 28, 2020

I guess for layrry-launcher-<version>-all.jar we'd strip module-info.class from the shaded jar, thus becoming just a regular fat jar that can be used with classpath, whereas the layrry-launcher-<version>.jar is modular, does not shade its dependencies, and can be used in the modulepath.

@sormuras
Copy link

sormuras commented Oct 28, 2020

Slightly off-topic: those shady -all.jar files (actually almost every JAR that was built by shading in another modular JAR) have already created soo many broken modules at Maven Central. For example, scan for "- :dvd: org.objectweb.asm -" in https://raw.githubusercontent.com/sormuras/modules/master/suspicious/impostors.md ... these artifacts all include the module-info.class from ASM. In various revisions. With modules directives that refer to package in the real ASM modules, but often not within theses modules. Impostors!

@aalmiray
Copy link
Contributor

Ye gods, the Shrinkwrap JARs contain split packages and there are no Auto-Module-Name directives in their manifests. The module descriptor for layrry-core looks like this

module org.moditect.layrry.core {
    exports org.moditect.layrry;

    requires org.moditect.layrry.config;
    requires shrinkwrap.resolver.api;
    requires shrinkwrap.resolver.api.maven;
    requires shrinkwrap.resolver.impl.maven;
    requires shrinkwrap.resolver.spi;
    requires shrinkwrap.resolver.spi.maven;
    requires directory.watcher;
    requires jdk.jfr;
}

And Maven barfs with

[WARNING] Can't extract module name from shrinkwrap-resolver-impl-maven-3.1.4.jar: Provider class org.jboss.shrinkwrap.resolver.spi.format.FileFormatProcessor not in module
[WARNING] Can't extract module name from shrinkwrap-resolver-api-3.1.4.jar: Provider class org.jboss.shrinkwrap.resolver.api.loadable.SpiServiceLoader not in module
[WARNING] ***************************************************************************************************************************************************************************************************************************************************************************************************
[WARNING] * Required filename-based automodules detected: [os-maven-plugin-1.6.2.jar, shrinkwrap-resolver-spi-maven-3.1.4.jar, shrinkwrap-resolver-spi-3.1.4.jar, shrinkwrap-resolver-api-maven-3.1.4.jar, directory-watcher-0.9.9.jar]. Please don't publish this project to a public artifact repository! *
[WARNING] ***************************************************************************************************************************************************************************************************************************************************************************************************
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 16 source files to /Users/aalmiray/dev/github/layrry/layrry-core/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /Users/aalmiray/dev/github/layrry/layrry-core/src/main/java/module-info.java:[21,33] module not found: shrinkwrap.resolver.api
[ERROR] /Users/aalmiray/dev/github/layrry/layrry-core/src/main/java/module-info.java:[23,38] module not found: shrinkwrap.resolver.impl.maven

@aalmiray
Copy link
Contributor

Psst @sormuras -> aalmiray@b2e382c

@gunnarmorling
Copy link
Member

So I'm still not sure what we'd actually gain by making the launcher a module? It's shading the dependencies currently, to have a single executable JAR, not requiring the user to fiddle with the classpath for the launcher itself. Happy about any suggestions for improvements, as long as usability doesn't suffer.

@aalmiray
Copy link
Contributor

aalmiray commented Nov 3, 2020

Making the launcher a module can help with #50
Given that Shrinkwrap has not seen much work lately I think we can't expect the project to move into proper modules, or at least to remove split packages. The alternative would be to shade all Shrinkwrap dependencies (and transitives) into a single JAR and have Layrry consume it. We could publish this shaded JAR as layrry-shrinkwrap-support or similar.

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

4 participants