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

[Maybe] Change mechanism to determine the corrdinartes of a Jar #145

Open
jjohannes opened this issue Sep 21, 2024 · 4 comments
Open

[Maybe] Change mechanism to determine the corrdinartes of a Jar #145

jjohannes opened this issue Sep 21, 2024 · 4 comments
Labels
a:enhancement New feature or request

Comments

@jjohannes
Copy link
Member

Split out from #129

Right now, for the transform input, we only know the file name. But we would like to know the G:A coordinates from which the Jar was downloaded. So that the user can use these instead of the Jar file name when defining a patch rule.

To determine the G:A, we look at the file path which (usually) contains the G:A information.
This is hacky, but the best known option.

It is a missing Gradle feature: gradle/gradle#11831

This issue is here to record this detail in the implementation of this plugin. Maybe it can be improved with a future Gradle version.


If you run into an issue with this, where you define a rule with coordinates. E.g.:

module("some.group:some.name", "some.name")

And the plugin does not work because the Jar files name or its location is not following the expected structure, you may use the file name of the Jar as alternative:

module("some-name-1.2.jar", "some.name"

This should not happen in normal Java project setups. It may be happening if you combine this plugins with others which also register transforms and put he transform of this plugin into a transform chain.

@satsen
Copy link

satsen commented Oct 11, 2024

I use a library which brings in transitive scala dependencies and all of those are named with the jar like: "circe-core_2.12-0.13.0.jar". So it gets converted to circe.core.2.12 which is invalid. I have to manually add automaticModule for all of these, so if the module name generator could be configurable or at least support this it would be very useful. Gradle without this plugin also generates those invalid names (I'm not sure if it's Java itself or Gradle generating those).

@jjohannes
Copy link
Member Author

@satsen do I understand correctly that this is about using deriveAutomaticModuleNamesFromFileNames = true? And influencing how the name is derived?
That's a different topic than this one. If we do anything, we should open a separate issue for that. But let's clarify first.

The effect you see without using the plugin is Java behavior: There is no way to get these Scala Jars working with the Module System without patching (or renaming) them. You can find the code that determines the name in the Java sources in jdk.internal.module.ModulePath#deriveModuleDescriptor.

If you do deriveAutomaticModuleNamesFromFileNames = true the plugin does the same, but at build time. This was added as a convenience, to get Gradle to put all Jars on the Module Path without further configuration (by default, Gradle puts such Jars on the Classpath aka into the Unnamed Module).

I was a bit hesitant to add this feature add all, but eventually added it as it was requested by a user.
I personally do not see much value anymore in using the Module System with Automatic Modules. Most features of the Module System are not usable once Automatic Modules are part of the story.
That's why this plugin allows you to add a real module-info.class to Jars that miss one. That's some more configuration effort, although by now this plugin offers a lot of features to keep it compact (like exportAllPackages() and requireAllDefinedDependencies()).

But I certainly don't know all the use cases and I understand that (maybe as an intermediate step) Automatic Modules are helpful in certain setups.

If we do something as you request, I would like to keep it simple. I could imagine a configuration option that takes the same arguments as Java's String.replaceAll() and then applies that on the derived name.
For example, in

deriveAutomaticModuleNamesFromFileNames = true
deriveAutomaticModuleNamesReplaceAll("\\.[0-9]+", "") // same semantics as String.replaceAll("\\.[0-9]+", "")
deriveAutomaticModuleNamesReplaceAll("circe\\.", "io.circe.")
// possibly more "deriveAutomaticModuleNamesReplaceAll()", will be applied in definition order

The above example would do:
circe-core_2.12-0.13.0.jar --name derivation-> circe.core.2.12 --replaceAll--> circe.core --replaceAll--> io.circe.core

@satsen
Copy link

satsen commented Oct 15, 2024

Yes it is about deriveAutomaticModuleNamesFromFileNames. I checked this plugin's source code and found the method that generates the names so I wonder if it could be configurable. Like replacing the method you know? I'm not an expert regarding modules, I just want to use modules sometimes and these annoying JARs that are added by other dependencies are making it annoying. Aren't automatic modules the same as running jdeps and requiring all those? While it could definitely require too much and also miss dynamically used things, it seems ok for some annoying transitive dependencies.

@jjohannes
Copy link
Member Author

Thanks for clarifying @satsen. I have created a separated issue for that and described the solution: #149

The plugin can't just make the method that computes the name exchangeable. This is due to the way how the Jar transformations are plugged in using Gradle's Artifact Transforms concept. Basically, everything we make configurable in the transform needs to be some sort of "data" that is serializable, as Gradle is using that to cache transform results and check if the cache is still valid. Hence my suggestion based on replaceAll wit a regex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants