Added OSGi metadata via build #6282

swimmesberger · 2020-09-27T17:15:15Z

Added bnd plugin to the gradle build so that META-INF/MANIFEST.MF file contains OSGi metadata.
This is analogous to the implementation in okio: square/okio#777

I added some comments for those modules where it was difficult for me to know which packages should be exported or not. There is also the okhttp-urlconnection module which uses the same package name as the root okhttp project which BND/OSGi does not really like. In OSGi each module should export only a uniquely identifiable package. Therefore for this package I had to find a "creative" way around it. Maybe the better solution would be to publish a OSGi specific artifact for that module with a changed package name (e.g. okhttp3.urlconnection). I don't really know how much that module is used and why it has the same package name.

swankjesse · 2020-09-28T13:07:12Z

okhttp-tls/build.gradle

- attributes('Automatic-Module-Name': 'okhttp3.tls')
- }
+ // modify these lines for MANIFEST.MF properties or for specific bnd instructions
+ // are okhttp3.tls.internal.* packages used in dependent libraries?


Lets talk about this. Can OkHttp libraries use each other’s internal packages?

No. In OSGi there is no concept that allows "special" modules to access packages other modules can not. Therefore if any other module needs access to the okhttp3.tls.internal.* packages we need to export them. If we do not export them, OSGi will detect at resolution time that the package okhttp3.tls.internal is imported from module x but module y does not export it.
There is the Fragment specification which might allow that but I think it will introduce more trouble than it solves.

swankjesse · 2020-09-28T13:09:27Z

okhttp-urlconnection/build.gradle

- }
+ // modify these lines for MANIFEST.MF properties or for specific bnd instructions
+ // because this project uses the same package name as the default okhttp project (which is mostly "invalid" for OSGi)
+ // we have to remove okhttp from the bnd classpath and import it's packages manually


what does OSGi want us to do in this case? Should we just say this package doesn’t exist for OSGi users? It’s pretty negligible in terms of value.

There are 2 solutions to this:

Rename the package name (OSGi preferred way - breaks backwards compatibility)

Make okttp-urlconnection a fragment bundle which will at runtime basically mean that the root okhttp bundle and the okhttp-urlconnection bundle use the same ClassLoader. This means that externally when both bundles are present it's like "a single bundle". Fragment bundles are not really best practices and should generally be avoided if possible but it might work for this case.

Can we make all of OkHttp’s libraries a fragment bundle and get the internal visibility we crave?

Aside from architectural purity, what’s the drawback of fragment bundles?

Fragment bundles are kind of niche and behave differently based on the environment on some cases. PDE for example the OSGi build system from eclipse (the eclipse version of "bnd") will fail to honor packages added by a fragment module which is not defined on the host module. To make things work for PDE we would additionally need the "Eclipse-ExtensibleAPI: true" attribute in the manifest.

For the okhttp use-case fragment bundles might work to not expose the internal packages because every sibling module depends on the okhttp core library anyway. Do you know if any popular thirdparty library depend on one or more internal packages? (e.g. retrofit?) If that's the case we would need to export these packages anyway.

OkHttp’s internal stuff is OkHttp-only.

Another option – we can stop reaching across submodules to do internal stuff? That might be simpler.

... turns out that’s a nonstarter for MockWebServer.

Getting rid of the internal stuff would be indeed the best solution. For okhttp-urlconnection we would still need a fragment because of the package name issue.

Btw. MockWebServer - I have missed that module, I have to modify the build.gradle for that one.

Maybe a hybrid solution would be possible? For those modules still requiring internal apis we use the fragment approach and for the other modules we create a "normal" OSGi bundle.

swankjesse · 2020-09-28T13:09:55Z

okhttp/build.gradle

- 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 interceptors use internal packages


interceptors? Or sibling libraries?

Yeah I meant the sibling libraries.

swankjesse · 2020-09-28T13:10:09Z

okhttp/build.gradle

+ bnd '''
+ Export-Package: \
+ okhttp3,\
+ okhttp3.internal.*


Is there a concept of a friend package?

See okhttp-tls comment

in zipkin we do things like this..

export your internal package..
https://github.com/openzipkin/brave/blob/master/brave/bnd.bnd#L9

explicitly import when internal is used
https://github.com/openzipkin/brave/blob/master/context/slf4j/bnd.bnd#L1

block import when something is source retention..
https://github.com/openzipkin/brave/blob/master/instrumentation/http/bnd.bnd#L2

Just to be sure I understand the approach you set the "braveinternal" attribute mandatory* which enforces that the "braveinternal" attribute is set for every Import-Package statement? Therefore if someone wants to Import one of the internal packages the "braveinternal" attribute must explicitly set otherwise the bundle would not resolve?

So for my understanding this would be like a "I agree that what I'm importing is not public API and can change at will". I kind of like that approach because it prevents accidently importing a package which you shouldn't because it's internal.

* For Reference: https://docs.osgi.org/specification/osgi.core/7.0.0/framework.module.html#i2515263

I think you understand it, yes. We have had bugs where we forgot to explicitly import the internal thing, but I think it is a better optimization anyway.

I did hope that bnd does not specify mandatory attributes automatically but I have just tried it and bnd does in fact automatically infer mandatory attributes for Import-Package statements. Therefore if someone is using bnd as a build system he wouldn't even notice that there is an mandatory attribute called "okhttpinternal"/"braveinternal"...

trying to understand the impact of this.. third parties outside this repo shouldn't be using the internal code anyway, so should be not impacted by ignorance of them. Only here, we have to make sure the bnds are coherent.. am I missing something?

We want to ensure that only the okhttp sibling libraries are able to use the internal package of the main okhttp project. Therefore we add a mandatory "okhttpinternal" directive which ensures that for each Import-Package: okhttp3.internal statement the attribute must be explicitly defined. For us it's easy to define that attribute for the siblings okhttp3 libraries which use those internal packages. If a 3rdparty consumer would also import one of the internal packages without defining the attribute the bundle would resolve with an error according to OSGi spec because the attribute is not defined.
Unfortunately if the 3rdparty is also using bnd as a build system (like we in our internal project for example) we could freely use the okhttp3.internal package (even though we shouldn't) and we wouldn't event notice that we should define a mandatory attribute because bnd figures that out automatically.
As an example: I tried to add that attribute to the root okhttp3 project and I didn't have to specify the attribute in the sibling okhttp-sse library manually because bnd automatically inferred the mandatory attribute which kind of defeats the whole purpose of the attribute.

Manifest-Version: 1.0 Automatic-Module-Name: okhttp3.sse Bnd-LastModified: 1601454505191 Bundle-ManifestVersion: 2 Bundle-Name: com.squareup.okhttp3.sse Bundle-SymbolicName: com.squareup.okhttp3.sse Bundle-Version: 4.10.0.SNAPSHOT Created-By: 14.0.2 (AdoptOpenJDK) Export-Package: okhttp3.sse;uses:="kotlin,kotlin.jvm,okhttp3";version= "4.10.0" Import-Package: kotlin;version="[1.4,2)",kotlin.io;version="[1.4,2)",k otlin.jvm;version="[1.4,2)",kotlin.jvm.internal;version="[1.4,2)",okh ttp3;version="[4.10,5)",okhttp3.internal;version="[4.10,5)";okhttpint ernal=true,okhttp3.internal.connection;version="[4.10,5)";okhttpinter nal=true,okio Private-Package: okhttp3.internal.sse Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))" Tool: Bnd-5.1.2.202007211702

If this automatically sucks in the internal stuff, it would seem to be a behavioral difference in manifest generation here vs for example maven-bundle-plugin. This might be a different interpretation of what is sane behavior, or simply an accident in one or the other.

In any case, would you agree that the internal attribution is important to annotate even the gradle plugin or default configuration thwarts that guard rail?

cc also @helgoboss for input as it is sometimes hard for project teams to come to a sane approach for things like this if they aren't actively using the tooling.

I have added the attribute although when bnd is used the benefit is negligible. I did not have to explicitly define the import in the sibling libraries because bnd adds the imports with the property set to true automatically...

swankjesse · 2020-09-28T13:10:52Z

okhttp/build.gradle

+ dalvik.system;resolution:=optional,\
+ org.conscrypt;resolution:=optional,\
+ org.bouncycastle.*;resolution:=optional,\
+ org.openjsse.*;resolution:=optional,\


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

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.

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

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.

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.

Added documentation to the CONTRIBUTING file

swankjesse · 2020-09-28T16:25:23Z

Is there a way to run our JUnit tests with OSGi somehow, so our tests fail if we break OSGi’s rules? I’d hate to introduce an internal-style dependency and ship a broken OSGi release.

swimmesberger · 2020-09-28T17:07:35Z

I could add a bnd Launchpad test that creates a OSGi environment with every okhttp module and tries to resolve it (resolving is the OSGi phase where OSGi tries to find a Export-Package for every Import-Package). Most OSGi issues will show in the resolving phase. When resolving works the OSGi metadata should be fine.

swankjesse · 2020-09-28T17:50:15Z

So I’m currently struggling with okhttp3.internal. I want MockWebServer to use code from here (low-level HTTP/2 protocol implementation) but I don’t want to publish that package to all OSGi users. As a library author I was expecting OSGi would give me better encapsulation, but it sounds like that isn’t true here because of the internal sharing for MockWebServer.

If OSGi doesn’t add encapsulation, what does it add?

swankjesse · 2020-09-28T17:51:47Z

From the Okio bug:

Not providing OSGi metadata wastes many hours in many enterprises around the world

What are enterprises getting for those hours? Is it encapsulation, just not in this case?

swimmesberger · 2020-09-28T18:06:16Z

For the MockWebServer we should consider using a fragment, it allows us to keep the internal package private and still give the MockWebServer module the possibility to access those packages.

OSGi enforces encapsulation, the issue is getting libraries not written with OSGi in mind to fit into the OSGi corset. When a project is fully engaging in the OSGi world it gives you encapsulation no other ecosystem is giving you but it comes with a cost.
As an example: The model of enforcing versions for imported packages gives OSGi the ability to keep multiple versions of different libraries in a single JVM without any class loading/reference issues, each module knows exactly which version of a library it should load. This adds a lot of flexibility in large projects because every module can be developed independently without taking special care about library version. Of course to keep things clean it's best practice to keep all modules on the same page but when different teams develop different modules that get's more difficult. OSGi also provides a very flexible and clean "dependency injection" API with the DS specification.
Our project for example provides a couple of key services (you can think of microservices in a single JVM) that other modules can use (it's like GRPC/REST microservice just in a single JVM and "simple" method calls). For our use-case (non cloud based) it's a perfect fit because we have basically a microservice way of developing (with the requirement on JVM languages...) a application in a single monolithic application. Which is especially important because we have no way to deploy a set of services on docker or stuff like that because as I said we are not in an cloud environment.

OSGi also provides a very fine granular permission model (as I said only if you fully engage in the OSGi world) with the permission admin service where you can define exact permissions who can access which service and much more.

swankjesse · 2020-09-28T18:23:12Z

Yep, good point about the classloading features.

Can we make okhttp3.internal published for now, and switch to fragments later?

swimmesberger · 2020-09-28T18:44:14Z

Sure - later removing the package would "require" a major version bump though because OSGi bundles normally adhere semantic versioning and removing a previously exported package would break backwards compatibility. Of course providing OSGi metadata would still be better than providing nothing and later removing the package would be collateral damage 😅
What's the reasoning behind the decision to not use fragments for now? Because it's uncommon? Because of the PDE issue I mentioned?

JakeWharton · 2020-09-28T18:47:21Z

The internal package is already exposed to jar and java module system users and we break its binary API constantly. I don't think we'd treat removal of it from OSGi as anything but a bug fix.

swankjesse · 2020-09-28T22:41:31Z

Wish list:

Nothing fragile. If I inadvertently break OSGi’s rules, it should fail the build.
Easy to maintain. I don't want to study bnd documentation.
More encapsulation. This is my lowest priority.

I think this means we publish internal because it's least fragile.

swimmesberger · 2020-09-29T10:37:42Z

I have added a test to verify that okhttp with all sibling modules can be resolved properly. The test fails currently because the dependent okio library used is not OSGi ready yet. Therefore the okio library can't be resolved.
I also only apply the bnd plugin to selected projects therefore mockwebserver, okhttp-hpacktests and okhttp-testing-support do not contain any OSGi metadata. For testing libraries OSGi metadata does not really make sense therefore I excluded them for now.

I have also changed the bnd classpath to use the OSGi ready kotlin stdlib with that in place bnd generates Import-Package statements for a specific kotlin stdlib version e.g. "kotlin;version="[1.4,2)"" instead of a unversioned Import-package statement. This ensures that if there are multiple kotlin stdlib versions on the OSGi classpath it always uses the correct version. I would recommend the same thing for okio too but I just want to hear back from you what you think before I create a pull request.

codefromthecrypt

Thanks for the help with this. cc @reta if you don't mind helping review

codefromthecrypt · 2020-09-29T23:51:25Z

okhttp/src/test/java/okhttp3/osgi/OkHttpOsgiTest.java

+import java.util.stream.Stream;
+
+/**
+ * Tries to resolve the OSGi metadata specified in the


someone died writing? :)

Hah - fixed it 🤷‍♂️

codefromthecrypt · 2020-09-29T23:52:48Z

okhttp/src/test/resources/okhttp3/osgi/resolveTest.bndrun

@@ -0,0 +1,13 @@
+-runfw: org.eclipse.osgi


it feels weird to split responsibility between code and text like this. I'm not sure it is bad per se, but I think sometimes of testcontainers and enjoy seeing the setup logic in one markup (even if that is compiled :) )

I have moved the bnd instructions from the bndrun, cnf/build.bnd into code so everything should be in one place now.

codefromthecrypt · 2020-09-29T23:55:45Z

okhttp/build.gradle

+ bnd '''
+ Export-Package: \
+ okhttp3,\
+ okhttp3.internal.*


in zipkin we do things like this..

export your internal package..
https://github.com/openzipkin/brave/blob/master/brave/bnd.bnd#L9

explicitly import when internal is used
https://github.com/openzipkin/brave/blob/master/context/slf4j/bnd.bnd#L1

block import when something is source retention..
https://github.com/openzipkin/brave/blob/master/instrumentation/http/bnd.bnd#L2

codefromthecrypt · 2020-09-29T23:57:03Z

okhttp/build.gradle

+ dalvik.system;resolution:=optional,\
+ org.conscrypt;resolution:=optional,\
+ org.bouncycastle.*;resolution:=optional,\
+ org.openjsse.*;resolution:=optional,\


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

swimmesberger · 2020-09-30T08:22:54Z

@adriancole thanks for helping out with this

swimmesberger · 2020-10-02T06:09:17Z

@adriancole @swankjesse is there anything still missing for this PR? @swankjesse is there a date when we can expect that a new okio version is released which we can use in okhttp so the tests get back to green?

codefromthecrypt

Looks good to me, disclaimer being I'm not an OSGi person.

In projects I've worked on, we used servicemix bundles of okhttp, so it is probably worth pinging someone from there for a second set of eyes cc @jbonofre @WillemJiang @iocanel

For example zipkin almost always uses okhttp for client reporting. With this in, we'd likely switch out re-bundling

