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

Caffeine 2.7.0 extra dependencies #300

Closed
guidomedina opened this issue Feb 26, 2019 · 10 comments
Closed

Caffeine 2.7.0 extra dependencies #300

guidomedina opened this issue Feb 26, 2019 · 10 comments

Comments

@guidomedina
Copy link

@ben-manes it looks like there was a side effect with some of the new checkers you added, or; was this intended? I have to explicitly exclude them now:

        <dependency>
            <groupId>com.github.ben-manes.caffeine</groupId>
            <artifactId>caffeine</artifactId>
            <version>2.7.0</version>
            <exclusions>
                <exclusion>
                    <groupId>org.checkerframework</groupId>
                    <artifactId>checker-qual</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.errorprone</groupId>
                    <artifactId>error_prone_annotations</artifactId>
                </exclusion>
            </exclusions>
        </dependency>
@ben-manes
Copy link
Owner

The side effect should only be as benign runtime dependencies, unless you are seeing more issues? The intent was to follow Guava’s example which has

https://github.com/google/guava/blob/2763649a49907a645b97afd860ca74257368ab45/guava/pom.xml#L33-L40

Some JVM languages can’t handle missing annotations so excluding causes surprising errors. The old jsr305 annotations are not module friendly so moving off was requested. Excluding should be safe and okay. Or did I screw something else up?

@guidomedina
Copy link
Author

You didn't screw up anything ;-) I was just making you aware of these dependencies being dragged now, since this seems to be a norm then close this issue, I'm also doing the same for Guava (excluding these explicitly)

@ben-manes
Copy link
Owner

I’d prefer to have it as an optional dependency, but the thread on Guava shows it’s annoying for people either way...

@sco0ter
Copy link

sco0ter commented Jul 1, 2019

Having this issue, too:

mvn dependency:tree yields this ouput:

+- com.github.ben-manes.caffeine:jcache:jar:2.7.0:compile
|  +- com.github.ben-manes.caffeine:caffeine:jar:2.7.0:compile
|  |  +- org.checkerframework:checker-qual:jar:2.6.0:compile
|  |  \- com.google.errorprone:error_prone_annotations:jar:2.3.3:compile
|  +- com.typesafe:config:jar:1.3.3:compile
|  \- javax.inject:javax.inject:jar:1:compile

In my opinion, ErrorProne should only be required by the compiler during compile time, but should not be deployed as required dependency, unless the caffeine.jar has imports from ErrorProne (which I doubt).

The same might be true for the typesafe and checker-qual (seems like they only do some static code analysis).

The issue is, that you when you collect all dependencies (e.g. with maven-dependency-plugin:copy-dependencies), e.g. for bundling and deploying your application, these libraries are included and delivered to the customer, although they are probably never needed.

So, the preferred way is to explicitly exclude them?

@guidomedina
Copy link
Author

guidomedina commented Jul 1, 2019

Unfortunately for Guava and sometime now Caffeine this is how I have them both declared, nothing has gone wrong with the applications when declared like this, so for now just exclude them:

         <dependency>
            <groupId>com.github.ben-manes.caffeine</groupId>
            <artifactId>caffeine</artifactId>
            <version>2.7.0</version>
            <exclusions>
                <exclusion>
                    <groupId>org.checkerframework</groupId>
                    <artifactId>checker-qual</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.errorprone</groupId>
                    <artifactId>error_prone_annotations</artifactId>
                </exclusion>
            </exclusions>
        </dependency>
        <dependency>
            <groupId>com.google.guava</groupId>
            <artifactId>guava</artifactId>
            <version>28.0-jre</version>
            <exclusions>
                <exclusion>
                    <groupId>com.google.code.findbugs</groupId>
                    <artifactId>jsr305</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>org.checkerframework</groupId>
                    <artifactId>checker-qual</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.errorprone</groupId>
                    <artifactId>error_prone_annotations</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.j2objc</groupId>
                    <artifactId>j2objc-annotations</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>org.codehaus.mojo</groupId>
                    <artifactId>animal-sniffer-annotations</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.guava</groupId>
                    <artifactId>listenablefuture</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

@ben-manes
Copy link
Owner

checker-qual And errorprone are annotations which Java doesn’t require at runtime unless you reflectively load them when inspecting the methods. Unfortunately other languages will not classload without them, making them required. You can exclude safely in Java.

Typesafe is a configuration library that is required for JCache.

@vlsi
Copy link
Contributor

vlsi commented Jul 1, 2019

Unfortunately other languages will not classload without them, making them required

Can you please elaborate?

Java specification says, that annotation is basically ignored if it cannot be loaded at runtime.
In other words, classloading must not fail if annotation class is missing.

@ben-manes
Copy link
Owner

ben-manes commented Jul 1, 2019

The Scala compiler requires that all classes referenced in signatures
of classes that you use are also available on the compile time
classpath.

https://www.scala-lang.org/old/node/10575

There is an ancient scalac bug that I can’t find atm.

@ben-manes
Copy link
Owner

ben-manes commented Jul 1, 2019

The closest bug I could find is newer than the one I recall, but similar
scala/bug#7751

And it was still an issue as of 2016, according to this blog post.

I wouldn't be surprised if Kotlin has a similar issue since it is Scala inspired.

Regardless, it made sense to follow Guava's lead when switching away from jsr305 annotations, which were in the provided scope. They went back and forth and settled on this. The dependencies are small and easy to exclude. If you can convince Guava to change, I'll follow.

@vlsi
Copy link
Contributor

vlsi commented Oct 4, 2020

Just in case, here are the cases for Guava google/guava#2824 and JUnit5 junit-team/junit5#1105

Both projects settled on making annotations available on the compile classpath.
For instance, the annotations are required on the classpath when running with Error Prone compiler (it needs annotations).

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