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.

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

3.x: osgi manifest.mf Bundle-SymbolicName not following conventions and *.internal.* packages not exported #7318

Closed
scottslewis opened this issue Aug 24, 2021 · 5 comments · Fixed by #7320

Comments

@scottslewis
Copy link

Howdy

In the manifest.mf of 3.1.0 rxjava.jar (from maven central) is this:

Manifest-Version: 1.0
Automatic-Module-Name: io.reactivex.rxjava3
Bnd-LastModified: 1628493098021
Bundle-Description: Reactive Extensions for the JVM – a library for
composing asynchronous and event-based programs using observable sequ
ences for the Java VM.
Bundle-DocURL: https://github.com/ReactiveX/RxJava
Bundle-ManifestVersion: 2
Bundle-Name: rxjava
Bundle-SymbolicName: rxjava

The Bundle-SymbolicName is typically fully qualified...e.g. here's what's in the rxjava version 2.2.9:

Manifest-Version: 1.0
Automatic-Module-Name: io.reactivex.rxjava2
Bundle-SymbolicName: io.reactivex.rxjava2.rxjava
Bundle-ManifestVersion: 2
Bnd-LastModified: 1584086650000
Bundle-DocURL: https://github.com/ReactiveX/RxJava
Bundle-Vendor: RxJava Contributors

Gor consistency, shouldn't the 3.1.0 bundle symbolic name be:

io.reactivex.rxjava3.rxjava

?

A second manifest issue:

In the 2.2.9 version of the manifest, Export-Package has all the packages (even internal) that exist within the rxjava namespace...i.e.

Export-Package: io.reactivex;version="2.2.19";uses:="io.reactivex.anno
tations,io.reactivex.disposables,io.reactivex.flowables,io.reactivex.
functions,io.reactivex.observables,io.reactivex.observers,io.reactive
x.parallel,io.reactivex.schedulers,io.reactivex.subscribers,org.react
ivestreams",io.reactivex.annotations;version="2.2.19",io.reactivex.di
sposables;version="2.2.19";uses:="io.reactivex.functions,io.reactivex
.internal.disposables,org.reactivestreams",io.reactivex.exceptions;ve
rsion="2.2.19",io.reactivex.flowables;version="2.2.19";uses:="io.reac
tivex,io.reactivex.annotations,io.reactivex.disposables,io.reactivex.
functions",io.reactivex.functions;version="2.2.19",io.reactivex.inter
nal.disposables;version="2.2.19";uses:="io.reactivex,io.reactivex.dis
posables,io.reactivex.functions,io.reactivex.internal.fuseable",io.re
activex.internal.functions;version="2.2.19";uses:="io.reactivex,io.re
activex.functions,io.reactivex.schedulers,org.reactivestreams",io.rea
ctivex.internal.fuseable;version="2.2.19";uses:="io.reactivex,io.reac
tivex.disposables,org.reactivestreams",io.reactivex.internal.observer
s;version="2.2.19";uses:="io.reactivex,io.reactivex.disposables,io.re
activex.functions,io.reactivex.internal.fuseable,io.reactivex.interna
l.util,io.reactivex.observers,org.reactivestreams",io.reactivex.inter
nal.operators.completable;version="2.2.19";uses:="io.reactivex,io.rea
ctivex.disposables,io.reactivex.functions,org.reactivestreams",io.rea
ctivex.internal.operators.flowable;version="2.2.19";uses:="io.reactiv
ex,io.reactivex.disposables,io.reactivex.flowables,io.reactivex.funct
ions,io.reactivex.internal.disposables,io.reactivex.internal.fuseable
,io.reactivex.internal.subscriptions,io.reactivex.internal.util,io.re
activex.schedulers,org.reactivestreams",io.reactivex.internal.operato
rs.maybe;version="2.2.19";uses:="io.reactivex,io.reactivex.disposable
s,io.reactivex.functions,io.reactivex.internal.fuseable,io.reactivex.
observers,org.reactivestreams",io.reactivex.internal.operators.mixed;
version="2.2.19";uses:="io.reactivex,io.reactivex.disposables,io.reac
tivex.functions,io.reactivex.internal.util,org.reactivestreams",io.re
activex.internal.operators.observable;version="2.2.19";uses:="io.reac
tivex,io.reactivex.disposables,io.reactivex.functions,io.reactivex.in
ternal.disposables,io.reactivex.internal.fuseable,io.reactivex.intern
al.util,io.reactivex.observables,io.reactivex.schedulers,org.reactive
streams",io.reactivex.internal.operators.parallel;version="2.2.19";us
es:="io.reactivex,io.reactivex.functions,io.reactivex.internal.util,i
o.reactivex.parallel,org.reactivestreams",io.reactivex.internal.opera
tors.single;version="2.2.19";uses:="io.reactivex,io.reactivex.disposa
bles,io.reactivex.functions,org.reactivestreams",io.reactivex.interna
l.queue;version="2.2.19";uses:="io.reactivex.internal.fuseable",io.re
activex.internal.schedulers;version="2.2.19";uses:="io.reactivex,io.r
eactivex.disposables,io.reactivex.functions,io.reactivex.internal.dis
posables",io.reactivex.internal.subscribers;version="2.2.19";uses:="i
o.reactivex,io.reactivex.disposables,io.reactivex.functions,io.reacti
vex.internal.fuseable,io.reactivex.internal.subscriptions,io.reactive
x.internal.util,io.reactivex.observers,org.reactivestreams",io.reacti
vex.internal.subscriptions;version="2.2.19";uses:="io.reactivex.dispo
sables,io.reactivex.internal.fuseable,org.reactivestreams",io.reactiv
ex.internal.util;version="2.2.19";uses:="io.reactivex,io.reactivex.di
sposables,io.reactivex.functions,io.reactivex.internal.fuseable,org.r
eactivestreams",io.reactivex.observables;version="2.2.19";uses:="io.r
eactivex,io.reactivex.annotations,io.reactivex.disposables,io.reactiv
ex.functions",io.reactivex.observers;version="2.2.19";uses:="io.react
ivex,io.reactivex.disposables,io.reactivex.functions",io.reactivex.pa
rallel;version="2.2.19";uses:="io.reactivex,io.reactivex.annotations,
io.reactivex.functions,org.reactivestreams",io.reactivex.plugins;vers
ion="2.2.19";uses:="io.reactivex,io.reactivex.flowables,io.reactivex.
functions,io.reactivex.observables,io.reactivex.parallel,org.reactive
streams",io.reactivex.processors;version="2.2.19";uses:="io.reactivex
,io.reactivex.annotations,org.reactivestreams",io.reactivex.scheduler
s;version="2.2.19";uses:="io.reactivex",io.reactivex.subjects;version
="2.2.19";uses:="io.reactivex,io.reactivex.annotations,io.reactivex.d
isposables",io.reactivex.subscribers;version="2.2.19";uses:="io.react
ivex,io.reactivex.disposables,io.reactivex.functions,io.reactivex.obs
ervers,org.reactivestreams"

In the 3.1.0 version of manifest the .internal. packages are absent...i.e.

io.reactivex.rxjava3.annotations;version="3.1.0",io.re
activex.rxjava3.core;uses:="io.reactivex.rxjava3.annotations,io.react
ivex.rxjava3.disposables,io.reactivex.rxjava3.flowables,io.reactivex.
rxjava3.functions,io.reactivex.rxjava3.observables,io.reactivex.rxjav
a3.observers,io.reactivex.rxjava3.parallel,io.reactivex.rxjava3.sched
ulers,io.reactivex.rxjava3.subscribers,org.reactivestreams";version="
3.1.0",io.reactivex.rxjava3.disposables;uses:="io.reactivex.rxjava3.f
unctions,org.reactivestreams";version="3.1.0",io.reactivex.rxjava3.ex
ceptions;version="3.1.0",io.reactivex.rxjava3.flowables;uses:="io.rea
ctivex.rxjava3.annotations,io.reactivex.rxjava3.core,io.reactivex.rxj
ava3.disposables,io.reactivex.rxjava3.functions";version="3.1.0",io.r
eactivex.rxjava3.functions;version="3.1.0",io.reactivex.rxjava3.obser
vables;uses:="io.reactivex.rxjava3.annotations,io.reactivex.rxjava3.c
ore,io.reactivex.rxjava3.disposables,io.reactivex.rxjava3.functions";
version="3.1.0",io.reactivex.rxjava3.observers;uses:="io.reactivex.rx
java3.core,io.reactivex.rxjava3.disposables,io.reactivex.rxjava3.func
tions";version="3.1.0",io.reactivex.rxjava3.parallel;uses:="io.reacti
vex.rxjava3.annotations,io.reactivex.rxjava3.core,io.reactivex.rxjava
3.functions,org.reactivestreams";version="3.1.0",io.reactivex.rxjava3
.plugins;uses:="io.reactivex.rxjava3.core,io.reactivex.rxjava3.flowab
les,io.reactivex.rxjava3.functions,io.reactivex.rxjava3.observables,i
o.reactivex.rxjava3.parallel,org.reactivestreams";version="3.1.0",io.
reactivex.rxjava3.processors;uses:="io.reactivex.rxjava3.annotations,
io.reactivex.rxjava3.core,org.reactivestreams";version="3.1.0",io.rea
ctivex.rxjava3.schedulers;uses:="io.reactivex.rxjava3.core";version="
3.1.0",io.reactivex.rxjava3.subjects;uses:="io.reactivex.rxjava3.anno
tations,io.reactivex.rxjava3.core,io.reactivex.rxjava3.disposables";v
ersion="3.1.0",io.reactivex.rxjava3.subscribers;uses:="io.reactivex.r
xjava3.core,io.reactivex.rxjava3.disposables,io.reactivex.rxjava3.obs
ervers,org.reactivestreams";version="3.1.0"

Note that the .internal. packages are not present in the exported packages. Also 3.1.0 has Private-Packages defined as:

Private-Package: io.reactivex.rxjava3.internal.disposables,io.reactive
x.rxjava3.internal.functions,io.reactivex.rxjava3.internal.fuseable,i
o.reactivex.rxjava3.internal.jdk8,io.reactivex.rxjava3.internal.obser
vers,io.reactivex.rxjava3.internal.operators.completable,io.reactivex
.rxjava3.internal.operators.flowable,io.reactivex.rxjava3.internal.op
erators.maybe,io.reactivex.rxjava3.internal.operators.mixed,io.reacti
vex.rxjava3.internal.operators.observable,io.reactivex.rxjava3.intern
al.operators.parallel,io.reactivex.rxjava3.internal.operators.single,
io.reactivex.rxjava3.internal.queue,io.reactivex.rxjava3.internal.sch
edulers,io.reactivex.rxjava3.internal.subscribers,io.reactivex.rxjava
3.internal.subscriptions,io.reactivex.rxjava3.internal.util

Which is basically all of the .internal. packages.

It would be easier on consumers to keep the same exported packages (i.e. export packages including .internal. package) as version 2.2.9 since consumers that have a 2.x version that uses the .internal. packages can import those same packages in 3.x.

Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"

@akarnokd
Copy link
Member

Bundle-SymbolicName

The plugin we use derives it from ArtifactID by default. I don't know if changing it would break existing usages.

consumers that have a 2.x version that uses the .internal. packages

Internal packages are not meant to be used outside RxJava itself (or else bear the consequences of possibly breaking it in any future patch). It was a mistake with 2.x and fixed in #6675 a while ago.

@scottslewis
Copy link
Author

Bundle-SymbolicName

The plugin we use derives it from ArtifactID by default. I don't know if changing it would break existing usages.

It would, but IMHO it would be worth it to have the bundle id follow bundle well-established naming conventions...e.g.

io.reactivex.rxjava3.rxjava

Especially for an 'api' bundle like rxjava3.

Also, having it simply be 'rxjava' (as it is currently) could be really problematic for future,, as the symbolic name must be globally unique (which is why the convention is to use the fully qualified name)

consumers that have a 2.x version that uses the .internal. packages

Internal packages are not meant to be used outside RxJava itself (or else bear the consequences of possibly breaking it in any future patch). It was a mistake with 2.x and fixed in #6675 a while ago.

I understand that .internal. packages were not intended to be exposed in 2.x, but having these packages be forced to internal makes it much more complicated for 2.x consumers to move to 3.x...since there are a number of currently internal interfaces that simplify extension of reactivex.

As an example, reactivex-grpc uses several interfaces in internal.fuseable: QueueFuseable (constants), SimpleQueue, PlainSimpleQueue. Some of these are used only for junit tests. Actually almost all the classes in internal.fuseable are interfaces, which seems a bit strange for a non-api package.

If not all, then perhaps some these packages (or at least some of the interfaces/classes in those packages) could be moved to API packages?

@akarnokd
Copy link
Member

Alright, I'll look into changing the Bundle-SymbolicName and the relocation of the fusion interfaces.

@scottslewis
Copy link
Author

Alright, I'll look into changing the Bundle-SymbolicName and the relocation of the fusion interfaces.

Thank you.

For the record, here are all of the internal classes that reactivex-grpc uses now (in rxgrpc-stub):

Important:

internal.fueseable package: QueueFuseable, QueueSubscription, SimplePlainQueue, SimpleQueue

internal.queue package: SpscArrayQueue

Trivial

internal.util package: Pow2

internal.functions package: Functions

@akarnokd
Copy link
Member

Posted #7319 and #7320.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants