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 JPMS module name or descriptor #53

Closed
Marcono1234 opened this issue Sep 1, 2024 · 6 comments · Fixed by #70
Closed

Add JPMS module name or descriptor #53

Marcono1234 opened this issue Sep 1, 2024 · 6 comments · Fixed by #70

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Sep 1, 2024

What happened?

For the Java Platform Module System, square/javapoet defines an Automatic-Module-Name (see pom.xml config).

It looks like this fork here neither defines an Automatic-Module-Name, nor does it have a module-info.class. This makes it difficult to use in a modular project.

What did you want to happen?

This project should either define an Automatic-Module-Name in the manifest, or contain a module-info.class (respectively META-INF/versions/9/module-info.class and in manifest Multi-Release: true).

@skku-daniilkim
Copy link
Contributor

I think the project should at least just follow the way the original repo did, and simply define the "Automatic-Module-Name" in the pom/gradle:

ext.moduleName = "com.palantir.javapoet"

jar {
    inputs.property("moduleName", moduleName)

    manifest {
       attributes  'Automatic-Module-Name': moduleName
   }
}

@skku-daniilkim
Copy link
Contributor

I would like to provide the PR, but I am not sure, which "build.gradle" should be used. Is it the one inside the "javapoet" folder or the root one?

@carterkozak
Copy link
Contributor

I believe it would go into this one: https://github.com/palantir/javapoet/blob/develop/javapoet/build.gradle

skku-daniilkim added a commit to skku-daniilkim/javapoet that referenced this issue Sep 16, 2024
@skku-daniilkim
Copy link
Contributor

skku-daniilkim commented Sep 16, 2024

Submitted a PR. It now adds the entry into the manifest properly:
image
Although I am not sure whether this will work, when built as an artifact for maven. Due to the unfamiliar build system ("gradlew build" builds two jar files: empty root jar file and the "javapoet" jar file), I only tested by building locally and linking to my other gradle project, and it seems to work with the actual "javapoet" jar file, and considering that the root jar file is literally empty, I'm pretty sure that the "build.gradle" that you mentioned is the correct one, so it should be fine.

bulldozer-bot bot pushed a commit that referenced this issue Sep 16, 2024
@carterkozak
Copy link
Contributor

Thanks for the contribution! I expect it to work, but you should be able to merge within ~10 minutes or so when the artifact hits maven central :-)

@skku-daniilkim
Copy link
Contributor

0.4.0 just hit the central; requires com.palantir.javapoet; in module-info now works properly, and the imports work as intended with JPMS projects without any warnings.

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

Successfully merging a pull request may close this issue.

3 participants