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 #2649

Closed
cowwoc opened this issue Oct 26, 2021 · 5 comments
Closed

Add module-info.java #2649

cowwoc opened this issue Oct 26, 2021 · 5 comments

Comments

@cowwoc
Copy link

cowwoc commented Oct 26, 2021

JPMS users who depend on this library get the following build warning:

[WARNING] requires transitive directive for an automatic module

Can you please publish a multi-release or exclusive JAR with module-info for users of Java 9+?

@cowwoc
Copy link
Author

cowwoc commented Oct 26, 2021

I tried ignoring the warning and ended up with an error later on in the build:

[ERROR] the unnamed module reads package org.checkerframework.dataflow.qual from both org.checkerframework.dataflow and org.checkerframework.checker.qual

and even more annoyingly:

[ERROR] the unnamed module reads package com.google.errorprone from both error.prone.core and error.prone.annotation

Is there a workaround for this (without dropping the use of JPMS in my own library)? Or am I stuck until module-info is added here?

@cowwoc
Copy link
Author

cowwoc commented Oct 26, 2021

It looks like this library cannot be used at all with JPMS due to the existence of split packages. See #1210

@cushon
Copy link
Collaborator

cushon commented Feb 4, 2023

A couple of notes:

We only support JDK 11 and newer at this point, so we could add a module-info without the need for multi-release jars.

The annotations jar is now using Automatic-Module-Name: 5793bca

@sgammon
Copy link
Contributor

sgammon commented Mar 8, 2024

@cowwoc There's a PR for this now at #4311.

copybara-service bot pushed a commit that referenced this issue Mar 8, 2024
## Summary

Minimal set of changes to ship a `module-info.java`, and support JVM 1.8 for the `annotations` module only.

### Reasoning

It costs almost nothing to support _only_ the annotations on JVM 1.8, for projects which transitively include `error-prone-annotations`, which can happen via a simple dependency on something like Guava. But, it would be ideal to [ship a `module-info.java`](#2649), so `annotations` is now a `Multi-Release` JAR, with a `module-info.class` in `META-INF/versions/9`.

I am working downstream to [support JPMS in Guava](google/guava#2970), and this PR will be necessary for that to eventually land (without hackery). [Checker Framework recently merged their support](typetools/checker-framework#6326) which means Error Prone is one of the last things on the classpath for Guava without a `module-info.java` definition.

Automatic Modules also aren't the answer because they cannot be used with `jlink`. Such dependencies must be processed by tools like Moditect before they can be used in fully modular Java builds. Because Error Prone Annotations is a dependency in Guava, and is itself very popular, many projects need Error Prone to adopt JPMS so they can ship their own code with modular Java support.

There may be libraries out there that depend on the annotations _and not the compiler_, who wish to ship a MRJAR and retain JVM 1.8 support (Guava).

### Non-release version number change

One wrinkle here is that the Maven project version [is used unconditionally](https://github.com/apache/maven-compiler-plugin/blob/74cfc72acae4f55708bca189b2170167e83df6b3/src/main/java/org/apache/maven/plugin/compiler/CompilerMojo.java#L304-L305) [as the `--module-version`][0]; however, [`HEAD-SNAPSHOT` is not a valid module identifier](https://lists.apache.org/thread/qtghq13qgjoc4pn6ovsdxgnjnm4ozv4d). This leaves only a few choices to support JPMS through recent (`3.8.x+`) Maven Compiler plugin versions:

- `HEAD-SNAPSHOT` needs to become `1.0-HEAD-SNAPSHOT`, and then it is a valid `--module-version`
- Or, Maven Compiler Plugin needs to ship a bugfix and Error Prone will need to wait for a release (if a bugfix ships at all)

I understand this can be a breaking change somewhere within Google, but I don't see a way around this without resorting to severe build hacks. I've gotten it to build anyway by building the `module-info.java` only in a separate Maven project, and then copying it into the JAR at the last moment, but there are serious issues with that route so I suggest it be abandoned in favor of a version string change during dev, if possible.

## Changelog

- feat: add `module-info` to `annotations` module
- feat: ship `annotations` as a `Multi-Release` JAR
- feat: support `1.8` through latest JDK for `annotations` module
- fix: remove automatic module definition from `annotations` module
- fix: `HEAD-SNAPSHOT` → `1.0-HEAD-SNAPSHOT`, because of a Maven Compiler Plugin issue [precluding use as a module version][0]

Fixes and closes #2649

[0]: https://issues.apache.org/jira/browse/MCOMPILER-579

Fixes #4311

FUTURE_COPYBARA_INTEGRATE_REVIEW=#4311 from sgammon:feat/jpms d209b0c
PiperOrigin-RevId: 614005681
@cushon cushon reopened this Mar 9, 2024
@cushon
Copy link
Collaborator

cushon commented Mar 9, 2024

I think a little more work is needed here: #4314

@cushon cushon closed this as completed Mar 11, 2024
benkard pushed a commit to benkard/jgvariant that referenced this issue Mar 12, 2024
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [com.google.errorprone:error_prone_core](https://errorprone.info) ([source](https://github.com/google/error-prone)) |  | minor | `2.25.0` -> `2.26.1` |
| [com.google.errorprone:error_prone_annotations](https://errorprone.info) ([source](https://github.com/google/error-prone)) | compile | minor | `2.25.0` -> `2.26.1` |

---

### Release Notes

<details>
<summary>google/error-prone</summary>

### [`v2.26.1`](https://github.com/google/error-prone/releases/tag/v2.26.1): Error Prone 2.26.1

[Compare Source](google/error-prone@v2.26.0...v2.26.1)

Changes:

-   Fix the module name of the annotations artifact: `com.google.errorprone.annotations` (google/error-prone@9d99ee7)

Full Changelog: google/error-prone@v2.26.0...v2.26.1

### [`v2.26.0`](https://github.com/google/error-prone/releases/tag/v2.26.0): Error Prone 2.26.0

[Compare Source](google/error-prone@v2.25.0...v2.26.0)

Changes:

-   The 'annotations' artifact now includes a `module-info.java` for Java Platform Module System support, thanks to [@&#8203;sgammon](https://github.com/sgammon) in [#&#8203;4311](google/error-prone#4311).
-   Disabled checks passed to `-XepPatchChecks` are now ignored, instead of causing a crash. Thanks to [@&#8203;oxkitsune](https://github.com/oxkitsune) in [#&#8203;4028](google/error-prone#4028).

New checks:

-   [`SystemConsoleNull`](https://errorprone.info/bugpattern/SystemConsoleNull): Null-checking `System.console()` is not a reliable way to detect if the console is connected to a terminal.
-   [`EnumOrdinal`](https://errorprone.info/bugpattern/EnumOrdinal): Discourage uses of `Enum.ordinal()`

Closed issues: [#&#8203;2649](google/error-prone#2649), [#&#8203;3908](google/error-prone#3908), [#&#8203;4028](google/error-prone#4028), [#&#8203;4311](google/error-prone#4311), [#&#8203;4314](google/error-prone#4314)

Full Changelog: google/error-prone@v2.25.0...v2.26.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants