Unnecessary-looking dependencies

[OrangeDog]

New in Release 22 are a bunch of Maven dependencies that look like they should be compile-only, but are now required at runtime.

[INFO] +- com.google.guava:guava:jar:22.0:compile
[INFO] |  +- com.google.code.findbugs:jsr305:jar:1.3.9:compile
[INFO] |  +- com.google.errorprone:error_prone_annotations:jar:2.0.18:compile
[INFO] |  +- com.google.j2objc:j2objc-annotations:jar:1.1:compile
[INFO] |  \- org.codehaus.mojo:animal-sniffer-annotations:jar:1.14:compile

[cpovirk]

The goal here was to fix https://github.com/google/guava/issues/2721. Evidently "compile-only" doesn't actually make the annotations available at compile time to projects that compile against Guava, and that causes problems for people who compile with all warnings as errors. But if making the dependencies required at runtime causes problems, too, we'll have to figure out what to do. Are they causing problems for you, or is it just surprising that we made this change?

[OrangeDog]

In case you didn't know "compile-only" is the provided scope. The compile scope is compile-time and later (i.e. including runtime and test).

Sounds like they should be either <optional>true</optional>, <scope>provided</scope>, or moved into a new "build tools" pom that people can depend on separately.

[cpovirk]

They used to be <optional>true</optional>, and that's what caused #2721. It's possible that <scope>provided</scope> would fare better; I don't know.

[OrangeDog]

I believe <scope>provided</scope> matches your intent, so is probably the correct solution.

[efenderbosch]

Any chance of a quick 22.1 release to resolve this? Otherwise we'll be adding a ton of exclusions for dependency convergence.

[ghost]

I believe #2721 (0e29934933f33379de953727171f7ca1ec616a58) is a reasonable change, but since it affects backward compatibility, I think it should be clarified at least in the release notes. Please be aware that changes to pom.xml do not appear in the Full JDiff Report.

[cpovirk]

I just tested with <scope>provided</scope>, and it reintroduces #2721 :(

I'll put something in the release notes. We should think more about whether dependency version bumps should show up in the release notes in general.

[cpovirk]

https://github.com/google/guava/wiki/Release22#additional-changes

That may be all we can do, but if anyone has other ideas, I'm all ears. Sorry for the trouble.

[OrangeDog]

Then my third suggestion still remains - move them to be dependencies of a new artifact. Then people who just want to use the library don't have to add exclusions, and anyone who wants to build against it avoiding #2721 adds the extra dependency.

<dependency>
  <groupId>com.google.guava<groupId>
  <artifactId>guava-annotations</artifactId>  <!-- first name I thought of -->
  <type>pom</type>
  <scope>provided</scope>
</dependency>

[cpovirk]

Ah, sorry, I'd missed that one. I think I feel better with the current version, which requires action from people who use dependency convergence (and who presumably understand the problem and how to fix it), than with the proposed alternative, which requires action from people who are turning on warnings or using annotation processors (who might see a variety of different errors and might not know how to debug them). I know that that's still a bad experience for you; it just seems like it has to be a bad experience for someone for all the reasons to diamond dependencies normally are a bad experience :(

[guidomedina]

Will Guava version 23.0 include that extra 4 dependencies? I can still see them been dragged on our Maven project by 23.0-rc1

[kluever]

/cc @cgdecker re. 23.0-rc1 concern

[cgdecker]

@cpovirk's conclusion above seems to have been that of the two options we have, both of which cause issues for some people in some way, we should stick with what we're currently doing. So 23.0 will continue to have those dependencies as non-optional.

[guai]

This is madness Just make them provided. Its how they should be declared If this declaration causes bugs then these bugs are somewhere else

[jbduncan]

The JUnit 5 team have gone and made their dependency on opentest4j (another annotations library) compile scope now, for reasons given in https://github.com/junit-team/junit5/issues/1105.

Maybe it goes to show that annotations aren't really as optional as we all might think? :thinking:

[guidomedina]

I'm dealing with it by using exclusion on its dependencies, ugly as hell but it works.

[cpovirk]

@jbduncan , thanks for the link to the JUnit 5 change. I mentioned it on the Maven feature request for a true compile-time scope.

[davidljuba15031979]

[INFO] |  |  +- com.google.guava:guava:jar:28.1-jre:compile
[INFO] |  |  |  +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO] |  |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO] |  |  |  +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO] |  |  |  +- org.checkerframework:checker-qual:jar:2.8.1:compile
[INFO] |  |  |  +- com.google.errorprone:error_prone_annotations:jar:2.3.2:compile
[INFO] |  |  |  +- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO] |  |  |  \- org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:compile

Copy-paste exclusions for future readers...

            <groupId>com.google.guava</groupId>
            <artifactId>guava</artifactId>
            <version>28.1-jre</version>
            <exclusions>
                <!--https://github.com/google/guava/issues/2824-->
                <exclusion>
                    <groupId>com.google.guava</groupId>
                    <artifactId>failureaccess</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>com.google.guava</groupId>
                    <artifactId>listenablefuture</artifactId>
                </exclusion>
                <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>
            </exclusions>

[guidomedina]

@davidljuba15031979 failureaccess is needed in half of the cases and in few cases you could need listenablefuture if you use that concurrent facility.

[talios]

@guidomedina maybe they should be <optional>true</optional> then?

[guidomedina]

@guidomedina maybe they should be <optional>true</optional> then?

