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

Do not create shaded artifact which prevents dependency override. #13

Closed
khmarbaise opened this issue Mar 24, 2018 · 7 comments
Closed

Comments

@khmarbaise
Copy link
Contributor

Currently your project shades the whole depenencies in particular org.ow2.* which means a consuming project can not override the used version by newer ones in cases where errors etc. occur.

khmarbaise added a commit to khmarbaise/jdependency that referenced this issue Mar 24, 2018
…ency override.

 o Based on the given configuration consumers can decide to use
   the shaded version or the default version.
@tcurdt
Copy link
Owner

tcurdt commented Mar 27, 2018

That's the exact idea of shading the dependencies. The library becomes a black box without transitive dependencies and the fixes have to be made by releasing the library. Overriding anything but minor releases of dependencies is a bad idea anyway. Which is why I am not really convinced it's worth maintaining another artifact. Not even sure how this would work with maven central.

Can you make a good case for it other than "I want to upgrade asm1 for java compatibility"?

Also see #11

@khmarbaise
Copy link
Contributor Author

Unfortunately it's not a black box cause you inherit the classes including the ones given inside the jar and furthermore the problem is if you have a issue with an older version of the library like ASM which is shaded you can't overwrite it in a simple manner (which is advantage of transitive dependencies). I'm aware of the problems based on minor versions etc. which would be my own responsibility ...but at the moment I'm trying to upgrade maven-shade-plugin (trying to make a new release) to newer versions of ASM based on JDK9 / JDK10 issues which is at the moment not possible...cause I can't overwrite the versions of ASM cause it's shaded inside of jdependency...

@tcurdt
Copy link
Owner

tcurdt commented Apr 1, 2018

The classes should be shaded that means they have completely different class names.

https://github.com/tcurdt/jdependency/blob/master/pom.xml#L143

Only inheritance would expose the rewritten package for ASM. The jdependency classes do not inherit from ASM only the internal ASM helper classes do that no one should use.

https://github.com/tcurdt/jdependency/tree/master/src/main/java/org/vafer/jdependency/asm

That said I think we should mark/document them as such.

So inheritance should not be a problem.
It should be a blackbox or otherwise there is a bug.

Which of course means if we need a newer version of ASM we also need to make a new jdependency release. Which to me sounds like the right thing to do.

@khmarbaise
Copy link
Contributor Author

Ah..haven't read the pom file carefully enough...It would be great If you could do a new release of jdependency...

@tcurdt
Copy link
Owner

tcurdt commented Apr 2, 2018

Right now I am on vacation with little/bad internet activity. I try my best. Otherwise it might have to wait 2 weeks. Can you confirm that master is all you need for the new release, @khmarbaise?

@tcurdt
Copy link
Owner

tcurdt commented Apr 27, 2018

@khmarbaise I am back in civilization. How about master and a release from it?

@tcurdt
Copy link
Owner

tcurdt commented May 18, 2018

Just release 1.4. This closes #15 and includes #14

@tcurdt tcurdt closed this as completed May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants