jbangdev / jbang

Unleash the power of Java - JBang Lets Students, Educators and Professional Developers create, edit and run self-contained source-only Java programs with unprecedented ease.
https://jbang.dev
MIT License
1.44k stars 159 forks source link

consider support //!DEPS for excluding dependencies #150

Open maxandersen opened 4 years ago

maxandersen commented 4 years ago

usecase:

[jbang] Building jar...
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/max/.m2/repository/ch/qos/logback/logback-classic/1.1.2/logback-classic-1.1.2.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/max/.m2/repository/org/slf4j/slf4j-nop/1.7.25/slf4j-nop-1.7.25.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [ch.qos.logback.classic.util.ContextSelectorStaticBinder]
revapi.(sh|bat) [-u|-h] -e <GAV>[,<GAV>]* -o <FILE>[,<FILE>]* -n <FILE>[,<FILE>]* [-s <FILE>[,<FILE>]*] [-t <FILE>[,<FILE>]*] [-D<CONFIG_OPTION>=<VALUE>]* [-c <FILE>[,<FILE>]*] [-r <DIR>]

which happens when you have:

//DEPS org.revapi:revapi-standalone:0.9.5
//DEPS org.jboss.shrinkwrap.resolver:shrinkwrap-resolver-api:3.1.4
//DEPS org.jboss.shrinkwrap.resolver:shrinkwrap-resolver-impl-maven:3.1.4
//DEPS org.slf4j:slf4j-nop:1.7.25
//DEPS info.picocli:picocli:4.2.0

could ofcourse just exclude slf4j-nop here but what if that is the one I want to use...thus we could have something like:

//!DEPS ch.qos.logback.logback-classic:1.1.2

to mark for exclusion instead of exclude

OLibutzki commented 3 years ago

I stumbled upon this issue as well. For a certain library I would like to exclude all transitive dependencies.

To be more precise. This is the exception I currently get:

Caused by: org.apache.logging.log4j.LoggingException: log4j-slf4j-impl cannot be present with log4j-to-slf4j
        at org.apache.logging.slf4j.Log4jLoggerFactory.validateContext(Log4jLoggerFactory.java:49)
        at org.apache.logging.slf4j.Log4jLoggerFactory.newLogger(Log4jLoggerFactory.java:39)
        at org.apache.logging.slf4j.Log4jLoggerFactory.newLogger(Log4jLoggerFactory.java:30)
        at org.apache.logging.log4j.spi.AbstractLoggerAdapter.getLogger(AbstractLoggerAdapter.java:53)
        ...

Without exclusions there is no way to get out of this...

maxandersen commented 3 years ago

So what you are after is that to be able to use a dependency but NOT get its transitive or would being able to exclude a dependency be what you are after ?

I think/fear both actually need doing as I bumped into both cases but thus far solved by fixing the broken dependencies upstream :)

OLibutzki commented 3 years ago

I guess excluding all transitive dependencies is a rare use case. I'd like to do something equivalent to maven exclusions:

<dependency>
  <groupId>...</groupId>
  <artifactId>...</artifactId>
  <version>...</version>
  <exclusions>
    <exclusion>
      <groupId>*</groupId>
      <artifactId>*</artifactId>
    </exclusion>
  </exclusions>
</dependency>

In this example it's used to exclude all transitive deps, but of course you can fine-tune it to exclude certain deps.

maxandersen commented 3 years ago

Hmm. That isn't a bad idea. Something like:

'//DEPS x:y:2.3!::*'

And then allow to have comma separate list of patterns after !

You won't be able to do a global exclude this way but can express exclude all deps or exclude specific deps per dep.

OLibutzki commented 3 years ago

You won't be able to do a global exclude this way but can express exclude all deps or exclude specific deps per dep.

Well, I have not been precise enough in my first comment. Your suggestion perfectly matches my requirements.

OLibutzki commented 3 years ago

In my opinion using a ! for exclusions makes perfect sense as ! is well known as not.

So to translate it into natural language: Add a dependency to this GAV, but not to these GAVs.

maxandersen commented 3 years ago

so just to outline the suggested expansion of dependency syntax:

//DEPS org.revapi:revapi-standalone:0.9.5 - fetch the dependency and its transitive dependencies

//DEPS org.revapi:revapi-standalone:0.9.5! - fetch the dependency and none of its transitive dependencies

//DEPS org.revapi:revapi-standalone:0.9.5!ch.qos.logback:logback-classic:1.1.2 - fetch the dependency and all of its transitive dependencies except ch.qos.logback:logback-classic:1.1.2

//DEPS org.revapi:revapi-standalone:0.9.5!ch.qos.logback:logback-classic:* - fetch the dependency and all of its transitive dependencies except any version of ch.qos.logback:logback-classic

//DEPS org.revapi:revapi-standalone:0.9.5!ch.qos.logback:*:*, //DEPS org.revapi:revapi-standalone:0.9.5!ch.qos.logback - fetch the dependency and all of its transitive dependencies except any artifact in the group ch.qos.logback.

//DEPS org.revapi:revapi-standalone:0.9.5!*:logback-classic:* - fetch the dependency and all of its transitive dependencies except any artifact with the name logback-classic.

The nice thing about this syntax is that it can be used on the command line too.

Probably want to figure out if #980 notion of runtime/compile dependencies is per dependency or just something done via //RDEPS or //DEPS(runtime) and thus would be per line and would for runtime require a --deps-runtime or similar flag.

Thoughts ?

quintesse commented 3 years ago

but do please suggest an alternative to this - i'm struggling to find one that is not worse

I don't have an alternative right now that is not worse, but I do feel that this increases complexity for that feature quite a bit so thinking about it some more might be advantageous. Because once we add this we'll have to support it forever :-)

OLibutzki commented 3 years ago

I like the syntax. The only thing that is too much magic in my opinion is //DEPS org.revapi:revapi-standalone:0.9.5!

Excluding all transitive deps is a rare use case and I prefer to make this more explicit by using //DEPS org.revapi:revapi-standalone:0.9.5!*:*:*

OLibutzki commented 3 years ago

And multiple transitive deps which should be excluded are comma-separated? You did not add an example for this use case.

maxandersen commented 3 years ago

Excluding all transitive deps is a rare use case and I prefer to make this more explicit by using //DEPS org.revapi:revapi-standalone:0.9.5!*:*:*

rareness is probably relative ;) i.e. any maven fat jar published in maven central i.e. quarkus cli will have a pom.xml that says it needs dependencies when it does not.

//DEPS io.quarkus:quarkus-cli:2.2.0.CR1;runner!

and jbang io.quarkus:quarkus-cli:2.2.0.CR1! is much nicer to deal with on command line than jbang jbang io.quarkus:quarkus-cli:2.2.0.CR1!*:*:*

thats the reson I prefer to allow the shorthand.

maxandersen commented 3 years ago

And multiple transitive deps which should be excluded are comma-separated? You did not add an example for this use case.

yes, comma separated with no whitespace allowed.

//DEPS io.quarkus:quarkus-cli:2.2.0.CR1!*:log4j*:*,something:somewhere:1.2.3

maxandersen commented 3 years ago

what I don't like is the inabiity to do a global exclude, i.e. always exclude log4j no matter what dependency its from.

Maybe allow //DEPS !*:log4j:* as a global filter to apply...question is if it should be global or only take effect after it been listed.

//DEPS io.quarkus:quarkus-cli:2.2.0.CR1
//DEPS !*:log4j:*
//DEPS io.something.log4j

would quarkus-cli be allowed to take in log4j ? I'm inclined to say no but that does mean we'll need to parse list in phases...

also, should //DEPS on something excluded be silently excluded or considered an error or even considered overruling the exclusoion ?

franden commented 3 years ago

Will the issue be fixed/improvement implemented?

maxandersen commented 3 years ago

Yes, Eventually. You want to make a go on it?