rules_jvm_external icon indicating copy to clipboard operation
rules_jvm_external copied to clipboard

JPMS and automatic modules

Open c-goetz opened this issue 2 years ago • 4 comments

Hi,

I'm currently porting our maven build to bazel. Our application uses modules. Some of our dependencies aren't modularized so we refer to them with their automatic module name. One example would be jul-to-slf4j. When using this jar via maven_install and depending on the generated target, the jvm_import rule creates a new jar header_jul-to-slf4j with the Class-Path entry removed from the manifest. The new jar name changes the automatic module name of the dependency. So we get a compilation failure later on.

The automatic module name is generated either from the file name of the jar or from a manifest entry: Automatic-Module-Name.

To fix my current problem I'd add a parameter to maven_install sth. like add_automatic_module_name (boolean). I'd pass this along to the jvm_import rule and when true would use the automatic module name rule to generate it from the original jar file name and write it into manifest.

Another option would be to make this configurable on each dependency by extending maven.artifact with either a boolean parameter to use an automatic name from the jar file name or a string to just set the automatic module name.

Two questions:

  • Would you recommend this or do you have another idea?
  • Would you be interested in a PR for this?

Thanks for your work on rules_jvm_external and have a nice day, Carlo

c-goetz avatar Nov 19 '21 10:11 c-goetz

I don't have any experience with using automatic modules, so have to defer to @shs96c / @cheister here. We have no examples that use JPMS, so it would be great to have that as well.

Generally speaking, this sounds like something that we have to support given that jul-to-slf4j is a legitimate artifact from Maven repositories.

Another option would be to make this configurable on each dependency by extending maven.artifact with either a boolean parameter to use an automatic name from the jar file name or a string to just set the automatic module name.

This sounds more configurable, since artifacts like this sounds like the exception and not the norm, right?

jin avatar Nov 25 '21 03:11 jin

The automatic module name is in pretty wide-spread usage, so we should really support this out of the box without needing a flag. The modification of the header jar's manifest is done by //private/tools/java/rules/jvm/external/jar:AddJarManifestEntry so it should be simple enough to ensure that we preserve the module name if it's configured using the Automatic-Module-Name manifest entry (I'm slightly surprised that this is missing)

If the module name is being calculated from the jar file's name, I'd suggest raising an issue with the upstream project: relying on the jar name is inherently flaky and error prone.

Can you please confirm that the problem is that the Automatic-Module-Name is being stripped?

shs96c avatar Dec 23 '21 12:12 shs96c

Currently I have solved this with a small extension to AddJarManifestEntry to add an Automatic-Module-Name inferred from the jar filename according to javadoc. We are in the process of modularizing our build. But getting all our dependencies modularized is quite time consuming (we are at it for sth. like 2 years). So currently we have a mix of modules and regular jars. Generating the Automatic-Module-Name works but we run into issues further down the line because Bazel does not construct the module path and the class path in the correct way when mixing modules with regular deps. For now I have just disabled modules when building with Bazel. I'm still in the process of porting our Maven build to Bazel and I'm prioritizing the build itself higher than module support.

Regarding the issue with constructing the module path: before tackling this I'd like to wait for the changes mentioned int the Starlark Rules Roadmap talk from last BazelCon. It sounds like contributing will be a bit easier when this is done.

It'll be probably some months until I start working on the module support again, Can we leave this issue open until then or would you prefer to close it and me creating a new one when I have some news/solution for this?

@shs96c Automatic-Module-Name isn't stripped when it exists. The issue is that we have multiple dependencies that aren't modularized and also don't set Automatic-Module-Name in the manifest. Fixing this upstream would be the preferred solution, but it takes quite some time to get our PRs merged into upstream.

c-goetz avatar Jan 11 '22 13:01 c-goetz

Adding your own automatic module name seems like the most reliable thing to do right now, since files can then survive such small matters as a jar being renamed.

Bazel's java rules are unaware of the modulepath, so it's not a huge surprise that they get this wrong. The best thing for that may be to file issues with rules_java.

Once you come to generate modules, in the Selenium project, there's code to generate a module-info.class and having something like that in rules_jvm_external might be useful.

shs96c avatar Jan 11 '22 16:01 shs96c