vert-x3 / issues

Apache License 2.0
37 stars 7 forks source link

Provide Automatic-Module-Name attribute in MANIFEST.MF #524

Closed afloarea closed 1 year ago

afloarea commented 4 years ago

Feature Description

Provide Automatic-Module-Name attribute in MANIFEST.MF file.

Use cases

Provides a reliable module name, instead of using the name derived from the JAR file, without having to target Java 9+ and/or use module-info java/class file. This can be used until there is a module-info file.

Contribution

If this feature request were to be approved I would be happy to work on it.

I believe that this change would require some additional configuration in the pom.xml maven jar plugin, for example:


<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-jar-plugin</artifactId>
    <configuration>
        <archive>
            <manifestEntries>
                <Automatic-Module-Name>io.vertx.core</Automatic-Module-Name>
            </manifestEntries>
        </archive>
    </configuration>
</plugin>

Naming Scheme

When choosing module names, these rules should be taken into consideration.

Table of automatic module names and their status:

Notes:

Core

Component Artifact id Automatic Module Name Status
Vertx core vertx-core io.vertx.core Merged

Web

Component Artifact id Automatic Module Name Status
Vertx web vertx-web io.vertx.web Merged
Vertx web client vertx-web-client io.vertx.web.client Merged
Vertx web common vertx-web-common io.vertx.web.common Merged
Vertx web OpenAPI vertx-web-openapi io.vertx.web.openapi Merged
Vertx web validation vertx-web-validation io.vertx.web.validation Merged
Vertx web templates vertx-web-templ-[x] io.vertx.web.template.[x] Merged
Vertx web API vertx-web-api-contract io.vertx.web.apicontract Merged
Vertx web API service vertx-web-api-service io.vertx.web.apiservice Merged
Web GraphQL Handler vertx-web-graphql io.vertx.web.graphql Merged
Web sstores vertx-web-sstore-[x] io.vertx.web.sstore.[x] Merged
Web Proxy vertx-web-proxy io.vertx.web.proxy Todo
Web OpenAPI Rtouer vertx-web-openapi-router io.vertx.web.openapi.router Todo

Data access

Component Artifact id Automatic Module Name Status
MongoDB client vertx-mongo-client io.vertx.client.mongo Merged
Redis client vertx-redis-client io.vertx.client.redis Merged
Cassandra client vertx-cassandra-client io.vertx.client.cassandra Merged
Common SQL interfaces vertx-sql-common N/A Merged
JDBC client vertx-jdbc-client io.vertx.client.jdbc Merged

Reactive SQL clients

Component Artifact id Automatic Module Name Status
SQL client vertx-sql-client io.vertx.client.sql Merged
DB2 client vertx-db2-client io.vertx.client.sql.db2 Merged
MSSQL client vertx-mssql-client io.vertx.client.sql.mssql Merged
MySQL client vertx-mysql-client io.vertx.client.sql.mysql Merged
PostgreSQL client vertx-pg-client io.vertx.client.sql.pg Merged
Templates vertx-client-sql-templates io.vertx.client.sql.templates Merged

Reactive

Component Artifact id Automatic Module Name Status
Reactive Streams vertx-reactive-streams io.vertx.reactivestreams Merged
Vertx Sync vertx-sync N/A -
Kotlin coroutines vertx-lang-kotlin-coroutines io.vertx.kotlin.coroutines Merged

Reactive Extensions

Component Artifact id Automatic Module Name Status
Rx code generator vertx-rx-gen io.vertx.rx.gen Postponed
RxJava code generator vertx-rx-java-gen io.vertx.rx.java.gen Postponed
RxJava API vertx-rx-java io.vertx.rx.java Postponed
RxJava2 Code Generator vertx-rx-java2-gen io.vertx.rx.java2.gen Postponed
RxJava2 API vertx-rx-java2 io.vertx.rx.java2 Postponed
JUnit 5 RxJava support vertx-junit5-rx-java io.vertx.junit5.rx.java Postponed
JUnit 5 RxJava2 support vertx-junit5-rx-java2 io.vertx.junit5.rx.java2 Postponed

Microservices

Component Artifact id Automatic Module Name Status
Circuit Breaker vertx-circuit-breaker io.vertx.circuitbreaker Merged

Vertx Config

Component Artifact id Automatic Module Name Status
Vertx Config vertx-config io.vertx.config Merged
Consul vertx-config-consul io.vertx.config.consul Merged
Git vertx-config-git io.vertx.config.git Merged
HOCON vertx-config-hocon io.vertx.config.hocon Merged
Kubernetes ConfigMap vertx-config-kubernetes-configmap io.vertx.config.configmap Merged
Redis vertx-config-redis io.vertx.config.redis Merged
Spring config server vertx-config-spring-config-server io.vertx.config.springconfigserver Merged
Vault vertx-config-vault io.vertx.config.vault Merged
YAML vertx-config-yaml io.vertx.config.yaml Merged
Zookeeper vertx-config-zookeeper io.vertx.config.zookeeper Merged

Vertx Service Discovery

Component Artifact id Automatic Module Name Status
Service Discovery vertx-service-discovery io.vertx.servicediscovery Merged
Consul Backend vertx-service-discovery-backend-consul io.vertx.servicediscovery.backend.consul Merged
Redis Backend vertx-service-discovery-backend-redis io.vertx.servicediscovery.backend.redis Merged
Zookeeper Backend vertx-service-discovery-backend-zookeeper io.vertx.servicediscovery.backend.zookeeper Merged
Consul Bridge vertx-service-discovery-bridge-consul io.vertx.servicediscovery.bridge.consul Merged
Docker links Bridge vertx-service-discovery-bridge-docker-links io.vertx.servicediscovery.bridge.dockerlinks Merged
Docker Bridge vertx-service-discovery-bridge-docker io.vertx.servicediscovery.bridge.docker Merged
Kubernetes Bridge vertx-service-discovery-bridge-kubernetes io.vertx.servicediscovery.bridge.kubernetes Merged
Zookeeper Bridge vertx-service-discovery-bridge-zookeeper io.vertx.servicediscovery.bridge.zookeeper Merged

Standards

Component Artifact id Automatic Module Name Status
Vertx OpenAPI vertx-openapi io.vertx.openapi Done
Vertx Json Schema vertx-json-schema io.vertx.jsonschema Todo
Vertx URI Template vertx-uri-temlate io.vertx.uritemplate Todo

HTTTP Proxy

Component Artifact id Automatic Module Name Status
Vertx Http Proxy vertx-http-proxy io.vertx.httpproxy Todo

MQTT

Component Artifact id Automatic Module Name Status
Vertx MQTT vertx-mqtt io.vertx.mqtt Merged

Authentication and Authorisation

Component Artifact id Automatic Module Name Status
Auth Common vertx-auth-common io.vertx.auth.common Merged
JDBC Auth vertx-auth-jdbc io.vertx.auth.jdbc Merged
JWT Auth vertx-auth-jwt io.vertx.auth.jwt Merged
JWT vertx-jwt N/A Artifact code merged into vertx-auth-common
LDAP vertx-auth-ldap io.vertx.auth.ldap Merged
Shiro Auth vertx-auth-shiro io.vertx.auth.shiro Merged
MongoDB Auth vertx-auth-mongo io.vertx.auth.mongo Merged
OAuth 2 vertx-auth-oauth2 Io.vertx.auth.oauth2 Merged
Properties vertx-auth-properties Io.vertx.auth.properties Merged
.htdigest Auth vertx-auth-htdigest io.vertx.auth.htdigest Merged
.htpasswd vertx-auth-htpasswd io.vertx.auth.htpasswd Merged
SQL Auth vertx-auth-sql io.vertx.auth.sql Merged
WebAuthN vertx-auth-webauthn io.vertx.auth.webauthn Merged

Messaging

