Use manifest entry for java 9 module name#3743
Conversation
|
|
It seems like we are happy that the names of commonly used modules are unique enough e.g. okhttp, mockwebserver, okhttp-urlconnection. But at the moment just renaming the jar files will change the names. Defining these based on advice here
|
swankjesse
left a comment
swankjesse
left a comment
There was a problem hiding this comment.
Looks reasonable, though I must admit I don't grasp the benefits or consequences. What happens if we just sit out of Java 9 modules? I’m anxious about the way this plays out for libraries like Moshi that do more aggressive reflection.
|
We get the same automatic names as the default names, but things break if someone renames a JAR file. These are not great names (namespaced etc), but I think the Jars that people use externally are good enough. |
|
Let's agree this before landing. I'm ok with either, but want this at minimum so a simple jar file rename doesn't change the module names. |
|
I agree these are not sufficient. I would do com.squareup.okhttp3 and com.squareup.okhttp3.mockwebserver, etc. |
yschimke
force-pushed
the
module_names
branch
from
8fc5710 to
faf2ffc
Compare
|
okhttp-urlconnection/pom.xml
Outdated
| <configuration> | ||
| <archive> | ||
| <manifestEntries> | ||
| <Automatic-Module-Name>${project.groupId}.urlconnection</Automatic-Module-Name> |
There was a problem hiding this comment.
I’m trying to find out whether we can have two modules, where one is OkHttp and the other is this one, and their subpackages overlap
|
n.b. I only added for jar files that there was a valid reason for someone to use externally. e.g. apache, urlconnection, logginginterceptor, core. |
|
My only question I can’t find an answer to online – is it allowed to have one module contain subpackages of another? |
|
Good question, n.b. this change doesn't actually change what is exported etc. Just continues the default automatic rules but with a specified module name. I don't know the answer, but I'm surprised if subpackages are a thing in Java 9 modules. Mostly the JVM seems to handle package names as arbitrary keys without any hierarchy rules. |
|
@jodastephen might be able to shed some light if he has some time? :-) |
|
It is a requirement of the Java module system that each package is in a single module. This is one reason why you would never turn a "jar-with-dependencies" into a module. If you have the same package in 2 jar files, you need to fix that before declaring module names. This appears to be the case with Subpackages don't exist in the JVM. Each package has a name - humans give them subpackage meaning, but the JVM does not. Your module name should be the same as the package name in that jar file, unless you have a good reason otherwise. ie. you as a human are deciding that one package is the base, and assigning that as the module name. Thus the I think using BTW, you can add a |
|
@jodastephen Am I correct that the current setup is strictly better than defaulting from JAR file names? i.e. is it worth landing something close to this, or deferring until a bigger rewrite for Java 9? In which case, we could just agree the automatic name for okhttp-core and a few other usable libraries and ship this for now. OkHttp could be suitable for modularisation if clients only use core and not urlconnection.
And that resolving overlap is most relevant when adding module-info.java files?
|
|
It is best if the clashing packages should be resolved before including module names. If you don't, then users will have problems if one or both of the jar files is on the module-path. (The clashing package check does not occur if both are on the class-path, however Maven will put them on the module-path, because of the Automatic-Module-Name. How hard is it to solve the clashing package... |
|
@jodastephen What I'm confused about is why this PR is worse than the current situation. At least this gives a stable name to a widely used library - OkHttp Core. The current release already has automatic names, they are just mutable. See okhttp.urlcnxn, in the renamed jar file. |
swankjesse
left a comment
swankjesse
left a comment
There was a problem hiding this comment.
I think this is right
okhttp-logging-interceptor/pom.xml
Outdated
| <configuration> | ||
| <archive> | ||
| <manifestEntries> | ||
| <Automatic-Module-Name>${project.groupId}.logginginterceptor</Automatic-Module-Name> |
There was a problem hiding this comment.
logginginterceptor –> logging ?
There was a problem hiding this comment.
we use logging-interceptor in the module name but just logging in the package name
okcurl/pom.xml
Outdated
| <configuration> | ||
| <archive> | ||
| <manifestEntries> | ||
| <Automatic-Module-Name>${project.groupId}.okcurl</Automatic-Module-Name> |
There was a problem hiding this comment.
I think we want curl here, to match the package name of okhttp3.curl
|
I think we want |
|
Actually, lets keep this ’cause it’s consistent with our packages. I just merged the Okio one but I’ll change it to be |
|
Yeah, I took the recommendation above
|
|
The module names in the merged PR look OK to me, although I'm still concerned by the clashing packages. |
This is the easy way out of getting module names stable and mapped to our entrypoint packages. I've only done this for the highly re-used jars. See square/okhttp#3743 thx @yschimke
This is the easy way out of getting module names stable and mapped to our entrypoint packages. I've only done this for the highly re-used jars. See square/okhttp#3743 thx @yschimke
This is the easy way out of getting module names stable and mapped to our entrypoint packages. I've only done this for the highly re-used jars. See square/okhttp#3743 thx @yschimke
This is the easy way out of getting module names stable and mapped to our entrypoint packages. I've only done this for the highly re-used jars. See square/okhttp#3743 thx @yschimke
This is the easy way out of getting module names stable and mapped to our entrypoint packages. I've only done this for the highly re-used jars. See square/okhttp#3743 thx @yschimke
5 participants
No description provided.