search

google / guava

Google core libraries for Java
Apache License 2.0
49.92k stars 10.82k forks source link

Add module-info.java #2970

Open hrchu opened 6 years ago

hrchu commented 6 years ago

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.

jbduncan commented 6 years ago

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?)

cpovirk commented 6 years ago

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.

hrchu commented 6 years ago

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

cpovirk commented 6 years ago

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.

hrchu commented 6 years ago

@cpovirk could you tell me more about this problem?

cpovirk commented 6 years ago

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.

orionll commented 6 years ago

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

jbduncan commented 6 years ago

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

orionll commented 6 years ago

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

jbduncan commented 6 years ago

@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:

HoldYourWaffle commented 6 years ago

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

jbduncan commented 6 years ago

@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 commented 6 years ago

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

jbduncan commented 6 years ago

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.

orionll commented 6 years ago

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.

HoldYourWaffle commented 6 years ago

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.

SamCarlberg commented 6 years ago

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.

hannes-transentials commented 5 years ago

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.

jbduncan commented 5 years ago

@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!)

orionll commented 5 years ago

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.

hannes-transentials commented 5 years ago

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.

jbduncan commented 5 years ago

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. :)

overheadhunter commented 5 years ago

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.

talios commented 5 years ago

@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.

seinecle commented 4 years ago

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

cowwoc commented 4 years ago

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).

orionll commented 4 years ago

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:

cowwoc commented 4 years ago

@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.

cpovirk commented 4 years ago

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.)

jodastephen commented 4 years ago

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.

overheadhunter commented 4 years ago

because it broke downstream users

@jodastephen even with MR-Jars? I made the experience that it even works with old Android 4.x apps.

cowwoc commented 4 years ago

@jodastephen To @overheadhunter's point, a recent change to Maven makes it very easy to build multi-release JARs. Check out https://issues.apache.org/jira/browse/MCOMPILER-417 and https://github.com/apache/maven-compiler-plugin/blob/master/src/it/multirelease-patterns/singleproject-runtime/pom.xml#L102-L108

orionll commented 4 years ago

@jodastephen How exactly can module-info.class break the users? Do you have examples?

cpovirk commented 4 years ago

I know we have seen this internally: Some tools assume that they can read every class file in the classpath. If those tools are written for Java 8, they can have trouble with module-info. (Other people may have seen other problems.)

Such tools can and should be fixed. As a practical matter, though, adding a module-info to Java can break things, so we end up trying to weigh the costs and benefits.

cowwoc commented 4 years ago

Hey Chris. I'm not sure that module-info.class would actually be the classpath in this context. If we publish a multi-release JAR and the user is running JDK 8 (because the aforementioned tools break in newer releases) then module-info.class shouldn't show up at all.

It would help to look at a concrete testcase. I suspect we'll be okay.

jodastephen commented 4 years ago

Tools have been known to scan all class files in each jar file found on the classpath. This is hardly surprising, as the whole multi-release jar file concept is new, why would an old tool know about it.

@cpovirk, weighing the costs and benefits is easy. There are no benefits to library projects like Guava, only costs. The situation might change when Guava has Java 11 or 17 as a baseline. Adding an Automatic-Module-Name is quite sufficient for those who are desperate to use modules.

overheadhunter commented 4 years ago

Some tools assume that they can read every class file in the classpath. If those tools are written for Java 8, they can have trouble with module-info.

@cpovirk In case of multi-release-jars, the module-info.class would be stored at META-INF/version/{n}/module-info.class ({n} being 9 or 11 or whatever).

Even with Java ≤ 8 it would violate the jar file specification, if tools looked for classes inside of META-INF, so regardless of JPMS or newer Java versions, it is legitimate to file a bug, if they do so.

There are no benefits to library projects like Guava

@jodastephen If you came to this conclusion for your project, fair enough. But could you please not express personal opinions as if they were facts?

Especially libraries should carefully consider if they can support JPMS, as they block downstream adaption. The less transitive dependencies there are, the easier a module-info.java can be implemented. Downstream projects can not start adapting JPMS before all their dependencies do so. Automatic module names are just a first step, but don't facilitate all possibilities.

Not only for API design but also from a security perspective, properly restricting access to non-public APIs should be reason enough for project leads to deal with JPMS.

That said, I fully agree that "module information should have been textual, not a class file" for the reasons stated above. :wink:

cpovirk commented 4 years ago

If people have horror stories about, say, other libraries that have used non-public Guava APIs and thus broken their users, that would help motivate this. (But of course there's a limit to what modules can prevent until they're universally used -- and even then, people could hack around them.) I think we've heard of a library that did something along those lines with gRPC or protobuf or something in the past?

Based on what we've heard, though:

The more specifics that people are able to share, the better. We have a long track record of doing things that "should be safe" that cause users trouble that we ought to have foreseen :(

jodastephen commented 4 years ago

I can't stress enough the aspect that modules do not provide benefits to library authors.

Is adding module-info low effort? No. It quadruples the potential testing scenarios, makes the build much more complex (when based on Java 8) and you are quite likely to do it wrong first time and spend months fixing edge cases. Tools like Maven and Javadoc still have major issues with modular projects.

Does adding module-info stop users accessing an "internal" package? No. The jar file can be quite legitimately be used on the classpath instead of the module path, and no protection will occur. Users will still access "internal" packages. Giving yourself a false sense of security isn't a benefit.

Does it break downstream users? Yes, many and various reports of this. Sure the tools concerned might be buggy, but so what. Those users will still complain.

Who does modules benefit? Very few. They are of some minor benefit in a closed world, where everything uses the module system, everything is 100% correct (no mistakes in any module-info file, no split packages, no circular dependencies etc.) The best way to adopt modules would be in a large company that isolates developers from Maven Central and fully vets all open source. Such a centralised enterprise team could download each external jar, add a module-info, check split packages etc and ensure that only the internal modularised version is used within the enterprise. A large cost and probably impractical in reality, as most enterprise software isn't written well enough wrt split packages and circular dependencies to be able to use modules anyway!

I get that people want to use shiny new things, but sometimes shiny and new is not the same as good and useful.

cowwoc commented 4 years ago

@cpovirk

I don't personally care whether people can hack around JPMS boundaries. Publishing module-info helps hiding private APIs from the vast majority of users thanks to great IDE support and for the rest of the world the published Javadoc (which makes use of module-info itself) makes it quite clear that what they are doing is not supported. Let's optimize for the happy path, which is growing over time.

JPMS reminds me of Generics. Yes, it has its flaws but it will only get better over time. Do you want to be that guy who keeps on exporting raw types for years after Java 5 came out? Yes, people can still use your library but increasingly it becomes a pain in the behind to do so.

So again, let's talk about concrete issues. You keep on mentioning that some people are impacted by the use of broken tools. Can you provide the names and numbers of said tools? Can we count how many people are impacted by this? If there are issues, let's fix them and move forward.

overheadhunter commented 4 years ago

It is correct that split packages are a pain in the a** when using the module system. But is it right to demonize the module system or should we rather think about split packages? That said, Guava uses its own namespace so it will only collide with other 3rd parties that claim com.google.common, too. To state the obvious: This isn't Guava's problem.

The jar file can be quite legitimately be used on the classpath instead of the module path, and no protection will occur.

If the library is on the classpath, that is. Would this be a regression? No.

Btw you just made a great point for adding a module-info: Users are not forced to use the module system. If you need to stick to the classpath, a modular dependency will do no harm. Just put it on the classpath and you're fine. As I said before, we have a mr-modular-jar mixed with a lot of "normal" dependencies inside an Android 4.x app. The devs don't even notice the difference.

johnmanko commented 4 years ago 
        <dependency>
            <groupId>com.google.code.gson</groupId>
            <artifactId>gson</artifactId>
            <version>2.8.6</version>
        </dependency>
        <dependency>
            <groupId>com.google.guava</groupId>
            <artifactId>guava</artifactId>
            <version>29.0-jre</version>
        </dependency>
        <dependency>
            <groupId>javax</groupId>
            <artifactId>javaee-api</artifactId>
            <version>8.0</version>            
            <scope>provided</scope>
        </dependency>
module MyModule {
    requires com.google.gson;
    requires com.google.common;
    requires javaee.api;
    requires jersey.guava;
    exports com.my.base;
}
[ERROR] module jersey.guava reads package javax.transaction.xa from both javaee.api and java.transaction.xa
[ERROR] module com.google.errorprone.annotations reads package javax.transaction.xa from both javaee.api and java.transaction.xa
[ERROR] module com.google.common reads package javax.transaction.xa from both javaee.api and java.transaction.xa
cpovirk commented 3 years ago

A recent experience in JPMS adoption: https://github.com/ben-manes/caffeine/issues/535 -> https://github.com/ben-manes/caffeine/pull/540

rbygrave commented 2 years ago

I can't stress enough the aspect that modules do not provide benefits to library authors.

Well, having added module-info to a library that is together made up of 12 odd modules (plus another 8 modules that are upstream dependencies), for me I would say that module-info forced me as a library author to be more explicit in terms of "public api" (exports) and "friend api" (exports to) and overall I'd say it was a pretty significant improvement for the longer term and a really good push to tighten up and tidy up some areas. I actually think the module-info provides a really nice and explicit summary (requires, provides, uses, exports) of how things tie together.

If a library is just a single artifact / module sure, the benefit is a lot smaller but then module-info is also generally super easy in that case.

Who does modules benefit?

For me, I think it's a longer term thing. We get to be more explicit with "public api" which helps libraries longer term. Using module-path has got a LOT better in tooling (honestly, there was tooling pain but to me that has got a lot better).

My opinion on the benfits of java modules @jodastephen

jensli commented 1 year ago

Ping! Making Guava into an explicit module is important! Without a module-info Guava blocks the use of jlink.

cowwoc commented 1 year ago

I gave up on Google-authored libraries when it comes to Java9+ support. I hope they prove me wrong.

In the meantime, the following tool works great: https://github.com/moditect/moditect

cpovirk commented 1 year ago

It's been long enough that I wouldn't worry about lack of tool support anymore. (While I wouldn't be surprised if some tool still had a problem with modules, I wouldn't expect that to be common, nor would I expect for another year's wait to help.)

So it seems likely that users would be better off if we were a proper module. The next trick would be to actually make that happen:

It seems likely that that's enough obstacles that we still aren't going to make that happen anytime soon. The point at which it's worth having a deeper look is probably when the JSR-305 dep is finally gone.

charneykaye commented 1 year ago

We're forced to remove dependency on Guava due to lack of module support.

bowbahdoe commented 1 year ago

For anyone who is interested in using guava with the module system/java9+ support (which looks like @charneykaye , @jensli , @cowwoc, @johnmanko , @seinecle, and probably someone reading this issue) I made a script which shades the source code and makes a series of modular jars

github.com/bowbahdoe/guava

Version 0.0.3 is taken from the current latest release of guava and i'll keep it up to date as I can. That may or may not serve your purposes, at least until all the problems here are worked out. You can reach out to me directly or on any forum if you want (but lets not clutter this issue)

But, relevant to this issue (and I think adding a module-info here is pretty important, downstream libraries need the "base" to be modular before they make the same leap) I discovered some things that are blockers in addition to the JSR-305 thing, as well as general "future java issues".

So it seems like no matter what, even ignoring module-info, guava is going to have to provide a multi-release jar. The removal of the security manager will make current guava builds totally unusable eventually and there is no way to keep supporting java 8 without using it.

If anyone is looking for work outside of what @cpovirk is doing w.r.t. the JSR-305 dep. and (I assume) jspecify, doing the build plumbing for a multi-release jar feels valuable.

cpovirk commented 1 year ago

Thanks for all the investigation.

Random responses: