guava icon indicating copy to clipboard operation
guava copied to clipboard

Add module-info.java

Open hrchu opened this issue 7 years ago • 65 comments

So that projects depend on this can be published to a public artifact repository. Note that this is not breaking backward compatibility. All codes except this file can be still compiled in Java 6.

hrchu avatar Oct 18 '17 07:10 hrchu

Guava has an Automatic-Module-Name in its MANIFEST.MF now, so I believe this is not quite as important as it may seem. But if I'm mistaken, then I'd be more than happy to be proven wrong.

(BTW, I think I might have misunderstood something, because module-info.java is Java 9 specific, right? Would a Java 8 compiler (which I believe is what is used to compile guava-jre) or an Android compiler (for guava-android) happily process it or otherwise ignore it?)

jbduncan avatar Oct 18 '17 08:10 jbduncan

Our current thinking is that we'll look into this next quarter. We have seen some problems from module-info files in third-party jars, since they're Java 9 .class files and not everyone uses Java 9 yet. So, if we can get away with Automatic-Module-Name, we'll continue to do so, possibly even beyond next quarter. But if there are cases in which Automatic-Module-Name isn't good enough, that would be good to know.

cpovirk avatar Oct 18 '17 11:10 cpovirk

It is possible to only compile modile-info.java in Java 9, so the jar file is still compatible to users who uses earlier Java version.

A maven example: https://github.com/twonote/radosgw-admin4j/blob/java9/pom.xml#L127

hrchu avatar Oct 18 '17 12:10 hrchu

Right, thanks. We've seen problems even when the main .class files are compiled for an older version but the module-info.class is present. As I understand it, various tools try to process all .class files, and they need to be updated to ignore module-info.class.

cpovirk avatar Oct 18 '17 14:10 cpovirk

@cpovirk could you tell me more about this problem?

hrchu avatar Oct 19 '17 01:10 hrchu

I wasn't personally involved in fixing the problems, but the basic idea seems to be that people scan the whole classpath (using something like ClassPath.getTopLevelClasses -- which might be an example of something that needs updated to ignore module-info.class :)) and then try to examine/load the classes with a tool that understands only, say, Java 8.

cpovirk avatar Oct 19 '17 11:10 cpovirk

It's worth mentioning that if we add module-info.java, all packages will not be open anymore.

orionll avatar Feb 20 '18 05:02 orionll

@orionll Am I right to think/remember that in the JPMS, open packages are packages whose internal classes can be inspected with reflection?

jbduncan avatar Feb 20 '18 15:02 jbduncan

@jbduncat Yes, exactly. And also private members of public classes.

orionll avatar Feb 20 '18 16:02 orionll

@orionll Cool, thanks for confirming things for me. :)

I personally wonder how important it would be for Guava's packages to be open when used on the module path. I struggle to imagine that reflectively calling Guava's internals is a common thing to do, especially considering Guava's (IMO) pretty durn good API. :thinking:

jbduncan avatar Feb 20 '18 16:02 jbduncan

Are there any reasons for it not being open? Even if it's uncommon it might still be done by some people.

HoldYourWaffle avatar Feb 20 '18 16:02 HoldYourWaffle

@HoldYourWaffle I think the main reason is it prevents people from using reflection to depend on internals which may change or disappear in future releases of Guava without warning.

jbduncan avatar Feb 20 '18 16:02 jbduncan

...which by my understanding makes things easier for everyone in the long-run.

jbduncan avatar Feb 20 '18 16:02 jbduncan

The only reason I can currently think of to have Guava's packages open in the module-info.java is if frameworks like Spring need to reflectively access its internals to do important stuff, but I don't know how important or common that is.

jbduncan avatar Feb 20 '18 16:02 jbduncan

All Guava packages should be closed because as @jbduncan said the dependence on class internals is a bad practice. If someone really wants to access the internals, they can use --add-opens command line option which forces the specified package to be open.

orionll avatar Feb 20 '18 16:02 orionll

Good point I forgot about --add-opens. Imho you should always be able to hack into internals (maybe you want to do something the developers didn't think of but it's too 'personal' that it's not worthy of a PR), with the risk of it breaking in new versions. --add-opens allows for this so it'd indeed be better to close the guava packages.

HoldYourWaffle avatar Feb 20 '18 17:02 HoldYourWaffle

Guava has an Automatic-Module-Name in its MANIFEST.MF now, so I believe this is not quite as important as it may seem. But if I'm mistaken, then I'd be more than happy to be proven wrong.

It does mean that jlink will not work, since the tool requires a module name to be specified in the module-info.java file; automatic module names will not be accepted.

SamCarlberg avatar Aug 17 '18 21:08 SamCarlberg

As much as I love Guava and appreciate Google's efforts, it is somehow embarrassing that a company like Google is not able to adopt Java modules within one year.

Either Google does not use Guava internally or they keep using JDK 8 and won't adopt Jigsaw.

hannes-transentials avatar Dec 09 '18 11:12 hannes-transentials

@hannes-transentials I think it's most likely that Google have not migrated to JDK 11 and adopted modules yet simply because their internal codebase is so mind-bogglingly humongous. ;)

I say this because I remember reading somewhere (or I inferred) that they use Guava or an superset internally, and I also remember they announced a few years ago that they'd finally migrated to JDK 8 after a lot of effort. So I'm sure that they'll announce support for JDK 11 or a later LTS version (and, by extension, modules) when they fully migrate away from Java 8 and when they feel that most of us non-Googlers have moved away from Java 8 too. (I know that my company hasn't done so yet simply because Java 9 was such a freaking big, backwards-incompatible change!)

jbduncan avatar Dec 09 '18 12:12 jbduncan

It's worth mentioning that adding module-info.java can break some existing tools. For example, I know that IDEA's Osmorc plugin does not work when both module-info.java exists and the library is an OSGi bundle (Guava is). So, while the tools are not ready yet, I would abstain from adding module-info.java to Guava.

orionll avatar Dec 09 '18 12:12 orionll

So I either do without modularized applications or stay away from Guava (and many other popular applications)? I somehow hoped that there was some middle ground.

hannes-transentials avatar Dec 09 '18 14:12 hannes-transentials

Well... you can use Guava as a module in a vanilla-Java modular application. But since Guava only includes an Automatic-Module-Name in its manifest, rather than going further and including amodule.info (out of necessity to stay Java-8-compatible), you won't be able to use it with jlink to create minimised modular Java applications.

Furthermore, frameworks built on top of Java that have their own programming models, like Spring, may have not fully migrated to be Java-11-compatible yet, so if you use such frameworks a lot, you may have to wait a bit longer.

That being said, if you do use a framework such a Spring, please check for yourself if Java 11 and modules work with it, since my knowledge of Spring and other frameworks is limited. :)

jbduncan avatar Dec 09 '18 14:12 jbduncan

out of necessity to stay Java-8-compatible

Well you can create multi-release jars, where the module-info.class file only gets included inside META-INF/versions/9 and is therefore invisible for old¹ class loaders.


¹ As long as no fancy custom class loaders eagerly load everything they find in a jar without reading the manifest entries.

overheadhunter avatar Dec 10 '18 15:12 overheadhunter

@hannes-transentials You could make use of something like https://github.com/moditect/moditect to adapt guava and add a module-info.java/.class at your applications side of things as a transitionary work around.

If module-info.java was to be added to guava, hopefully it'd be done so as a modular jar so we don't break java 8< versions.

talios avatar Dec 11 '18 02:12 talios

So Guava is still not natively JPMS compatible in May 2020?

seinecle avatar May 11 '20 08:05 seinecle

No. And it isn't hard to add support without breaking pre-Java9 support. As @overheadhunter mentioned, all they need to do is add META-INF/versions/9/module-info.class (multi-release JAR) and it would be compatible with both versions.

The easiest way to provide Java9 support is to release a separate artifact for it but if they don't want to maintain multiple artifacts then the multi-release JAR approach would work as well (it's just harder to build).

cowwoc avatar May 11 '20 12:05 cowwoc

So Guava is still not natively JPMS compatible in May 2020?

Yes, it's not 100% compatible, but it has an Automatic-Module-Name attribute which is better than nothing.

all they need to do is add META-INF/versions/9/module-info.class

This is very simple on first sight, but when you actually start implementing this, you will be surprised by the number of unexpected issues you will need to solve:

  • How to compile module-info.java to module-info.class? You will need Java 9 for this, and the rest of the classes will be compiled with --release 8. This is achievable but this is not trivial to implement in the build system.
  • What the module declaration should actually contain? It should probably export all packages, but should it open all packages? Currently, guava is an automatic module, so it exports and opens everything. If we gonna change this, how much user code will be broken? For sure, some users are using reflection on Guava classes, so they will be upset when their code stops working.

orionll avatar May 12 '20 05:05 orionll

@orionll

  1. I recently did this for Hikari (https://github.com/brettwooldridge/HikariCP/pull/1573) and it turns out that maven-compiler-plugin recently added support that makes this even easier (https://github.com/metlos/multi-release-jar-maven-plugin/issues/3#issuecomment-403975603). You can use these as inspiration for the kind of changes needed in Guava.

  2. Start with exports most/all packages. If people need open let them request it. The only thing we technically promised end-users are public methods. If they are using reflection to access private members then I think it is reasonable to chance breaking their code.

cowwoc avatar May 12 '20 13:05 cowwoc

Thanks to everyone who's been contributing details here.

I thought I had some memory that @jodastephen used to recommend Automatic-Module-Name over a full module-info (though I figured things might have changed since then). But in fact I haven't found any such recommendation, whether in what he said on our issue tracker or on his blog.

[edit: note also module-info in threeten-extra]

The closest thing I see to a recommendation against is that he recommends waiting until...

all of your runtime dependencies have been modularised (either as a full module or with a MANIFEST.MF entry)

We should figure out if our annotation-only dependencies matter, as I don't know that they've all been modularized (though we could probably push for it, and we control some of them ourselves).

(I could see starting with either exports (which makes the most sense) or open (which seems less likely to immediately break people), possibly moving from one to the other over time.)

cpovirk avatar May 12 '20 19:05 cpovirk

Personally I don't think there is much to gain from adding module-info. With my own projects I had to release a parallel jar file without it because it broke downstream users. The time to add module-info.java is when Guava moves to Java 11 or 17 as a baseline.

jodastephen avatar Jul 02 '20 20:07 jodastephen