fix #913 Apply BND convention to shadowJar as well

[simonbasle]

The bnd plugin only applies to the default jar task. Since we're now
using the shadow plugin, shadowJar has no OSGI data in its MANIFEST.

This commit fixes the situation by applying the bnd plugin convention to
the shadowJar task. It also copies the bnd configuration since this is
not inherited.

[simonbasle]

cc @simondaudin @agjini @swimmesberger can you look at the generated MANIFEST and assert its correctness?

MANIFEST for `shadowJar`

Manifest-Version: 1.0
Automatic-Module-Name: reactor.netty
Bnd-LastModified: 1574879057870
Bundle-ManifestVersion: 2
Bundle-Name: reactor-netty
Bundle-SymbolicName: reactor-netty
Bundle-Version: 0.9.3.BUILD-SNAPSHOT
Created-By: 1.8.0_222 (AdoptOpenJDK)
Export-Package: reactor.netty;uses:="io.netty.bootstrap,io.netty.buffe
 r,io.netty.channel,io.netty.util.concurrent,org.reactivestreams,react
 or.core,reactor.core.publisher,reactor.util.annotation,reactor.util.c
 ontext";version="0.9.3",reactor.netty.channel;uses:="io.micrometer.co
 re.instrument,io.netty.bootstrap,io.netty.buffer,io.netty.channel,io.
 netty.handler.logging,org.reactivestreams,reactor.core,reactor.core.p
 ublisher,reactor.netty,reactor.util.context";version="0.9.3",reactor.
 netty.http;uses:="io.micrometer.core.instrument,io.netty.buffer,io.ne
 tty.channel,io.netty.handler.codec.http,io.netty.handler.codec.http.c
 ookie,org.reactivestreams,reactor.core.publisher,reactor.netty,reacto
 r.netty.channel,reactor.netty.resources,reactor.netty.tcp,reactor.uti
 l.annotation";version="0.9.3",reactor.netty.http.client;uses:="io.net
 ty.bootstrap,io.netty.buffer,io.netty.handler.codec.http,io.netty.han
 dler.codec.http.cookie,io.netty.handler.codec.http.multipart,org.reac
 tivestreams,reactor.core.publisher,reactor.netty,reactor.netty.http,r
 eactor.netty.http.websocket,reactor.netty.resources,reactor.netty.tcp
 ,reactor.util.context";version="0.9.3",reactor.netty.http.server;uses
 :="io.netty.handler.codec.http,io.netty.handler.codec.http.cookie,org
 .reactivestreams,reactor.core.publisher,reactor.netty,reactor.netty.h
 ttp,reactor.netty.http.websocket,reactor.netty.tcp";version="0.9.3",r
 eactor.netty.http.websocket;uses:="io.netty.buffer,io.netty.handler.c
 odec.http,io.netty.handler.codec.http.websocketx,org.reactivestreams,
 reactor.core.publisher,reactor.netty,reactor.util.annotation";version
 ="0.9.3",reactor.netty.resources;uses:="io.netty.bootstrap,io.netty.c
 hannel,io.netty.channel.socket,reactor.blockhound,reactor.blockhound.
 integration,reactor.core,reactor.core.publisher,reactor.netty,reactor
 .util.annotation";version="0.9.3",reactor.netty.tcp;uses:="io.netty.b
 ootstrap,io.netty.channel,io.netty.channel.socket,io.netty.handler.co
 dec.http,io.netty.handler.logging,io.netty.handler.proxy,io.netty.han
 dler.ssl,io.netty.resolver,io.netty.util,org.reactivestreams,reactor.
 core.publisher,reactor.netty,reactor.netty.channel,reactor.netty.reso
 urces,reactor.util.annotation";version="0.9.3",reactor.netty.udp;uses
 :="io.netty.bootstrap,io.netty.channel,io.netty.channel.socket,io.net
 ty.handler.logging,io.netty.util,org.reactivestreams,reactor.core.pub
 lisher,reactor.netty,reactor.netty.channel,reactor.netty.resources";v
 ersion="0.9.3"
Implementation-Title: reactor-netty
Implementation-Version: 0.9.3.BUILD-SNAPSHOT
Import-Package: io.netty.channel.kqueue;resolution:=optional;version="
 [4.1,5)",io.netty.handler.codec.haproxy;resolution:=optional;version=
 "[4.1,5)",io.micrometer.core.instrument;resolution:=optional,io.micro
 meter.core.instrument.composite;resolution:=optional,io.micrometer.co
 re.instrument.config;resolution:=optional,reactor.blockhound;resoluti
 on:=optional,reactor.blockhound.integration;resolution:=optional,io.n
 etty.bootstrap;version="[4.1,5)",io.netty.buffer;version="[4.1,5)",io
 .netty.channel;version="[4.1,5)",io.netty.channel.epoll;version="[4.1
 ,5)",io.netty.channel.nio;version="[4.1,5)",io.netty.channel.socket;v
 ersion="[4.1,5)",io.netty.channel.socket.nio;version="[4.1,5)",io.net
 ty.handler.codec;version="[4.1,5)",io.netty.handler.codec.http;versio
 n="[4.1,5)",io.netty.handler.codec.http.cookie;version="[4.1,5)",io.n
 etty.handler.codec.http.multipart;version="[4.1,5)",io.netty.handler.
 codec.http.websocketx;version="[4.1,5)",io.netty.handler.codec.http.w
 ebsocketx.extensions.compression;version="[4.1,5)",io.netty.handler.c
 odec.http2;version="[4.1,5)",io.netty.handler.logging;version="[4.1,5
 )",io.netty.handler.proxy;version="[4.1,5)",io.netty.handler.ssl;vers
 ion="[4.1,5)",io.netty.handler.stream;version="[4.1,5)",io.netty.hand
 ler.timeout;version="[4.1,5)",io.netty.resolver;version="[4.1,5)",io.
 netty.util;version="[4.1,5)",io.netty.util.concurrent;version="[4.1,5
 )",javax.net.ssl,org.reactivestreams;version="[1.0,2)",reactor.core;v
 ersion="[3.3,4)",reactor.core.publisher;version="[3.3,4)",reactor.cor
 e.scheduler;version="[3.3,4)",reactor.util;version="[3.3,4)",reactor.
 util.annotation;version="[3.3,4)",reactor.util.concurrent;version="[3
 .3,4)",reactor.util.context;version="[3.3,4)"
Private-Package: reactor.netty.internal.shaded.reactor.pool
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-4.2.0.201903051501

[violetagg]

Can we add a test that checks that OSGi metadata is generated?
We have checks for the final binary here https://github.com/reactor/reactor-netty/blob/master/src/jarFileTest/java/reactor/netty/JarFileShadingTest.java

[violetagg]

do we need this org.gradle.api.internal.plugins.osgi.OsgiHelper

[simonbasle]

I removed it, but that made me realize that the test I was adding had to account for the case where a custom version is built with an additional version component 👍

[swimmesberger]

@simonbasle at first glance the posted manifest seems ok - when I compare it to the 0.9.1 version of the manifest I see a difference in the symbolic name:
0.9.1.RELEASE = io.projectreactor.netty.reactor-netty
0.9.3.BUILD-SNAPSHOT = reactor-netty

That should probably corrected so we don't have a symbolic name change between a minor version (maybe that can be unit tested too?)

I would also suggest that the string provided to bnd for both build task (shaded and not shaded) should be extracted into a variable which can be used by both to ensure that for future changes both tasks are similarly up to date.

[simondaudin]

@simonbasle the manifest seems ok, I see correct optional imports in 'Import-Package' section.
I managed to launch our application with the generated jar.

[simonbasle]

@violetagg added tests for the content of the MANIFEST, and addressed the inconsistencies found by @swimmesberger

[simondaudin]

Actually, I have one issue, with 'internal' imports
In version 0.8.x there is an import io.netty.util.internal;version="[ 4.1,5)" in "Import-Package" section of the Manifest
In version 0.9.x there is not (I guess due to "Import-Package": '!*internal*,...)
I took a look in the code, and the only place an internal package is use is in PooledConnectionProvider with import io.netty.util.internal.PlatformDependent;
At runtime, in my tests, I actually have a org.apache.felix.log.LogException: java.lang.NoClassDefFoundError: io/netty/util/internal/PlatformDependent
That, I guess, is due to that.

[swimmesberger]

Like @simondaudin said by further comparing the 0.9.1.RELEASE version with the 0.9.3.BUILD-SNAPSHOT the following is missing:

• "io.netty.util.internal;version="[4.1,5)"

[simonbasle]

@simondaudin @swimmesberger please suggest a configuration for bndtools to fix that, I have no idea why and how all this stuff is generated, nor whether or not these differences between what aQute.bnd and the deprecated osgi Gradle plugin generate are meaningful...

[swimmesberger]

If the idea was to simply blacklist the shaded "reactor.netty.internal" package from the manifest changing "!internal" to "!reactor.netty.internal*" would be enough.

[simonbasle]

@swimmesberger @simondaudin that's for the Import-Package, right? Does the exportcontents clause and result seem correct?

[simonbasle]

ok I'll need to split this PR into two: one is fixing the shadowJar having no OSGI metadata at all (913) and the other one is the SymbolicName and internal issues. One only affect 0.9.x while the later also impacts 0.8.x :(

[swimmesberger]

@simonbasle I kind of advice against using such a broad wildcard like !internal. So if you simply want to ensure that the shaded package is not exported you can safely change it to "!reactor.netty.internal*" too.

Generally I would suggest following instructions:

Export-Package: !reactor.netty.internal*,*;-noimport:=true
Import-Package: !reactor.netty.internal*,!javax.annotation,io.netty.channel.kqueue;resolution:=optional;version="[4.1,5)",io.netty.handler.codec.haproxy;resolution:=optional;version="[4.1,5)",!reactor.pool,io.micrometer.*;resolution:=optional,reactor.blockhound.*;resolution:=optional,*

Dunno why extactly you used exportcontents but Export-Package should work fine in most use cases.