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.

Sign up for GitHub

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

Added OSGi metadata via build #6282

Closed
wants to merge 10 commits into from
+309 −26
Closed

Added OSGi metadata via build #6282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2b5c155
Added bnd plugin
swimmesberger Sep 27, 2020
8d2a979
Fixed comment
swimmesberger Sep 28, 2020
e02bd10
Added OSGi test, only apply bnd plugin to selected projects
swimmesberger Sep 29, 2020
c749a35
Renamed osgi test deployment configuration for better clarity
swimmesberger Sep 30, 2020
ef432d9
Moved workspace/bndrun definition into code instead of text files
swimmesberger Sep 30, 2020
3d3a006
Better comments, cleanup
swimmesberger Sep 30, 2020
6992eed
Removed cleared comment
swimmesberger Sep 30, 2020
3accc40
Preserving attr order
swimmesberger Sep 30, 2020
bb104b3
Added OSGi documentation to CONTRIBUTING.md
swimmesberger Sep 30, 2020
1d215e3
Added okhttpinternal package attribute to export package statement
swimmesberger Sep 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ Running Android Tests

$ ANDROID_SDK_ROOT=PATH_TO_ANDROID_HOME/sdk ./gradlew :android-test:connectedCheck

OSGi
---------------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding this


Ensure that added dependencies are OSGi compatible.
If added dependencies are optional the package imports in [okhttp/build.gradle][okhttp_build] need to
be adjusted.


Committer's Guides
------------------

Expand All @@ -59,3 +67,4 @@ Committer's Guides
[releasing]: http://square.github.io/okhttp/releasing/
[security]: http://square.github.io/okhttp/security/
[works_with_okhttp]: http://square.github.io/okhttp/works_with_okhttp/
[okhttp_build]: https://github.com/square/okhttp/blob/master/okhttp/build.gradle
33 changes: 29 additions & 4 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import net.ltgt.gradle.errorprone.CheckSeverity

buildscript {
ext.versions = [
'animalSniffer': '1.18',
Expand All @@ -20,7 +18,9 @@ buildscript {
'okio': '2.8.0',
'ktlint': '0.38.0',
'picocli': '4.2.0',
'openjsse': '1.1.0'
'openjsse': '1.1.0',
'bnd': '5.1.2',
'equinox': '3.16.0'
]

ext.deps = [
Expand All @@ -43,13 +43,18 @@ buildscript {
'moshi': "com.squareup.moshi:moshi:${versions.moshi}",
'moshiKotlin': "com.squareup.moshi:moshi-kotlin-codegen:${versions.moshi}",
'okio': "com.squareup.okio:okio:${versions.okio}",
'openjsse': "org.openjsse:openjsse:${versions.openjsse}"
'openjsse': "org.openjsse:openjsse:${versions.openjsse}",
'bnd': "biz.aQute.bnd:biz.aQute.bnd.gradle:${versions.bnd}",
'bndResolve': "biz.aQute.bnd:biz.aQute.resolve:${versions.bnd}",
'equinox': "org.eclipse.platform:org.eclipse.osgi:${versions.equinox}",
'kotlinStdlibOsgi': "org.jetbrains.kotlin:kotlin-osgi-bundle:${versions.kotlin}"
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 4.2M .jar, compared to 1.4M for kotlin-stdlib, presumably because this includes kotlin-reflect.

]

dependencies {
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.10"
classpath "org.jetbrains.dokka:dokka-gradle-plugin:0.10.1"
classpath "com.android.tools.build:gradle:4.0.1"
classpath deps.bnd
}

repositories {
Expand Down Expand Up @@ -107,6 +112,26 @@ allprojects {
}
}

ext.applyOsgi = { project ->
project.apply plugin: 'biz.aQute.bnd.builder'

project.sourceSets {
// own source set for OSGi specific dependencies
osgi
}

project.jar { t ->
// this sets only the source set for OSGi
// setSourceSet is a method provided by bnd
t.setClasspath(project.sourceSets.osgi['compileClasspath'] + project.sourceSets.main['compileClasspath'])
}

project.dependencies {
// when we provide the OSGi version for the kotlin standard library BND can infer the bundle version for the import package statements
Copy link
Collaborator

Choose a reason for hiding this comment

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

tricky. Presumably this doesn’t show up elsewhere. Whew.

osgiApi deps.kotlinStdlibOsgi
}
}

/** Configure building for Java+Kotlin projects. */
subprojects { project ->
if (project.name == 'android-test') return
Expand Down
11 changes: 8 additions & 3 deletions okhttp-brotli/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.brotli')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
bnd '''
Export-Package: okhttp3.brotli
Automatic-Module-Name: okhttp3.brotli
Bundle-SymbolicName: com.squareup.okhttp3.brotli
'''
}

dependencies {
Expand Down
11 changes: 8 additions & 3 deletions okhttp-dnsoverhttps/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.dnsoverhttps')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
bnd '''
Export-Package: okhttp3.dnsoverhttps
Automatic-Module-Name: okhttp3.dnsoverhttps
Bundle-SymbolicName: com.squareup.okhttp3.dnsoverhttps
'''
}

dependencies {
Expand Down
10 changes: 7 additions & 3 deletions okhttp-logging-interceptor/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
apply plugin: 'me.champeau.gradle.japicmp'
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.logging')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
bnd '''
Export-Package: okhttp3.logging
Automatic-Module-Name: okhttp3.logging
Bundle-SymbolicName: com.squareup.okhttp3.logging
'''
}

dependencies {
Expand Down
10 changes: 7 additions & 3 deletions okhttp-sse/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
apply plugin: 'me.champeau.gradle.japicmp'
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.sse')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
bnd '''
Export-Package: okhttp3.sse
Automatic-Module-Name: okhttp3.sse
Bundle-SymbolicName: com.squareup.okhttp3.sse
'''
}

dependencies {
Expand Down
10 changes: 7 additions & 3 deletions okhttp-tls/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
apply plugin: 'me.champeau.gradle.japicmp'
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.tls')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
bnd '''
Export-Package: okhttp3.tls
Automatic-Module-Name: okhttp3.tls
Bundle-SymbolicName: com.squareup.okhttp3.tls
'''
}

dependencies {
Expand Down
15 changes: 11 additions & 4 deletions okhttp-urlconnection/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
apply plugin: 'me.champeau.gradle.japicmp'
applyOsgi(this)

def mainProj = project(':okhttp')
jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3.urlconnection')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
// urlconnection is a OSGi fragment because the package name is the same as the okhttp3 module
bnd """
Fragment-Host: com.squareup.okhttp3; bundle-version="\${range;[==,+);\${version_cleanup;${mainProj.version}}}"
Automatic-Module-Name: okhttp3.urlconnection
Bundle-SymbolicName: com.squareup.okhttp3.urlconnection
-removeheaders: Private-Package
"""
}

dependencies {
api project(':okhttp')
api mainProj
compileOnly deps.jsr305
compileOnly deps.animalSniffer

Expand Down
40 changes: 37 additions & 3 deletions okhttp/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
apply plugin: 'me.champeau.gradle.japicmp'
applyOsgi(this)

jar {
manifest {
attributes('Automatic-Module-Name': 'okhttp3')
}
// modify these lines for MANIFEST.MF properties or for specific bnd instructions
// internal package needs to be exported too because sibling libraries use internal packages
// to prevent requiring certain android/ssl packages at runtime we explicitly define those as optional here
bnd '''
Export-Package: \
okhttp3,\
okhttp3.internal.*;okhttpinternal=true;mandatory:=okhttpinternal
Import-Package: \
android.*;resolution:=optional,\
dalvik.system;resolution:=optional,\
org.conscrypt;resolution:=optional,\
org.bouncycastle.*;resolution:=optional,\
org.openjsse.*;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.

If we introduce new dependencies, is there a test that’ll fail to remind us to update this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If new dependencies are introduced you have to decide on a case by case basis if it's an optional dependency or not. If you forget to add the library in the list the worst case will be that a OSGi user of that library has to put that library on the classpath to ensure OSGi can resolve all imports. The resolution optional attribute basically means that those packages are not required at runtime.

Copy link

Choose a reason for hiding this comment

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

maybe a move the comment about optional (ex why not Import-Package: *) here?

Platform or similar might need an update to mention when adding optional library support, to not forget to update this and wherever the test cases are! It could also be useful to use those paths in the OSGi test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we add a simple "*" here all the android/ssl packages would be required on the classpath for the bundle to resolve therefore I explicitly define the packages which are not required in every case at runtime.

Hmm... maybe we should add the comment to the build.gradle dependency block instead of the platform class because the dependency block must be touched to add new dependencies. I kind of dislike adding comments to code which are build related.

Copy link

Choose a reason for hiding this comment

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

I suppose I meant whatever the package-info equiv of kotlin. I suspect any new member of this directory would possibly taint here. https://github.com/square/okhttp/tree/master/okhttp/src/main/kotlin/okhttp3/internal/platform

Not too fussed where the comment goes, it can be in the CONTRIBUTING file.

thx for the clarification on not '*' that part could be a comment inline here in the gradle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added documentation to the CONTRIBUTING file

sun.security.ssl;resolution:=optional,\
*
Automatic-Module-Name: okhttp3
Bundle-SymbolicName: com.squareup.okhttp3
'''
}

sourceSets {
Expand All @@ -21,6 +37,18 @@ task copyJavaTemplates(type: Copy) {
filteringCharset = 'UTF-8'
}

// make osgiTestDeploy configuration jars available to the test environment
configurations {
osgiTestDeploy
}
task copyOsgiTestDeployment(type: Copy) {
from configurations.osgiTestDeploy
into "${buildDir}/resources/test/okhttp3/osgi/deployments"
}
tasks.test.dependsOn(copyOsgiTestDeployment)

// ensure that if a dependency is added here which is not necessarily required at runtime (compileOnly!)
// is also added as a optional package import to the bnd definition above
dependencies {
api deps.okio
api deps.kotlinStdlib
Expand All @@ -37,10 +65,16 @@ dependencies {
testImplementation project(':okhttp-urlconnection')
testImplementation project(':mockwebserver')
testImplementation project(':okhttp-logging-interceptor')
testImplementation project(':okhttp-brotli')
testImplementation project(':okhttp-dnsoverhttps')
testImplementation project(':okhttp-sse')
testImplementation deps.conscrypt
testImplementation deps.junit
testImplementation deps.assertj
testImplementation deps.openjsse
testImplementation deps.bndResolve
osgiTestDeploy deps.equinox
osgiTestDeploy deps.kotlinStdlibOsgi
testCompileOnly deps.jsr305
}

Expand Down
Loading