Component Artifact id Automatic Module Name Status
AMQP Client vertx-amqp-client io.vertx.client.amqp Merged
STOMP vertx-stomp io.vertx.stomp Merged
RabbitMQ Client vertx-rabbitmq-client io.vertx.client.rabbitmq Merged
Vert.x Proton vertx-proton io.vertx.proton Merged

Integration

Component Artifact id Automatic Module Name Status
Kafka Client vertx-kafka-client io.vertx.client.kafka Merged
Mail Client vertx-mail-client io.vertx.client.mail Merged
Consul Client vertx-consul-client io.vertx.client.consul Merged

JCA Adaptor

Component Artifact id Automatic Module Name Status
JCA Adapter vertx-jca-adapter N/A -
JCA API vertx-jca-api N/A -

Event Bus Bridge

Component Artifact id Automatic Module Name Status
Common vertx-bridge-common io.vertx.eventbusbridge.common Merged
TCP Event Bus Bridge vertx-tcp-eventbus-bridge io.vertx.eventbusbridge.tcp Under discussion
Camel Bridge vertx-camel-bridge io.vertx.eventbusbridge.camel Under discussion

Devops

Component Artifact id Automatic Module Name Status
Dropwizard metrics vertx-dropwizard-metrics io.vertx.metrics.dropwizard Merged
Micrometer vertx-micrometer-metrics io.vertx.metrics.micrometer Merged
Health Check vertx-health-check io.vertx.healthcheck Merged
Shell vertx-shell io.vertx.shell Merged
Docker vertx-stack-docker N/A -
Stack Manager vertx-stack-manager N/A -

Tracing

Component Artifact id Automatic Module Name Status
Zipkin vertx-zipkin io.vertx.tracing.zipkin Merged
Opentracing vertx-opentracing io.vertx.tracing.opentracing Merged

Testing

Component Artifact id Automatic Module Name Status
Vertx Unit vertx-unit io.vertx.testing.unit Merged
JUnit 5 vertx-junit5 io.vertx.testing.junit5 Merged

Clustering

Component Artifact id Automatic Module Name Status
Hazelcast vertx-hazelcast io.vertx.clustermanager.hazelcast Merged
Infinispan vertx-infinispan io.vertx.clustermanager.infinispan Merged
Apache Ignite vertx-ignite io.vertx.clustermanager.ignite Merged
Zookeeper vertx-zookeeper io.vertx.clustermanager.zookeeper Merged

Services

Component Artifact id Automatic Module Name Status
Service Proxies vertx-service-proxy io.vertx.serviceproxy Merged
Service Factories vertx-service-factory N/A -
HTTP Service Factory vertx-http-service-factory N/A -
SockJs Service Proxy vertx-sockjs-service-proxy io.vertx.serviceproxy.sockjs Merged
Maven Service Factory vertx-maven-service-factory N/A -

gRPC

Component Artifact id Automatic Module Name Status
gRPC vertx-grpc io.vertx.grpc Merged
gRPC vertx-grpc-client io.vertx.grpc.client Done
gRPC vertx-grpc-server io.vertx.grpc.server Done
gRPC vertx-grpc-common io.vertx.grpc.common Done
gRPC vertx-grpc-context-storage N/A -
gRPC protoc plugin vertx-grpc-protoc-plugin N/A -

Internal Magic

Component Artifact id Automatic Module Name Status
Codegen vertx-codegen io.vertx.codegen Merged
Docgen vertx-docgen N/A -
Codetrans vertx-codetrans N/A -
Bridge Common vertx-bridge-common io.vertx.eventbusbridge.common Merged
Distro Stack Manager vertx-stack-manager io.vertx.stack.manager N/A

Languages

Component Artifact id Automatic Module Name Status
Groovy lang vertx-lang-groovy N/A N/A

Kotlin

Component Artifact id Automatic Module Name Status
Kotlin gen vertx-lang-kotlin-gen io.vertx.kotlin.gen Under discussion
Kotling lang vertx-lang-kotlin io.vertx.kotlin Merged

Scala

Component Artifact id Automatic Module Name Status
Scala lang codegen vertx-lang-scala-codegen N/A -
Scala language support vertx-lang-scala_2.13 N/A -
vietj commented 4 years ago

