Add automatic module name - #1269
Open
raphw wants to merge 3 commits into
Open
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.
envelope.proto sets 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. That makes the Java package a local choice: add the java_package option upstream omits, following the convention the sibling protos already use, where package dev.sigstore.X maps to java_package dev.sigstore.proto.X. The option has to live in the proto, and the proto arrives inside an artifact, so carry a copy in src/main/proto next to fulcio.proto. It takes precedence on the --proto_path, and protoc refuses an input it has shadowed, so the extracted copy is dropped from the source set while staying on the include path. 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.
Member
|
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? |
Author
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.