Add automatic module name - #1269
Conversation
Declare a stable JPMS automatic module name so consumers building modular applications get a predictable module name instead of one derived from the jar file name. The name matches the jar's root package, dev.sigstore. Only sigstore-java is given a name. The CLIs are shipped as shadowed uber jars, and the Gradle and Maven plugins are loaded by their build tool's own classloader, so none of them are ever resolved on a module path.
sigstore-java compiled its own copies of google/api/{annotations,field_behavior,
http}.proto from src/main/proto, emitting com.google.api classes into the jar.
Those classes are byte-for-byte duplicates of ones in proto-google-common-protos,
which is already on sigstore-java's runtime classpath via grpc-protobuf, so the
copies never provided isolation: the fully-qualified names are identical and
which one wins is a matter of classpath ordering.
On the module path it is a hard failure rather than a silent one:
java.lang.module.ResolutionException: Module proto.google.common.protos
contains package com.google.api, module dev.sigstore exports package
com.google.api to proto.google.common.protos
Drop the vendored copies and let protoc resolve those imports from the include
path, which proto-google-common-protos already populates. The dependency is now
declared explicitly because BundleVerifier and the generated code link against
com.google.api directly; it was previously only an undeclared transitive.
The README justifying the copies said proto-google-common-protos had gone stale.
That is no longer so: it is at 2.74.0, released two weeks ago, and its copies of
the three protos are identical to the vendored ones apart from a C++-only
cc_enable_arenas option that does not affect Java codegen.
Resolves to 2.74.0, above the 2.64.1 grpc-protobuf requests. Its protobuf-java
4.33.6 stays below the BOM-pinned 4.35.1, so the BOM continues to win.
|
I think we should just update the intoto envelope proto in sigstore/protobuf-specs. I'll put that in and we can update this PR after: sigstore/protobuf-specs#923. I wish I used the module system more, but if we were to split sigstore-java into multiple modules (potentially planned), how would the naming strategy change? |
|
You'd keep the "main module" with its current name and likely suffix the other modules with a dot. Have a look at jenesis.build if you are curious about easier access. My current pet project. |
|
hey, I think you can continue with this for now, and update the dependency on protobuf-specs to |
b382199 to
76f522f
Compare
envelope.proto set go_package and ruby_package but no java_package, so protoc emitted Envelope and Signature into io.intoto, a namespace sigstore does not own, and sigstore-java shipped those nine classes in its own jar. Since 0.3.2 protobuf-specs publishes protos only, with no compiled classes, so these are generated here rather than consumed from upstream, which left the Java package to be chosen downstream. protobuf-specs 0.5.2 adds the missing option upstream, following the convention the sibling protos already use, where package dev.sigstore.X maps to java_package dev.sigstore.proto.X. Bump to it and pick the package up from there rather than shadowing the proto locally. Only Java codegen moves. The proto package stays io.intoto, so the descriptor is still io.intoto.Envelope, Bundle.dsse_envelope still points at it, and the wire and JSON encodings are byte for byte what they were. Signed bundles remain valid and interoperable. This is a breaking change for Java callers: Bundle.getDsseEnvelope() and friends now return dev.sigstore.proto.dsse.EnvelopeOuterClass.Envelope. The jar now contains only packages sigstore owns, so dev.sigstore no longer exports a foreign package to everyone who requires it. The bump also carries a new sigstore_monitor.proto, which generates into dev.sigstore.proto.monitor.v1, along with an ML_DSA_44 entry and deprecation markers on SHA3_256 and SHA3_384 in the common protos. Nothing in sigstore-java references any of them yet.
76f522f to
51710fc
Compare
|
My bad I wasn't really thinking. Lemme do that protobuf-specs update in a separate pr so we don't pollute this one |
|
okay sorry, so I got that done, we can just move this to automatic module name |
|
and what are the consequences of changing the module names later? |
|
I would not recommend changing the name. It would lead to any module compiled against your module to fail. During compiletime or runtime. It's a name you should choose once and stick to it. Similarly to a groupId and artifactId. It's going to be the durable representation of your artifact on the module path. |
|
okay, I think this sounds good then. sigstore-java may break out into multiple modules, but a sigstore-module umbrella module will always exist. Are you in a time crunch for this? can it wait a few weeks while we flesh out how we plan to split this up? |
|
Yes, that is the recommended refactoring. Have one empty module that imports submodules. Starting at root from the reverse DNS name of the library's domain. |
|
I do think it would be weird to have this be the dev.sigstore module though. This is just a client. We could have other dev.sigstore libraries coming from other parts of the ecosystem. |
|
That's fair. I would still avoid using Java in the name, as this is a Java-scoped coordinate already. But possibly append client? |
|
that would require us to set all packages to dev.sigstore.client.xyz? |
|
No, you can have packages whereever, but it is a convention to align root package and module name. What you cannot have us that packages in one module also exist in another module. Packages are reserved by each module on startup and overlaps crash the VM. |
|
okay cool. Do you mind if I come back to this next week. I want to see how we can keep things aligned. Its starting to sound like |
|
Certainly. Better to get this right than to rename later! |
|
|
|
okay lets go with |
3 participants
Closes #1268.
Adds
Automatic-Module-Name: dev.sigstore. As in the issue, only sigstore-java gets one.The other two commits are what make that name usable, both are about compatibility on the module path.
sigstore-java compiled its own copies of
google/api/{annotations,field_behavior,http}.protoand shipped the resultingcom.google.apiclasses in the jar. Those are the same class names thatproto-google-common-protospublishes, and that artifact is already on the runtime class path via grpc-protobuf. So the copies never really solved anything: the fully qualified names are identical either way, and which one wins comes down to class path order. On the module path it is not silent any more:The README kept the copies because
proto-google-common-protoshad gone stale. It is at 2.74.0 now, and its versions of those three files are identical to ours apart from acc_enable_arenasoption that only affects C++. So I dropped them and declared the dependency explicitly, sinceBundleVerifierlinks againstcom.google.apidirectly and only got it transitively before.Same story for the DSSE envelope.
envelope.protosetsgo_packageandruby_packagebut nojava_package, so protoc putEnvelopeandSignatureinio.intotoand shipped them. protobuf-specs has published protos only since 0.3.2, so these are generated here regardless and the Java package is ours to pick. I set it todev.sigstore.proto.dsse, matching what the sibling protos already do.That last one is a breaking change for Java callers,
Bundle.getDsseEnvelope()and friends change type. Only the Java package moves though. The proto package staysio.intoto, so the descriptor is stillio.intoto.Envelope, and the wire and JSON encodings are unchanged. Existing bundles stay valid.Happy to split the envelope commit out if you would rather take that separately.