I believe we cannot use package names because some package have the ext name and some don't, which is inconsistent. So I would like module to be rather named after the related Maven artifact, likely prefixed with io.vertx instead of vertx-, e.g

That being said, are we allowed to have a module named io.vert.web and another one named io.vertx.web.client ?

There is in the rules defines by https://sormuras.github.io/blog/2018-11-16-invalid-automatic-module-names#how-to-name-a-module a rule I do not understand: Creating a module with a particular name takes ownership of that package name and everything beneath it.:

afloarea commented 4 years ago

I believe that there is no issue with having automatic modules/submodules, such as io.vertx.web and io.vertx.web.client as long as they do not have any split packages between them.

A thing to note: Say you have 2 artifacts/jars/libraries A and B which have a split package P (they both have classes in package P).

If they do not have a module name (via MANIFEST property) and are placed on the module path, then they will get mashed up into the unnamed module. The client JPMS application will run.

If however, they had module names (via MANIFEST), they would each become a (automatic) module. The client JPMS application will not run in this case because of the split package. This outcome would be the same if module A would be named but module B would not have a name, since they will end up in different modules again.

Client code can require/use both io.vertx.web and io.vertx.web.client as long as the 2 jars/artifacts/modules do not have any split packages.

In regards to the ownership of the package name and everything beneath it - What I understand from this is that if you choose the package name io.vertx then you are responsible for creating unique modules under that name (io.vertx.x.y.z) that do not have split packages.

There is no strict relationship between the module name and the package name. It is only a recommandation that facilitates module name uniqueness.

Considering your example, you can indeed create a module foo.bar that contains a com.whatever package. Since you own whatever.com, no one else would create classes under that package , but they could use the module name foo.bar, say with com.something.else package. A client application would not be able however to use both modules at the same time, even thought there are totally different packages in the 2 modules and that is because the modules have the same name.

To summarize, I believe that the requirement is to have unique modules names under io.vertx that do not have split packages. Maybe more useful/detailed information could be found here: https://blog.joda.org/2017/04/java-se-9-jpms-module-naming.html

So yes, the artifact name prefixed with io.vertx is a viable option, but the problem of split packages must be considered.

vietj commented 4 years ago

So the rule Creating a module with a particular name takes ownership of that package name and everything beneath it. is not a JPMS rule to satisfy as I understand it ?

afloarea commented 4 years ago

No, I believe it is not. You are right.

vietj commented 4 years ago

ok, so let's get first a proposal for providing automatic module names to vertx jars (when it makes sense) and then we can see how this can be achieved in the stack which will require to update the vertx-parent pom

afloarea commented 4 years ago

I am not sure I understand. I think your suggestion is good, that is, the module name would be io.vertx.${processed-artifact-name}, where the processed artifact name would be the artifact name without the vertx- prefix and with - converted to . (i.e. the vertx-jdbc-client-<version> jar would have the module name io.vertx.jdbc.client)

vietj commented 4 years ago

by proposal I mean list all module names according to this convention and then see discuss this in community to see if there are issues with that

vietj commented 4 years ago

I think there are a few names we shall override, e.g

etc...

afloarea commented 4 years ago

I have updated the reactive streams entry.

Which of the 2 sql client options would be preferred?

In my opinion, it would be helpful to keep the metrics module names consistent. That is, io.vertx.micrometer & io.vertx.dropwizard or io.vertx.micrometer_metrics & io.vertx.dropwizard_metrics. Again, which of these would be preferred?

Also, what should be the work flow? Should work start when the entire list is green lit, or start work on approved modules names so that work on the approved module names and the approval of other module names happen in parallel?

I am just not sure of the next steps. Can you please provide some insight information?

vietj commented 4 years ago

I think we can do both in parallel.

For the packaging, can we study the modification of the vertx-parent to have the automatic module name applied optionally ? i.e if we define a property in the pom.xml then there is an automatic module otherwise it is left outside (I believe there are some module that won't benefit from automatic module name such as vertx-lang-groovy).

vietj commented 4 years ago

Perhaps for keeping track of this we should use the wiki, is that possible to get PR on a GitHub wiki ?

afloarea commented 4 years ago

Thank you for the prompt response @vietj . I will look into the change to the vertx-parent and raise a PR on the GitHub wiki and get back to you.

vietj commented 4 years ago

please do an RFC like page in wiki

afloarea commented 4 years ago

I have created the RFC page, but I cannot push it to a remote branch in order to create PR because I do not have permission. I assumed that this is the wiki.

I have also looked into changing the parent pom so that when a property in the child pom is defined, then the module name is applied. Unfortunately, I could not find a way for this to work, since maven profiles cannot be activated by a property defined in a pom.xml, only via system property, file, etc, so the closest thing would be mvn package -Dautomatic.module=io.vertx.core.

In my opinion, it would be easier / more straightforward to just add the <automatic.module>io.vertx.whatever</automatic.module> as a maven property in the child pom and also add the jar plugin configuration in the child pom for the default-jar execution and not modify the parent pom at all.

vietj commented 4 years ago

@afloarea where are your changes ?

afloarea commented 4 years ago

I have cloned the wiki and created a local branch on my machine but I cannot push the branch because I do not have permission to push to the wiki repo. Is there a different way to contribute that I am not aware of?

vietj commented 4 years ago

let me investigate how the wiki thing work :-)

On 24 Apr 2020, at 19:45, Adrian Floarea notifications@github.com wrote:

I have cloned the wiki and created a local branch on my machine but I cannot push the branch because I do not have permission to push to the wiki repo. Is there a different way to contribute that I am not aware of?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/issues/issues/524#issuecomment-619156009, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCTW6HXIPTBG3M7WZB3ROHF5FANCNFSM4MF7SADQ.

vietj commented 4 years ago

what if a problem should not have an automatic name ? e.g we have some jars which will never be modules.

vietj commented 4 years ago

@afloarea unfortunately the wiki access management of GitHub is quite weak, so the best I think is that you edit this issue content instead of commenting and we continue using this issue to discuss

afloarea commented 4 years ago

what if a problem should not have an automatic name ? e.g we have some jars which will never be modules.

I agree, not every jar file needs an automatic module name. My proposal is that based on the component/project table, a name is approved and marked as approved in the table. After that, a PR is made for only that component that will edit the pom.xml file of that project/component only. The PR would simply add an <automatic.module.name> property in the <properties> section of the pom.xml and add the additional jar-plugin configuration of the manifest and use that property. The PR would only affect the child pom.xml of the component, so only one component.

So, for example, say you approve the io.vertx.core name, then a PR would be made to the child pom.xml of the vertx-core component which will add the automatic module name. Upon merge, only the vertx-core jar would have an automatic module name.

If, for example, it is decided that the vertx-lang-groovy jar should remain without an automatic module name, then no PR is raised for that component's pom.xml file, and the table is simply updated with 'not applicable' or something similar and that's that. It will remain as is (i.e. without a name)

What do you think?

vietj commented 4 years ago

+1 Adrian

vietj commented 4 years ago

can you edit this topic and move here the table and add checks ?

afloarea commented 4 years ago

Of course. You are referring to the table from above, right? When you say 'add checks' do you want me to add checkboxes or do you want me to mark some entries as checked / confirmed name.

I was thinking that you could confirm some of the names and I would update the table and with your approval I would start working on a PR for the confirmed names and possibly provide a link to the PR in the table.

vietj commented 4 years ago

@afloarea I mean in the first message of the issue you created by check box I mean using this:

afloarea commented 4 years ago

@vietj I have moved the table to the top and I have added check boxes as you suggested.

afloarea commented 4 years ago

Hey @vietj . I have went ahead and raised a PR for adding automatic module name for vertx-core -> https://github.com/eclipse-vertx/vert.x/pull/3401

Could you please confirm other module names so that I may start working on those as well? Is it ok to carry on with the web category?

Also, would it be possible to shorten the contribution flow? I saw that for small changes it may be possible to contribute directly to master. It's a bit cumbersome to fork the project, create a branch with one commit with 2 additional lines and then raise a PR.

vietj commented 4 years ago

@afloarea for the name I think we need to do a meeting with the team and discuss that, e.g I feel that for clients we should maybe name to io.vertx.client.mongo rather than io.vertx.mongo.client

vietj commented 4 years ago

@afloarea I've setup an hangout meeting next Monday to agree on the name of modules, if you give me your email you can join the meeting

afloarea commented 4 years ago

@vietj Thank you for the invite. My email is adrianfloarea07@yahoo.com

vietj commented 4 years ago

you should receive an invite.

On 7 May 2020, at 17:36, Adrian Floarea notifications@github.com wrote:

@vietj https://github.com/vietj Thank you for the invite. My email is adrianfloarea07@yahoo.com mailto:adrianfloarea07@yahoo.com — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/issues/issues/524#issuecomment-625330093, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCUW4TEQ2PRR7OHGZ5LRQLIQLANCNFSM4MF7SADQ.

vietj commented 4 years ago

See https://github.com/vert-x3/vertx-auth/issues/388

vietj commented 4 years ago

See https://github.com/vert-x3/vertx-sql-common/issues/69

vietj commented 4 years ago

@afloarea vertx 4 projects have been updated with new parent pom

afloarea commented 4 years ago

Thanks @vietj . I have updated the PR for vertx-core. I will proceed in a similar fashion with the web artifacts.

vietj commented 4 years ago

@afloarea I updated the tables with more approvals

vietj commented 4 years ago

can you point out the pending PR to merge ?

afloarea commented 4 years ago

@vietj The PR should appear on this issue. They are just above your comment where you say that you updated the tables.

Currently vertx-mongo-client has been merged, the redis, cassandra and jdbc clients have an open PR and the web artifactis also have an open PR, but I guess that is on hold until the openAPI gets merged.

Also, I have seen that you approved more service discovery artifacts. I forgot to update the suggested automatic module name for those that you approved and they were left with sevice.discovery in the module name instead of servicediscovery (without the dot). I will change them to the latter (just FYI, let me know if you did actually want them with the dot and I will change them back)

I will also update the table with any open PRs.

vietj commented 4 years ago

@afloarea I don't see any pending PR linked in this document, am I missing something ?

afloarea commented 4 years ago

@vietj

image

I am also updating the table as we speak.

afloarea commented 4 years ago

@vietj I have updated the table

afloarea commented 4 years ago

@vietj I noticed you added Distro Stack Manager under 'Internal Magick'. Is this different than the stack manager under 'Devops'? Or did I made a mistake by putting it there?

vietj commented 4 years ago

I just meant that stack manager is something we don't really care about.

pmlopes commented 4 years ago

@afloarea I've just merged the vertx-jwt into vertx-auth-common as discussed in the issue. I think you are now able to work the vertx-auth modules without a problem!

vietj commented 4 years ago

@afloarea can we try getting the property in the parent pom for multi module projects when it is possible ?

afloarea commented 4 years ago

Thanks @pmlopes . I have raised a PR for auth.

@vietj Yes, I actually don't know why I didn't do that in the first place 😅. I have updated the auth and service discovery to use the parent pom. I will update the others as well

afloarea commented 4 years ago

@vietj @pmlopes I can't build the jars for vertx-web anymore, so I coudn't test with the parent pom. It fails to compile because it can't find io.vertx.ext.jwt.*. I also merged from upstream vertx-web but it still failed

pmlopes commented 4 years ago

Ah, that's my bad, there's a reference to a class from the merged modules in auth. I'll fix it...

pmlopes commented 4 years ago

web should be building correctly now...

afloarea commented 4 years ago

I tested web with parent pom and I updated the PR. Thank you @pmlopes :+1:

vietj commented 4 years ago

I believe I merged all the open PR @afloarea keep the PR flow coming in :-)