I don't think they are going to change it, we will have to live with the long exclusion list.

[milosonator]

I am surprised this hasn't been dealt with properly at this time. We use Guava in a lot of our projects and have a bunch of custom build code just dealing with excluding all the compile-time stuff from our runtime and distributions. It's a huge hassle to deal with this and there are no other projects doing something like this. Really hope this will all be reverted.

[guidomedina]

There is a very old comment regarding splitting and moving the annotations classes to guava-annotations https://github.com/google/guava/issues/2824#issuecomment-304062556

Then probably making such optional or provided which is the one dragging most of the extra dependencies, please guys go back to that comment and re-read previous comments which makes me believe it would be a better solution to this problem.

Initially there were only 4 extra dependencies but now there are 6 already which has been becoming more and more of a hassle.

It would make the dependencies tree look like this (notice the com.google.guava:annotations I added to the original tree):

|  |  +- com.google.guava:guava:jar:28.1-jre:compile
|  |  |  +- com.google.guava:failureaccess:jar:1.0.1:compile
|  |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
|  |  |  +- com.google.guava:annotations:jar:1.0.0:compile
|  |  |  | +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
|  |  |  | +- org.checkerframework:checker-qual:jar:2.8.1:compile
|  |  |  | +- com.google.errorprone:error_prone_annotations:jar:2.3.2:compile
|  |  |  | +- com.google.j2objc:j2objc-annotations:jar:1.3:compile
|  |  |  | \- org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:compile

[cpovirk]

I see. The previous post had suggested that the guava-annotations artifact would be provided or optional (and maybe you're suggesting that, too)? I still believe that provided/optional is the wrong solution, since it's not a true compile-time scope (and Maven doesn't provide such a scope).

The idea of a non-provided, non-optional guava-annotations artifact is a little more appealing. It would (I think?) give users an easy way to exclude all the annotation artifacts. It does still have downsides:

• We'll have to find a name that makes clear that it doesn't contain com.google.common.annotations.
• I don't know if tools will be upset that Guava will then be relying on a transitive dependency for its annotations.
• People may depend on guava-annotations directly, at which point they might see conflicts with the version that Guava itself is using.

For that reason, I'm still not sure that a change is a net win.

In better news:

• In the medium term, we expect to replace jsr305 and checker-qual with a single artifact of nullness annotations.
• animal-sniffer-annotations was just removed in https://github.com/google/guava/pull/3652 (though we haven't made a release with that yet).
• I'd be happy to take a similar pull request that replaces the j2objc-annotations dependency with package-private custom @J2ObjCIncompatible annotations.

[guidomedina]

@cpovirk I would say it is fine as a normal dependency (guava-annotations), it will be very easy to exclude and can be re-worked/re-factored separately in the future for people looking to use it.

[cpovirk]

Still unsure, and keeping this on the back burner, but that's the best alternative I've heard so far. Call it "guava-optional-at-runtime-deps" or something.

Also, I was confused about j2objc: We use more annotations than just J2ObjCIncompatible. I had briefly hoped that we could continue to use the j2objc artifact but make it provided or optional, since J2ObjCIncompatible has source retention. But other annotations that we use from the artifact have class retention.

[guidomedina]

@cpovirk this is a well known used pattern by other projects, when an API starts getting cluttered by "non-related" things or cover several groups then each group is put into a smaller artifact specialized in doing a single group of things, annotations is a good example, for example the famous Jackson dependencies have:

• jackson-core
• jackson-databind
• jackson-annotations

This also applies to many other projects which do exactly this same split process; cluttering an API without clear division/delegation of responsibilities has a negative effect; I will tell you what I think before I decide to include Guava in my projects:

• Week 1: I only need one function that I can implement myself, won't drag it yet.
• Week 2: I only need two functions that I can implement myself, won't drag it yet.
• Week 3: I only need three functions that I can implement myself, won't drag it yet; still not worth dragging 2mb jar + a train of unrelated dependencies just for this.
• Week 4: I'm screwed, I have no other choice than to suffer the painful process of including Guava and excluding its train of dependencies.

Please don't take my thought process wrong; this is what the mind does and denying it would be hypocritical on my side, a good example of this would be to split Guava into:

• guava-core
• guava-collections
• guava-concurrent
• guava-annotations

Of course; in due time, guava-annotations can be a good start for this pattern and then slowly divide in between releases of major versions; another benefit in doing is that you won't be limited to add things to each group because such dependency will be added by people interested in such particular area, say; guava-graphs or guava-geometry, the possibilities are endless with this approach.

[overheadhunter]

I hope someone still pays attention to an issue that has been inactive for three years now. Otherwise I threaten you with opening a new one! 🏴‍☠️

Still unsure, and keeping this on the back burner, but that's the best alternative I've heard so far. Call it "guava-optional-at-runtime-deps" or something.

Despite simplifying dependency exclusion, this would also help with #2970, since guava-optional-at-runtime-deps could have an Automatic-Module-Name manifest entry, which Guava's module-info.java can "require" without the need to wait for all the (then transitive) annotation jars becoming module-compatible (they will just remain on the classpath - if not excluded 😉).

[jjohannes]

With #3683 integrated, the next Guava release will publish Gradle Metadata. This would make it possible to define the originally requested dependency setup (for Gradle users).

I created this Draft PR (#6606) for further discussion on the topic. Anyone interested in this, please give your feedback there (if only 👍 ). I think it would mostly be interesting to share reasons/cases why this should not be done.