Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Identical packages in opentracing-api and opentracing-noop cause build failure with JDK 9 + jigsaw #145

Closed
kratz74 opened this issue May 16, 2017 · 7 comments

Comments

@kratz74
Copy link
Contributor

kratz74 commented May 16, 2017

Maven dependency on opentracing-util required to use GlobalTracer.get() introduced transitive dependency on opentracing-noop and opentracing-api.
Unfortunately both opentracing-noop and opentracing-api contains the same package "io.opentracing" and this causes error in JDK 9 + jigsaw projects containing module-info.java.

[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] module <module_name> reads package io.opentracing from both opentracing.api and opentracing.noop
[ERROR] module reads package io.opentracing from both opentracing.noop and opentracing.api
[ERROR] module opentracing.api reads package io.opentracing from both opentracing.noop and opentracing.api
[ERROR] module opentracing.util reads package io.opentracing from both opentracing.noop and opentracing.api
[ERROR] module opentracing.noop reads package io.opentracing from both opentracing.noop and opentracing.api

It would be good to rename "io.opentracing" package in opentracing-noop to something like "io.opentracing.noop"

@kratz74
Copy link
Contributor Author

kratz74 commented May 16, 2017

This fixed this problem in my current dependency - 0.21.0 (release-0.21.0 tag)
release-0.21.0.patch.zip
Similar change should be applied on master too.

@bhs
Copy link
Contributor

bhs commented May 27, 2017

@kratz74 sorry for the delay. Can you submit a PR and @bhs me?

@pavolloffay
Copy link
Member

Agree that noop should live in a separate package. I tried to compile with jdk-9+171 and it worked (I had to bump versions of some plugins).

@kratz74 do you have steps to reproduce your build?

@kratz74
Copy link
Contributor Author

kratz74 commented May 30, 2017

bugSample.zip
Here is simple mvn project to reproduce it. It needs module-info.java

@kratz74
Copy link
Contributor Author

kratz74 commented May 30, 2017

Added PR against temporary fork in my workspace. I'll delete this fork when this is merged.

@bhs here it is
@pavolloffay let me know if you see build error. there should be

-------------------------------------------------------------
COMPILATION ERROR : 
-------------------------------------------------------------
module  reads package io.opentracing from both opentracing.noop and opentracing.api
module opentracing.noop reads package io.opentracing from both opentracing.noop and opentracing.api
module opentracing.util reads package io.opentracing from both opentracing.api and opentracing.noop
module opentracing.api reads package io.opentracing from both opentracing.noop and opentracing.api
module-info.java:[1,1] module bugsample reads package io.opentracing from both opentracing.api and opentracing.noop
5 errors 

Edit: Now tested against patched 0.30.1-SNAPSHOT and build passed.

@pavolloffay
Copy link
Member

pavolloffay commented May 30, 2017

@kratz74 thanks for submitting #150 I agree that noop should ideally live in a separate package. I am wondering whether there are other solutions to this (as it will be breaking change). I will have a closer loot tomorrow.

@pavolloffay let me know if you see build error. there should be

confirmed it does not build with provided configuration.

@sparkoo
Copy link

sparkoo commented May 31, 2017

Jigsaw does not allow same package name in two or more modules. So I guess only reasonable option is to rename one of the packages as #150 suggests.

Note: When two modules has same package and both exports it and other module reads them, it fails at compile time (your case).
When two modules has same package and one of them exports it and other module reads them, it fails at startup time.
Anyway not possible in current state of jigsaw.

@bhs bhs closed this as completed in 6dee0c9 Jun 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants