Skip to content

Conversation

stIncMale
Copy link
Member

No description provided.

@stIncMale stIncMale requested a review from vbabanin September 23, 2024 19:53
@stIncMale stIncMale self-assigned this Sep 23, 2024
Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments for other reviewers.

'com.github.luben.zstd.*;resolution:=optional',
'org.slf4j.*;resolution:=optional',
'jnr.unixsocket.*;resolution:=optional',
'com.mongodb.crypt.capi.*;resolution:=optional',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

driver-core doesn't reference MongoCryptException (the only class left in this package), so this needed to be removed or else we get a warning.

jar.manifest.attributes['Bundle-SymbolicName'] = 'org.mongodb.driver-reactivestreams'
jar.manifest.attributes['Import-Package'] = [
'com.mongodb.crypt.capi.*;resolution:=optional',
'com.mongodb.internal.crypt.capi.*;resolution:=optional',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reactive-streams (and sync) reference MongoCryptException, and also the classes that were moved into the internal package, so both packages are now needed.

manifest {
attributes(
"-exportcontents" to "com.mongodb.crypt.capi.*;-noimport:=true",
"-exportcontents" to "com.mongodb.*;-noimport:=true",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a weird one. driver-core uses * for this, but * causes a warning with mongodb-crypt because the jar file contains directories for each of the shared library files, and some of those (e.g. linux-aarch64) are not valid Java package names. Using com.mongodb.* does the trick (it could also just be com.*).

Copy link
Member

@vbabanin vbabanin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@stIncMale stIncMale merged commit 49f7eb4 into mongodb:master Sep 23, 2024
55 of 59 checks passed
@stIncMale stIncMale deleted the fix-OSGi-crypt-manifest-entries branch September 23, 2024 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants