jbang icon indicating copy to clipboard operation
jbang copied to clipboard

consider support //!DEPS for excluding dependencies

Open maxandersen opened this issue 4 years ago • 15 comments

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

maxandersen avatar Jun 30 '20 10:06 maxandersen

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...

OLibutzki avatar Aug 16 '21 08:08 OLibutzki

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 :)

maxandersen avatar Aug 17 '21 13:08 maxandersen

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.

OLibutzki avatar Aug 17 '21 13:08 OLibutzki

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.

maxandersen avatar Aug 17 '21 16:08 maxandersen

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 avatar Aug 17 '21 16:08 OLibutzki

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.

OLibutzki avatar Aug 21 '21 08:08 OLibutzki

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 ?

maxandersen avatar Aug 21 '21 08:08 maxandersen

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 :-)

quintesse avatar Aug 21 '21 08:08 quintesse

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 avatar Aug 21 '21 10:08 OLibutzki

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

OLibutzki avatar Aug 21 '21 10:08 OLibutzki

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 avatar Aug 21 '21 11:08 maxandersen

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 avatar Aug 21 '21 11:08 maxandersen

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 ?

maxandersen avatar Aug 21 '21 11:08 maxandersen

Will the issue be fixed/improvement implemented?

franden avatar Nov 23 '21 19:11 franden

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

maxandersen avatar Nov 23 '21 20:11 maxandersen