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 Automatic-Module-Name to artifacts, particularly javac #1116

Closed
paulduffin opened this issue Sep 10, 2018 · 3 comments
Closed

Add Automatic-Module-Name to artifacts, particularly javac #1116

paulduffin opened this issue Sep 10, 2018 · 3 comments

Comments

@paulduffin
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

What version of Error Prone are you using?

I'm using the javac artifact, version 9+181-r4173-1.

Does this issue reproduce with the latest release?

AFAIK it is the latest.

What did you do?

Tried to use the above from a Java 9 module-info.java file. I added the above to the module-path (using gradle so had no control over the jar name) so it would be treated as an automatic module.

What did you expect to see?

That classes would be available with a useful module name.

What did you see instead?

Java 9 reported that the classes could not be found but were in module javac.9.181.r4173.1. That is not a valid module name so it was impossible to add a "requires javac.9.181.r4173.1" to the module-info.java file to gain access to the classes.

I worked around it by adding an Automatic-Module-Name entry in the manifest (used com.google.errorprone.javac) so that would be used instead of java trying to construct one from the jar name. That would be a minimum viable solution. Alternatively, adding a complete module-info.java would work too as long as all the internal classes were exported.

It would also be helpful if the other artifacts were fixed in a similar way.

@cushon
Copy link
Collaborator

cushon commented Sep 10, 2018

Thanks, there's a related bug for google-java-format's javac dependency: google/google-java-format#266

@tbroyer
Copy link
Contributor

tbroyer commented Sep 10, 2018

On Error Prone side, javac is only needed when using Java 8; I'd rather remove it as a dependency from error_prone_core (and adapt Maven config to add an explicit (plugin) dependency on it)
WDYT?

cpovirk added a commit that referenced this issue Oct 4, 2019
This makes it practical to use from code that uses modules.

Compare to Guava:
https://github.com/google/guava/blob/5496c37d4d904869297c2ced1f0d20e6f1507eaa/guava/pom.xml#L60
google/guava#2920

This is a step toward #1116 (though it doesn't address most artifacts, including javac).

Modules users may still see problems because com.google.errorprone.annotations is split between error_prone_annotations and error_prone_type_annotations. I wonder if the best solution to that is going to be to merge the two into error_prone_annotations, turning error_prone_type_annotations into an empty "forwarding" artifact. That would require compiling some files for Java 7 and some for Java 8, but that's doable (if a little clumsy). It might cause problems from some tool somewhere, though. Given that error_prone_type_annotations isn't nearly as commonly used, maybe you can wait it out until it's time to drop Java 7 support?

In any case, I don't think this CL makes things any worse.

RELNOTES=Set Automatic-Module-Name for annotations package.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272890641
@cpovirk cpovirk mentioned this issue Oct 4, 2019
cpovirk added a commit that referenced this issue Oct 4, 2019
This makes it practical to use from code that uses modules.

Compare to Guava:
https://github.com/google/guava/blob/5496c37d4d904869297c2ced1f0d20e6f1507eaa/guava/pom.xml#L60
google/guava#2920

This is a step toward #1116 (though it doesn't address most artifacts, including javac).

Modules users may still see problems because com.google.errorprone.annotations is split between error_prone_annotations and error_prone_type_annotations. I wonder if the best solution to that is going to be to merge the two into error_prone_annotations, turning error_prone_type_annotations into an empty "forwarding" artifact. That would require compiling some files for Java 7 and some for Java 8, but that's doable (if a little clumsy). It might cause problems from some tool somewhere, though. Given that error_prone_type_annotations isn't nearly as commonly used, maybe you can wait it out until it's time to drop Java 7 support?

In any case, I don't think this CL makes things any worse.

RELNOTES=Set Automatic-Module-Name for annotations package.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272890641
@sgammon
Copy link
Contributor

sgammon commented Mar 11, 2024

@cushon I think this one can be closed now too.

@cushon cushon closed this as completed Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants