fix #920 OSGI: explicit SymbolicName and fix imports-exports

[simonbasle]

• the Import-Package clause was missing an entry
  io.netty.util.internal;version="[ 4.1,5)"
• the Bundle-SymbolicName clause was changed to project name instead
  of a more fully qualified name

[simonbasle]

cc @swimmesberger @simondaudin this one is a split off #915 that focuses on OSGI metadata content regressions due to switching to bnd plugin, while the old one will focus on generating metadata for the shadowJar.

[simonbasle]

@simonbasle I can of adive 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.

(copied from #915 discussion, let's continue discussion on the bnd configuration itself here @swimmesberger)

[simonbasle]

@swimmesberger AFAIK we use -exportcontents to export the packages but avoid including them in the classpath and thus in the generated jar. if I use Export-Package I get all the dependencies' classes in the resulting jar 🙅‍♂

[simondaudin]

I would say, that excluding internal packages from import and export Packages can be a proper convention if the project respect those, which seems to be the case. The only use of an internal package that I found was the one I mentioned in #915
PooledConnectionProvider with import io.netty.util.internal.PlatformDependent
I'd say, if that's important for you to ensure no usage of internal packages (except this one), you can add the exception of your exclusion in the "Import-Package" like
"Import-Package": '!*internal*,io.netty.util.internal;version="[4.1,5)",!javax.annotation,... (not sure if that's a good thing to do)
Otherwise, maybe, as @swimmesberger suggested, remove this broad wildcard: !*internal*

[swimmesberger]

@simonbasle thats because bnd respects everything on the classpath but if you would change the wildcard export "*" to simply: "reactor.netty.*" it would only consider packages starting with "reactor.netty". (except the internal one as we blacklisted that)

Therefore with the suggestion applied it would be:

Export-Package: !reactor.netty.internal*,reactor.netty.*;-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,*

Bnd way of thinking is being as explicit as possible therefore broad wildcards are generally discouraged (except for imports as bnd is good in analyzing imports).

Edit: I kind of wonder why we need to blacklist the "reactor.netty.internal" for imports anyway because "reactor.netty.internal" should be on the classpath when bnd calculates the imports therefore it should not consider it for imports? 🤔

[simonbasle]

@simonbasle thats because bnd respects everything on the classpath but if you would change the wildcard export "" to simply: "reactor.netty." it would only consider packages starting with "reactor.netty". (except the internal one as we blacklisted that)

ok it seems to do the trick for using Export-Package

Edit: I kind of wonder why we need to blacklist the "reactor.netty.internal" for imports anyway because "reactor.netty.internal" should be on the classpath when bnd calculates the imports therefore it should not consider it for imports? 🤔

Right, so we don't want to expose that to the outside world, but since it is internal bnd shouldn't consider it in Import-Package?

[simonbasle]

Edit: I kind of wonder why we need to blacklist the "reactor.netty.internal" for imports anyway because "reactor.netty.internal" should be on the classpath when bnd calculates the imports therefore it should not consider it for imports? 🤔

Right, so we don't want to expose that to the outside world, but since it is internal bnd shouldn't consider it in Import-Package?

Removing the exclude from Import-Package seems to make 0 difference in the output, indeed (when testing on 0.9.x branch)

[swimmesberger]

Export-Package lists the packages you want to expose to the outer world and
Import-Package says which packages you need from the outer world as the internal Package is part of the classpath bnd should notice that and therefore does not need to Import. Therefore the blacklist for Import should be unnecessary 👍

[codecov-io]

Codecov Report

Merging #921 into 0.8.x will increase coverage by 0.12%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##              0.8.x     #921      +/-   ##
============================================
+ Coverage     66.54%   66.67%   +0.12%     
  Complexity     1405     1405              
============================================
  Files           133      133              
  Lines          6523     6523              
  Branches        860      860              
============================================
+ Hits           4341     4349       +8     
+ Misses         1724     1718       -6     
+ Partials        458      456       -2

Impacted Files	Coverage Δ	Complexity Δ
...ctor/netty/resources/PooledConnectionProvider.java	78.49% <0%> (+0.71%)	21% <0%> (ø)	⬇️
.../main/java/reactor/netty/channel/MonoSendMany.java	67.07% <0%> (+1.21%)	4% <0%> (ø)	⬇️
src/main/java/reactor/netty/FutureMono.java	51.31% <0%> (+3.94%)	4% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 974f70f...cd3cedf. Read the comment docs.

[simonbasle]

ok @swimmesberger thanks for the help. I've updated the latest changes and here is what is generated for 0.8:

Details

Manifest-Version: 1.0
Bnd-LastModified: 1575046054902
Bundle-ManifestVersion: 2
Bundle-Name: reactor-netty
Bundle-SymbolicName: io.projectreactor.netty.reactor-netty
Bundle-Version: 0.8.15.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.8.15",reactor.netty.channel;uses:="io.netty.bootst
 rap,io.netty.buffer,io.netty.channel,io.netty.handler.logging,org.rea
 ctivestreams,reactor.core,reactor.core.publisher,reactor.netty,reacto
 r.util.context";version="0.8.15",reactor.netty.http;uses:="io.netty.b
 uffer,io.netty.channel,io.netty.handler.codec.http,io.netty.handler.c
 odec.http.cookie,org.reactivestreams,reactor.core.publisher,reactor.n
 etty,reactor.netty.channel,reactor.netty.resources,reactor.netty.tcp,
 reactor.util.annotation";version="0.8.15",reactor.netty.http.client;u
 ses:="io.netty.bootstrap,io.netty.buffer,io.netty.handler.codec.http,
 io.netty.handler.codec.http.cookie,io.netty.handler.codec.http.multip
 art,org.reactivestreams,reactor.core.publisher,reactor.netty,reactor.
 netty.http,reactor.netty.http.websocket,reactor.netty.resources,react
 or.netty.tcp,reactor.util.context";version="0.8.15",reactor.netty.htt
 p.server;uses:="io.netty.handler.codec.http,io.netty.handler.codec.ht
 tp.cookie,org.reactivestreams,reactor.core.publisher,reactor.netty,re
 actor.netty.http,reactor.netty.http.websocket,reactor.netty.tcp";vers
 ion="0.8.15",reactor.netty.http.websocket;uses:="io.netty.buffer,io.n
 etty.handler.codec.http,io.netty.handler.codec.http.websocketx,org.re
 activestreams,reactor.core.publisher,reactor.netty,reactor.util.annot
 ation";version="0.8.15",reactor.netty.resources;uses:="io.netty.boots
 trap,io.netty.channel,io.netty.channel.socket,reactor.core,reactor.co
 re.publisher,reactor.netty,reactor.util.annotation";version="0.8.15",
 reactor.netty.tcp;uses:="io.netty.bootstrap,io.netty.channel,io.netty
 .channel.socket,io.netty.handler.codec.http,io.netty.handler.logging,
 io.netty.handler.proxy,io.netty.handler.ssl,io.netty.resolver,io.nett
 y.util,org.reactivestreams,reactor.core.publisher,reactor.netty,react
 or.netty.resources,reactor.util.annotation";version="0.8.15",reactor.
 netty.udp;uses:="io.netty.bootstrap,io.netty.channel,io.netty.channel
 .socket,io.netty.handler.logging,io.netty.util,org.reactivestreams,re
 actor.core.publisher,reactor.netty,reactor.netty.resources";version="
 0.8.15"
Implementation-Title: reactor-netty
Implementation-Version: 0.8.15.BUILD-SNAPSHOT
Import-Package: io.netty.channel.kqueue;resolution:=optional;version="
 [4.1,5)",io.netty.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.cha
 nnel.pool;version="[4.1,5)",io.netty.channel.socket;version="[4.1,5)"
 ,io.netty.channel.socket.nio;version="[4.1,5)",io.netty.handler.codec
 ;version="[4.1,5)",io.netty.handler.codec.haproxy;version="[4.1,5)",i
 o.netty.handler.codec.http;version="[4.1,5)",io.netty.handler.codec.h
 ttp.cookie;version="[4.1,5)",io.netty.handler.codec.http.multipart;ve
 rsion="[4.1,5)",io.netty.handler.codec.http.websocketx;version="[4.1,
 5)",io.netty.handler.codec.http.websocketx.extensions.compression;ver
 sion="[4.1,5)",io.netty.handler.codec.http2;version="[4.1,5)",io.nett
 y.handler.logging;version="[4.1,5)",io.netty.handler.proxy;version="[
 4.1,5)",io.netty.handler.ssl;version="[4.1,5)",io.netty.handler.strea
 m;version="[4.1,5)",io.netty.handler.timeout;version="[4.1,5)",io.net
 ty.resolver;version="[4.1,5)",io.netty.util;version="[4.1,5)",io.nett
 y.util.concurrent;version="[4.1,5)",io.netty.util.internal;version="[
 4.1,5)",javax.net.ssl,org.reactivestreams;version="[1.0,2)",reactor.c
 ore;version="[3.2,4)",reactor.core.publisher;version="[3.2,4)",reacto
 r.core.scheduler;version="[3.2,4)",reactor.util;version="[3.2,4)",rea
 ctor.util.annotation;version="[3.2,4)",reactor.util.concurrent;versio
 n="[3.2,4)",reactor.util.context;version="[3.2,4)"
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-4.2.0.201903051501

[simonbasle]

@swimmesberger @simondaudin barring more feedback from you I'll merge this in a couple hours, worst case scenario we can reopen a follow-up PR. thanks for your help 👍

[swimmesberger]

@simonbasle the 0.8 manifest looks OK. If you provide a 0.9 manifest too I will take a quick final look over it 👍

[simonbasle]

@swimmesberger sure
here is the manifest of the shadowJar (final jar):

Shadow Jar MANIFEST

Manifest-Version: 1.0
Automatic-Module-Name: reactor.netty
Bnd-LastModified: 1575366943929
Bundle-ManifestVersion: 2
Bundle-Name: reactor-netty
Bundle-SymbolicName: io.projectreactor.netty.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
 )",io.netty.util.internal;version="[4.1,5)",javax.net.ssl,org.reactiv
 estreams;version="[1.0,2)",reactor.core;version="[3.3,4)",reactor.cor
 e.publisher;version="[3.3,4)",reactor.core.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

and here is the one of the original jar in 0.9:

Original Jar MANIFEST

Manifest-Version: 1.0
Automatic-Module-Name: reactor.netty
Bnd-LastModified: 1575366942340
Bundle-ManifestVersion: 2
Bundle-Name: reactor-netty
Bundle-SymbolicName: io.projectreactor.netty.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
 )",io.netty.util.internal;version="[4.1,5)",javax.net.ssl,org.reactiv
 estreams;version="[1.0,2)",reactor.core;version="[3.3,4)",reactor.cor
 e.publisher;version="[3.3,4)",reactor.core.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)"
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Tool: Bnd-4.2.0.201903051501

[swimmesberger]

The shadowJar manifest looks good!

For the original jar Manifest where the "reactor.pool.*" packages are not shaded the package should be imported.

Thats probably a issue because for the original jar somehow the import "reactor.pool" is probably blacklisted ("!reactor.pool").

In my opinion this blacklist is not needed because when bnd acts upon the shadowed jar class files there should be no reactor.pool.* package in use because in this case the packages used are "reactor.netty.internal.shaded.reactor.pool". In the original not shadowed jar bnd should find classes which use classes from the "reactor.pool" package and automatically add an import package for that.

Therefore removing the "!reactor.pool" blacklist from the Import-Package statement should fix that issue but I'm not 100% sure how bnd and shadowJar plays together.

[simonbasle]

Therefore removing the "!reactor.pool" blacklist from the Import-Package statement should fix that issue but I'm not 100% sure how bnd and shadowJar plays together.

Indeed @swimmesberger, I tried that and the original has the import while the shaded correctly doesn't 👍

[swimmesberger]

@simonbasle nice! Then we should be good to go 👍

[simondaudin]

@simonbasle Thank you for your work, it seems all good.
@swimmesberger Thank you for your insights, there were some details I did not really master.