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

Add module-info.java #2970

Open
hrchu opened this issue Oct 18, 2017 · 65 comments · May be fixed by #7094
Open

Add module-info.java #2970

hrchu opened this issue Oct 18, 2017 · 65 comments · May be fixed by #7094

Comments

@hrchu
Copy link

hrchu commented Oct 18, 2017

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
Copy link
Contributor

jbduncan commented Oct 18, 2017

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
Copy link
Member

cpovirk commented Oct 18, 2017

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
Copy link
Author

hrchu commented Oct 18, 2017

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
Copy link
Member

cpovirk commented Oct 18, 2017

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
Copy link
Author

hrchu commented Oct 19, 2017

@cpovirk could you tell me more about this problem?

@cpovirk
Copy link
Member

cpovirk commented Oct 19, 2017

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
Copy link

orionll commented Feb 20, 2018

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

@jbduncan
Copy link
Contributor

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

@orionll
Copy link

orionll commented Feb 20, 2018

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

@jbduncan
Copy link
Contributor

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

@HoldYourWaffle
Copy link

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

@jbduncan
Copy link
Contributor

@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
Copy link
Contributor

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

@jbduncan
Copy link
Contributor

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
Copy link

orionll commented Feb 20, 2018

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Contributor

jbduncan commented Dec 9, 2018

@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
Copy link

orionll commented Dec 9, 2018

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
Copy link

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
Copy link
Contributor

jbduncan commented Dec 9, 2018

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
Copy link

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
Copy link

talios commented Dec 11, 2018

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

sgammon added a commit to sgammon/guava that referenced this issue Mar 9, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
copybara-service bot pushed a commit that referenced this issue Mar 9, 2024
This is another baby step toward #2970.

RELNOTES=n/a
PiperOrigin-RevId: 614005269
copybara-service bot pushed a commit that referenced this issue Mar 9, 2024
This is another baby step toward #2970.

RELNOTES=n/a
PiperOrigin-RevId: 614005269
copybara-service bot pushed a commit that referenced this issue Mar 9, 2024
This is another baby step toward #2970.

RELNOTES=n/a
PiperOrigin-RevId: 614005269
copybara-service bot pushed a commit that referenced this issue Mar 9, 2024
This is another baby step toward #2970.

RELNOTES=n/a
PiperOrigin-RevId: 614287410
@sgammon sgammon linked a pull request Mar 10, 2024 that will close this issue
sgammon pushed a commit to sgammon/guava that referenced this issue Mar 10, 2024
This is another baby step toward google#2970.

RELNOTES=n/a
PiperOrigin-RevId: 614287410
sgammon added a commit to sgammon/guava that referenced this issue Mar 10, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
@sgammon
Copy link
Contributor

sgammon commented Mar 10, 2024

Hello everybody!

Firstly, wow, thank you for the support on that comment! As you've probably seen from this issue, there has been a lot of motion on this from Google, and they deserve a big thank you for this (especially @cushon and @cpovirk!).

I'm commenting here now because Guava with JPMS is ready to try. It's early, but the tests all pass and there are samples building successfully.

Current state of things

  • All upstream PRs are merged and on their way to release. This includes Checker Framework, J2ObjC and Error Prone.
  • Guava has a draft PR for JPMS; it will need review and some cleanup
  • All of these changes remain compatible with JDK 8, they are issued as multi-release JARs
  • There is now a custom Maven repository, called the JPMS Attic, that you can use to try all this safely

Library versions

The JPMS Attic repo has all of the below libraries, i.e. everything transitively needed to fully build Guava (+ potentially your library or app) via modular Java. The repository contents are fully on GitHub for easy inspection. They don't build on GitHub yet (I'd like to do this so there is clear provenance), but they will shortly.

Until official Maven releases, these libraries have versions suffixed with -jpms, in order to avoid conflicts with Maven Central. Here's the table of artifacts available now:

Coordinate Version
com.google.errorprone:error_prone_annotations 2.25.0-jpms
com.google.guava:guava 33.0.0-jre-jpms
com.google.j2objc:j2objc-annotations 3.0.0-jpms
org.checkerframework:checker-qual 3.43.0-SNAPSHOT

Dependency Verification

Obviously, these artifacts are not yet signed with Google's GPG keys. In the meantime I have signed them with mine. This matches commit signatures on GitHub for the changes applied. Key fingerprint is:

A166AC62F6DE8C63732277731D5903AF4582BCF4
Artifact digests
c2681472076c8da3883d5d94fb15b362acc7920e7885d53a652133450c6aa8e0  ./org/checkerframework/checker-qual/maven-metadata.xml
325419bd2c7b84445b6fafc136e9d59421b633f31022cabc4532c68f7cac8a8f  ./org/checkerframework/checker-qual/maven-metadata.xml.md5
2988c067d6d46dd5e65558d6d32172fd676cbcd6f95d2e9ac9e6c3a639f6e1f0  ./org/checkerframework/checker-qual/maven-metadata.xml.sha1
efcf78c2e21b6062e2b8113fcff6a5b7dd36ec555c82f049a7d4eed3338494c4  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/checker-qual-3.43.0-SNAPSHOT-sources.jar
dcb92f0425cbd5fa7d9b64b46ff484d7cb9f0ba8348277501ff1f902947f276c  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/checker-qual-3.43.0-SNAPSHOT-javadoc.jar
5ac2f9186d54139e804fc88f9cdf17034c19c499fd49ecb790bc417c3c47ad71  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/checker-qual-3.43.0-SNAPSHOT.pom
c5ae827b30115e8eb1251c4371a9b2f2c6892be3ded43d736c86b5cdc5d66edf  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/checker-qual-3.43.0-SNAPSHOT.module
a8224eb81224e809b741a3cecadc65be5bb3ea29964779fb120476311086f538  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/maven-metadata-local.xml
f9a749d838626b2ba60449e267c31fb4257405ad0b2ca03b77d4dce9a7a8d700  ./org/checkerframework/checker-qual/3.43.0-SNAPSHOT/checker-qual-3.43.0-SNAPSHOT.jar
760f8a465b22877547761227fa6fe9e77b68488841beb76283285b8775f862ed  ./com/google/guava/guava/maven-metadata.xml
8b267cd72e328f211c17a38890516ed2203ae877d905e9cdd15a95c632d818a1  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.jar.md5
920e137661a12a7ccb32933568a34e6a05be01d0de87c32ff73749b9f8c506d4  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.pom.md5
84626ef4bfbef95e2f9d7d0983a0e8829376697a7d07d6c3de328d445f9b88b8  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.jar.sha1
3ed27fa5bdc2589d1d1227c4c99c1db1c0117c7f30bd6acebddb1ddd58f960fd  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.jar
a7f41c1cfea3fd7eb0fc305a155b7d4931fc692a5c0a077d202d92afed14d82c  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.pom.sha1
9fe95275923730b89704c2fd6d85fd7a8880e688716a558aee61f8f5fb60895f  ./com/google/guava/guava/33.0.0-jre-jpms/guava-33.0.0-jre-jpms.pom
3c0b16b1c401730cd57519f4a9cdb459fab977634eb05396dec238308e06189d  ./com/google/guava/guava/maven-metadata.xml.md5
4c62d866a06ec05f7d7cc1299534229863f9e505afd7e78bd80fa9bdb0482e1e  ./com/google/guava/guava/maven-metadata.xml.sha1
511b91d2a05c18a0384491fb27ca6b1b3354e8dd7a5fe42a70d2f8058fb9e4b3  ./com/google/errorprone/error_prone_annotations/maven-metadata.xml
b9f01dc5b94ad3b25c84f82c53949c8e035d54f88fd839130f45507a53e5f6c4  ./com/google/errorprone/error_prone_annotations/maven-metadata.xml.md5
604f844f85afee7607b96a639e4b404f43a8431c964c87ca34c1dfed346fb2a2  ./com/google/errorprone/error_prone_annotations/maven-metadata.xml.sha1
081ed76568a21a1c23a2f70b793cad4003550ee8ece0f89974b7217af44dbf27  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.jar.md5
3b0060a8dec47f9a0281d381ef9431b3b2409ab211a93e1e79208f4239feb6ce  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.pom.md5
89a49c9861b01fecc123f469c856b4b2e166eecf7a1fdabdbc9c30f06e33a6d4  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.jar.sha1
8745e4577e4c23611b021641bbded603bfbac9edc322bec41623f909aefa88a1  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.pom
65fbc0149dca836a304bfb74b4c7d113d278ec8e4f83c9687cbeef130836bfa1  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.jar
bb48f614d531d53bde372b0d3f6e6bf8b8ec1787b3388f24db64600192b4b923  ./com/google/errorprone/error_prone_annotations/2.25.0-jpms/error_prone_annotations-2.25.0-jpms.pom.sha1
a813595eb6988d03fca84942769cddd7fc9ec324ba298240e5561c6abc31d807  ./com/google/j2objc/j2objc-annotations/maven-metadata.xml
caf413a656b17e817fb7a6750fa34423a0f9c59a8b630ba3ea065fdb6732e8ba  ./com/google/j2objc/j2objc-annotations/maven-metadata.xml.md5
07c18b57336a6c1e06c4c6810bd92442f5ceafbc7fdc52a964fd352d74a2a3a9  ./com/google/j2objc/j2objc-annotations/maven-metadata.xml.sha1
c9528d7c15ae180a24ff248ed189fd1e7fd12ed84ea7a8a81ef8d23dd1a1919d  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.pom.md5
e6f5e5c4471eedba428638b51475c54b789e9c695fd3905a11c5816107bc5fdb  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.jar.sha1
29994c04ca63f8f81d26ae36a1f90ff59454238dead14e4d33cc80af8fcc488f  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.jar.md5
cd20c532f794f30046a089354727b215788a7077bc3ae34bc6cab31d4ca7de81  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.pom
a9575f66820a1d03d78da53e2e60a290eaadbe2543837d09e6a28be18b17847e  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.jar
419b97e5e51386e6e301015262795ad212d0d6f698e7a99db35dc5029ddd6433  ./com/google/j2objc/j2objc-annotations/3.0.0-jpms/j2objc-annotations-3.0.0-jpms.pom.sha1

Trying it out

Here are some inlined instructions for using the JPMS Attic repo in your build. If something isn't covered here that you need, check the GitHub repo, file an issue, or let me know here with a tag:

Maven

There is a Maven sample used in integration tests

  <repositories>
    <repository>
      <id>jpms-attic</id>
      <name>JPMS Attic</name>
      <url>https://jpms.pkg.st/repository</url>
    </repository>
  </repositories>

Gradle

There is a Gradle sample used in integration tests

Groovy
repositories {
    maven {
        url "https://jpms.pkg.st/repository"
    }
}
Kotlin
repositories {
    maven {
        url = uri("https://jpms.pkg.st/repository")
    }
}

Other notes

Would you rather use a Maven BOM, or a Gradle Version Catalog? Do you need to constrain versions to these JPMS libraries transitively, using a Gradle Platform? All of these ancillary artifacts are also available from the JPMS attic repo.

sgammon added a commit to sgammon/guava that referenced this issue Mar 11, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
sgammon added a commit to sgammon/guava that referenced this issue Mar 11, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
sgammon added a commit to sgammon/guava that referenced this issue Mar 12, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
@ben-manes
Copy link
Contributor

There are probably PRs that we could accept, and I'd probably be willing to mess around to make sure that any patches work ... internally. But this is another case in which I don't want to give people the green light and then discover (only after people have done the work) that the changes are more disruptive than I'd expected.

@sgammon Please be aware of this comment and Guava's general stance on contributions. A lot of necessary work, but also racing well ahead of the maintainers and their priorities. It is often best to think of Google OSS as shared with the community, rather than driven by it, so as to realize it closely follows Google's internal needs rather than external ones. Your PRs are interesting and good work, but also quite large with Caliper to JMH migration, Java clean up, and lots of CI & build additions. Yet, most Googlers are only intimately familiar with Google's internal stack (Bazel, TAP) which also builds Guava for its usage. Therefore much of your work has to replicated into their internal stack and given a lack of a Google champion, that seems like a high bar.

I really like your ideas and direction but as a former Googler its hard for me to get bug fixes in from the outside. I'd recommend after proving it all out to split your work down into into small PRs with clear objectives, which could then be phased in incrementally as the Guava team has time. And certainly don't get demotivated by what I say, just be realistic that your work might be a PoC that guides them to support once that becomes a necessity.

@sgammon
Copy link
Contributor

sgammon commented Mar 12, 2024

Hey @ben-manes,

Firstly, big fan of your work... Gradle Versions, Caffeine, so many libraries. I built an entire company (partly) on Caffeine. Shameless plug for Buildless, which soon will be fully free forever for open source.

I hope you will allow me to respond out-of-order here.

Your PRs are interesting and good work [...] I really like your ideas and direction

Want to say ahead of the rest of this that I will frame this in my mind. You're a giant and that's very kind. Thank you!

It is often best to think of Google OSS as shared with the community, rather than driven by it, so as to realize it closely follows Google's internal needs rather than external ones

You could not be more correct and that is a very healthy way to think about it.

Please be aware of this comment and Guava's general stance on contributions. A lot of necessary work, but also racing well ahead of the maintainers and their priorities

I am aware, and I know that JPMS is arguably a new feature, even though it maybe shouldn't be considered as such. I could argue that we're not that ahead of the maintainers here, given that the Guava team has had this under consideration for some time (speaking about just JPMS here for a moment). Your point stands, though, regardless of those arguments, that JPMS is not a priority inside Google, at least for OSS, and @cpovirk has been clear and honest about that, to his great credit.

Luckily, I've contributed to libraries at Google before (though nothing as widely used as Guava), so this is not a surprise at all and I totally understand that Googlers straddle several (sometimes diverging) priorities. The first few of these PRs are already merged on Error Prone, J2ObJC, and on Guava.

but also quite large with Caliper to JMH migration, Java clean up, and lots of CI & build additions

This was and remains my fear as well, and so I have split off the JDK 21 and CI stuff entirely. Each stands on its own, and the CI stuff is entirely optional. If the Guava team chooses not to merge these, no worries, we can rebase the JPMS pull cleanly and move on.

That said, I think you're right about Caliper vs. JMH. During that work I actually did figure out that the benchmarks were running from Maven, though probably not... reporting anywhere or doing anything. In any case, they should probably be left alone so that internal Google tooling stays working. At the very least this should be another split point in the PR.

The bench changes, thankfully, were made very late in the PR and stand alone, so those can be split away cleanly.

And certainly don't get demotivated by what I say, just be realistic that your work might be a PoC that guides them to support once that becomes a necessity

This is excellent feedback and I deeply appreciate you taking the time to share it. I don't find it demotivating at all. In fact, I am only further motivated because the feedback that has poured in so far has been great, including yours. Some of it positive, some of it not, but all really really useful and helpful and engaged.

I am consuming these artifacts myself for my own projects from the JPMS attic repo, and they are working fine after a few tweaks, so my code is not blocked by this, and others have an option to try it out early, too. I think, all of the above considered, the Guava project can take their time and we (the group of people who care about this, in this issue) can test, refine, and eventually get a very clear plan going that minimizes breakage downstream.

Anyway, thank you for sharing your thoughts. Honesty is an expensive gift.

@jbduncan
Copy link
Contributor

@sgammon I'm in awe of your realistic and professional yet persistent approach to getting JPMS support and a better CI setup into Guava, as I'm acutely aware how Guava is more of a "throw over the fence" project than a community project, so well done and I wish you all the best moving things forward! 👍

@sgammon
Copy link
Contributor

sgammon commented Mar 12, 2024

@jbduncan Thank you! That's very kind of you, but I'm standing on the shoulders of giants here. This issue was filed nearly 10 years ago. We should make custom jackets.

Update: Taking orders

Front Back

@bowbahdoe
Copy link

@sgammon I'll take one, but maybe just GUAVA 2970 on the front. I don't actually want anyone to ask me questions

sgammon added a commit to sgammon/guava that referenced this issue Mar 13, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
sgammon added a commit to sgammon/guava that referenced this issue Apr 15, 2024
This changeset adds full support for modular Java builds in Guava,
and in libraries which depend on Guava.

The Guava JAR for JRE now structures as a Multi-Release JAR, with
a module definition situated in `META-INF/versions/9/`. Guava
remains compatible with JDK 8.

- feat: add `module-info.java` to `guava` module
- chore: update `guava` to build MRJAR
- chore: adjust dev version → `1.0-HEAD-[jre|android]-SNAPSHOT`
- chore: upgrade maven compiler plugin → `3.12.1`

Fixes and closes google#2970

Relates-To: elide-dev/jpms#1
Signed-off-by: Sam Gammon <[email protected]>
@cpovirk
Copy link
Member

cpovirk commented Jul 26, 2024

The toolchain setup of #7333 may lay the groundwork for continuing to run tests under JDK 8 even after we requires JDK 9 to build (at least if you want module-info). It may well be duplicative of existing work in #7094, which I still need to get back to :(

@sgammon
Copy link
Contributor

sgammon commented Jul 26, 2024

@cpovirk Thank you for the ping; no worries, I'm happy to rebase or construct it again from scratch if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.