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

consider support //!DEPS for excluding dependencies #150

Open
maxandersen opened this issue Jun 30, 2020 · 15 comments
Open

consider support //!DEPS for excluding dependencies #150

maxandersen opened this issue Jun 30, 2020 · 15 comments

Comments

@maxandersen
Copy link
Collaborator

maxandersen commented Jun 30, 2020

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
Copy link
Contributor

OLibutzki commented Aug 16, 2021

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
Copy link
Collaborator Author

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
Copy link
Contributor

OLibutzki commented Aug 17, 2021

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
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@maxandersen
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link

franden commented Nov 23, 2021

Will the issue be fixed/improvement implemented?

@maxandersen
Copy link
Collaborator Author

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

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

4 participants