yschimke · 2020-10-06T07:14:41Z

@swimmesberger bumping okio deps here #6307, I believe Jesse's plan is RC of okhttp to test this and graal specifically.

swankjesse · 2020-10-06T21:51:30Z

CONTRIBUTING.md

@@ -46,6 +46,14 @@ Running Android Tests

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

+OSGi
+---------------------


Thanks for adding this

swankjesse · 2020-10-06T21:59:34Z

build.gradle

+ '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}"


this is a 4.2M .jar, compared to 1.4M for kotlin-stdlib, presumably because this includes kotlin-reflect.

swankjesse · 2020-10-06T22:00:34Z

build.gradle

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


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

swankjesse · 2020-10-07T01:35:09Z

I attempted to run this locally, and it failed like this:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Failed to deploy idea_rt.jar
Failed to deploy junit5-rt.jar
Failed to deploy junit-rt.jar
Deployed okio-jvm-2.9.0.jar
Failed to deploy kotlin-stdlib-1.4.10.jar
Failed to deploy android-all-11-robolectric-6757853.jar
Deployed bctls-jdk15on-1.66.jar
Deployed bcprov-jdk15on-1.66.jar
Deployed conscrypt-openjdk-uber-2.5.1.jar
Failed to deploy openjsse-1.1.0.jar
Deployed jsr305-3.0.2.jar
Failed to deploy animal-sniffer-annotations-1.19.jar
Failed to deploy svm-20.2.0.jar
Failed to deploy kotlin-stdlib-common-1.4.10.jar
Failed to deploy annotations-13.0.jar
Failed to deploy graal-sdk-20.2.0.jar
Failed to deploy kotlin-stdlib-jdk8-1.4.10.jar
Failed to deploy kotlin-stdlib-jdk7-1.4.10.jar
Failed to deploy junit-4.13.jar
Deployed assertj-core-3.15.0.jar
Deployed biz.aQute.resolve-5.1.2.jar
Failed to deploy AmazonCorrettoCryptoProvider-1.3.1-linux-x86_64.jar
Deployed dec-0.1.2.jar
Failed to deploy hamcrest-core-1.3.jar
Deployed biz.aQute.repository-5.1.2.jar
Deployed biz.aQute.bndlib-5.1.2.jar
Deployed slf4j-api-1.7.25.jar
Deployed bcpkix-jdk15on-1.66.jar

org.osgi.service.resolver.ResolutionException: Unable to resolve <<INITIAL>>: missing requirement osgi.identity;filter:='(osgi.identity=com.squareup.okhttp3)'

	at biz.aQute.resolve.ResolveProcess.augment(ResolveProcess.java:354)
	at biz.aQute.resolve.ResolveProcess.augment(ResolveProcess.java:211)
	at biz.aQute.resolve.ResolveProcess.resolveRequired(ResolveProcess.java:83)
	at biz.aQute.resolve.RunResolution.resolve(RunResolution.java:83)
	at biz.aQute.resolve.Bndrun.resolve(Bndrun.java:100)
	at biz.aQute.resolve.Bndrun.resolve(Bndrun.java:94)
	at okhttp3.osgi.OkHttpOsgiTest.testMainModuleWithSiblings(OkHttpOsgiTest.java:63)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
Caused by: org.apache.felix.resolver.reason.ReasonException: Unable to resolve <<INITIAL>>: missing requirement osgi.identity;filter:='(osgi.identity=com.squareup.okhttp3)'
	at org.apache.felix.resolver.Candidates$MissingRequirementError.toException(Candidates.java:1341)
	at org.apache.felix.resolver.ResolverImpl.doResolve(ResolverImpl.java:434)
	at org.apache.felix.resolver.ResolverImpl.resolve(ResolverImpl.java:421)
	at org.apache.felix.resolver.ResolverImpl.resolve(ResolverImpl.java:375)
	at biz.aQute.resolve.BndResolver.resolve(BndResolver.java:32)
	at biz.aQute.resolve.ResolveProcess.resolveRequired(ResolveProcess.java:81)
	... 30 more

swankjesse · 2020-10-07T01:41:46Z

Okay, figured it out! I needed to run tests with Gradle, and I needed to add com.oracle.svm.core.annotate to our imported packages.

swankjesse · 2020-10-07T02:42:15Z

Merged manually, including some small style changes. 74f9781

Thanks for your patience and endurance on this!

swimmesberger · 2020-10-07T06:25:27Z

Thanks everyone involved! @swankjesse FYI: In the final commit the CONTRIBUTING.md stuff seems to be missing. I can also understand the concerns about using "kotlin-osgi-bundle" but we only use it in the osgiApi configuration which will only be added to the bnd classpath. Therefore bnd could potentially use classes/packages (to calculate stuff) not available in the "default" jar but in practice it simply leads to proper versioned packages which is always a win for OSGi.

swankjesse · 2020-10-07T09:47:25Z

@swimmesberger the CONTRIBUTING change is deliberate. We've never had an external contributor introduce a new dependency and I think that documenting how to do it might imply that it was a thing we would expect or accept. OkHttp is very prudent with its dependencies (2!) so I don't think we lose much here.

Added bnd plugin

2b5c155

yschimke requested review from swankjesse and JakeWharton September 27, 2020 17:33

swankjesse reviewed Sep 28, 2020

View reviewed changes

Fixed comment

8d2a979

Added OSGi test, only apply bnd plugin to selected projects

e02bd10

codefromthecrypt mentioned this pull request Sep 29, 2020

Light OSGi tests openzipkin/brave#1254

Open

codefromthecrypt reviewed Sep 29, 2020

View reviewed changes

swimmesberger added 2 commits September 30, 2020 10:04

Renamed osgi test deployment configuration for better clarity

c749a35

Moved workspace/bndrun definition into code instead of text files

ef432d9

swimmesberger added 5 commits September 30, 2020 13:18

Better comments, cleanup

3d3a006

Removed cleared comment

6992eed

Preserving attr order

3accc40

Added OSGi documentation to CONTRIBUTING.md

bb104b3

Added okhttpinternal package attribute to export package statement

1d215e3

codefromthecrypt approved these changes Oct 2, 2020

View reviewed changes

yschimke added this to the 4.10 milestone Oct 6, 2020

swankjesse approved these changes Oct 7, 2020

View reviewed changes

swankjesse closed this Oct 7, 2020

swimmesberger mentioned this pull request Oct 7, 2020

Added OSGi metadata via build square/retrofit#3477

Open

Added OSGi metadata via build #6282

Added OSGi metadata via build #6282

Conversation

swimmesberger commented Sep 27, 2020

Choose a reason for hiding this comment

swimmesberger Sep 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swimmesberger Sep 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swimmesberger Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swimmesberger Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swankjesse commented Sep 28, 2020

swimmesberger commented Sep 28, 2020

swankjesse commented Sep 28, 2020

swankjesse commented Sep 28, 2020

swimmesberger commented Sep 28, 2020 • edited Loading

swankjesse commented Sep 28, 2020

swimmesberger commented Sep 28, 2020

JakeWharton commented Sep 28, 2020

swankjesse commented Sep 28, 2020

swimmesberger commented Sep 29, 2020 • edited Loading

codefromthecrypt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swimmesberger commented Sep 30, 2020

swimmesberger commented Oct 2, 2020 • edited Loading

codefromthecrypt left a comment

Choose a reason for hiding this comment

yschimke commented Oct 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swankjesse commented Oct 7, 2020

swankjesse commented Oct 7, 2020

swankjesse commented Oct 7, 2020

swimmesberger commented Oct 7, 2020 • edited Loading

swankjesse commented Oct 7, 2020 • edited Loading

swimmesberger Sep 28, 2020 •

edited

Loading

swimmesberger Sep 28, 2020 •

edited

Loading

swimmesberger Sep 30, 2020 •

edited

Loading

swimmesberger Sep 30, 2020 •

edited

Loading

swimmesberger commented Sep 28, 2020 •

edited

Loading

swimmesberger commented Sep 29, 2020 •

edited

Loading

swimmesberger commented Oct 2, 2020 •

edited

Loading

swimmesberger commented Oct 7, 2020 •

edited

Loading

swankjesse commented Oct 7, 2020 •

edited

Loading