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 to most (not all) submodules #155

Merged
merged 10 commits into from
Apr 6, 2022
Merged

Conversation

dmlloyd
Copy link
Collaborator

@dmlloyd dmlloyd commented Apr 5, 2022

This adds module-info to every submodule which falls under one of these categories:

  • has no dependencies
  • has dependencies on other artifacts from this project which have module-info
  • has dependencies on artifacts with established module identifiers

The module-info descriptor is generated by my module-info Maven plugin. This is because the operation of that plugin is very simple and predictable (to me), and it produces a module-info.class file directly without trying to pass an intermediate module-info.java through javac, which doesn't work well. Another reason is that the generated module-info.class has a complete package list and version, which javac does not add (normally the jar tool handles this but Maven doesn't use it).

There are a few modules which depend on JBoss Logging or another dependency which does not yet have a module-info descriptor:

  • expression
  • net
  • version
  • vertx-context

One temporary solution for the first three could be to throw on a dependency for org.jboss.logging and let it break until we get it sorted out in that project with a release.

Another note: the surefire plugin has to be updated otherwise Java 8 testing breaks terribly with module-info. If #153 is merged, then surefire doesn't need to be upgraded since Java 8 testing would be removed.

An exception is the annotation submodule, which is the instigating case for this PR; the dependency used for javax.inject.Qualifier does not contain a module-info nor does it contain an automatic module name. Since it's an annotation, it seems like we could get away without the dependency. We may want to introduce a requires static for the official module coordinates of that artifact, if there are any.

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Apr 5, 2022

I'm looking into the Windows failure now. This is fixed.

@radcortez
Copy link
Member

@jponge will this work for you?

@jponge
Copy link
Member

jponge commented Apr 6, 2022

Let me check 😃

@jponge
Copy link
Member

jponge commented Apr 6, 2022

Just tested locally, I can resolve io.smallrye.common.annotation fine 👍

Thanks @dmlloyd @radcortez

@cescoffier cescoffier merged commit a173d67 into smallrye:main Apr 6, 2022
@dmlloyd dmlloyd deleted the mi branch April 6, 2022 13:42
@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Apr 6, 2022

See also jboss-logging/jboss-logging#45

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 this pull request may close these issues.

None yet

4 participants