Unnecessary-looking dependencies #2824

OrangeDog · 2017-05-23T12:33:04Z

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

The text was updated successfully, but these errors were encountered:

cpovirk · 2017-05-23T14:57:15Z

The goal here was to fix #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 · 2017-05-23T15:12:34Z

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 · 2017-05-23T15:18:16Z

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 · 2017-05-23T15:25:30Z

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

efenderbosch · 2017-05-23T18:22:19Z

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

ghost · 2017-05-24T03:57:44Z

I believe #2721 (0e29934) 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 · 2017-05-25T16:40:47Z

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 · 2017-05-25T16:48:34Z

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 · 2017-05-25T16:56:14Z

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 · 2017-05-25T18:30:44Z

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 · 2017-08-01T13:51:46Z

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 · 2017-08-01T14:06:45Z

/cc @cgdecker re. 23.0-rc1 concern

cgdecker · 2017-08-02T20:20:26Z

@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 · 2018-01-15T17:18:04Z

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 · 2018-01-15T17:57:39Z

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

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

guidomedina · 2018-01-15T19:48:33Z

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

cpovirk · 2018-01-16T19:04:33Z

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

See google/guava#2824

…guava#2824

I was going to say that this also paves the way for including the annotation as a non-optional dependency, should we wish to follow our Guava precedent for annotations: - google/guava#2824 - google/guava#2721 But I see that it's retention=SOURCE anyway, so there isn't much reason to do that -- except maybe consistency with other annotation packages someday. (Maybe it's still a negative then, as it might still let people rely on our transitive dependency?) I think the relationship of all this to Java 11 was that I might have to set an Automatic-Module-Name on AutoService, and it makes more sense to set it after we've done the processor-vs.-annotation artifact split. Once I was upgrading, it made sense to set up the annotation processor the Right Away, now that we're using a version in which that works. (Or maybe it always worked but now it's nice that it gets the processor off the classpath?) Or maybe there was some other reason for the change to the annotation-processor setup; once again, I forget. It looks like it might have been that AutoService stops running when I switch how we run Error Prone. Hopefully this was the solution :) But it's probably a good idea in any case. This CL is basically following the "alternatively" instructions in https://github.com/google/auto/blob/master/value/userguide/index.md#in-pomxml ...even though the AutoService instructions haven't been similarly updated yet: https://github.com/google/auto/tree/master/service#download ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=272720556

davidljuba15031979 · 2019-10-19T12:59:26Z

[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 · 2019-10-19T16:52:27Z

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

talios · 2019-10-21T01:06:07Z

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

guidomedina · 2019-10-21T07:05:53Z

@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 · 2019-10-21T15:00:10Z

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 · 2019-10-22T09:34:39Z

There is a very old comment regarding splitting and moving the annotations classes to guava-annotations #2824 (comment)

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 · 2019-10-22T15:23:09Z

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 switch to an internal annotation for ignoring JRE requirements #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 · 2019-10-22T15:34:09Z

@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 · 2019-10-23T01:06:57Z

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 · 2019-10-23T07:48:41Z

@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 · 2022-04-04T04:07:23Z

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

To be conservative, this commit does not removes most annotation artifacts from Gradle's runtime classpath, only j2objc-annotations. The other artifacts contain at least some annotations with RUNTIME visibility (IIRC). (Even this change could theoretically affect users who assume that they can read CLASS-retention annotations (of which j2objc-annotations has some) from bytecode and find them in the runtime classpath. But that seems unlikely, especially for j2objc annotations.) We may consider being more aggressive in the future. For now, this particular commit addresses #2824 for Gradle users just a tiny bit, and it helps with the problem that prompted #6567. Fixes #3683 RELNOTES=Added [Gradle Module Metadata](https://docs.gradle.org/current/userguide/publishing_gradle_module_metadata.html). If you use Gradle 6 or higher, Gradle can automatically intelligently resolve conflicts between `guava-android` and `guava-jre`, among [other benefits](#3683). PiperOrigin-RevId: 337348979

To be conservative, this commit does not removes most annotation artifacts from Gradle's runtime classpath, only j2objc-annotations. The other artifacts contain at least some annotations with RUNTIME visibility (IIRC). (Even this change could theoretically affect users who assume that they can read CLASS-retention annotations (of which j2objc-annotations has some) from bytecode and find them in the runtime classpath. But that seems unlikely, especially for j2objc annotations.) We may consider being more aggressive in the future. For now, this particular commit addresses #2824 for Gradle users just a tiny bit, and it helps with the problem that prompted #6567. Fixes #3683 RELNOTES=Added [Gradle Module Metadata](https://docs.gradle.org/current/userguide/publishing_gradle_module_metadata.html). If you use Gradle 6 or higher, Gradle can automatically intelligently resolve conflicts between `guava-android` and `guava-jre`, among [other benefits](#3683). PiperOrigin-RevId: 544108700

jjohannes · 2023-06-29T12:03:20Z

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.

cpovirk closed this as completed May 25, 2017

This was referenced May 30, 2017

Non optional compile dependencies in Guava 22.0 #2831

Closed

Can't compile with Guava 20 and 21 when using annotation processor #2837

Closed

JakeWharton mentioned this issue Jun 20, 2017

Guava 22.0 introduced new runtime dependencies #2855

Closed

jbduncan mentioned this issue Aug 31, 2017

Guava's Maven dependencies are suddenly extensive #2932

Closed

vruusmann added a commit to jpmml/jpmml-evaluator that referenced this issue Feb 18, 2018

Updated Guava dependency

c967aa6

See google/guava#2824

cpovirk mentioned this issue Mar 27, 2018

Moe Sync google/auto#606

Merged

andrus mentioned this issue Apr 1, 2018

See if we can exclude Guava subdependencies bootique/bootique#213

Closed

jsichi mentioned this issue Apr 3, 2018

Guava adapter jgrapht/jgrapht#532

Merged

loverdos mentioned this issue Apr 11, 2018

[CGKIELE-107] mallet input-output-hk/mantis#436

Merged

fdlk added a commit to fdlk/molgenis that referenced this issue May 16, 2018

Exclude compile scope annotation dependencies from guava. See google/…

bba54be

…guava#2824

fdlk mentioned this issue May 16, 2018

Exclude compile scope annotation dependencies from guava molgenis/molgenis#7362

Merged

8 tasks

fdlk added a commit to fdlk/molgenis that referenced this issue May 17, 2018

Exclude compile scope annotation dependencies from guava. See google/…

7f82161

…guava#2824

cpovirk mentioned this issue Jun 24, 2019

Auto-value dependency drags dep into truth's downstream google/truth#590

Open

cpovirk mentioned this issue Oct 2, 2019

Mark autovalue-annotations optional (Maven) or compileOnly (gradle) google/auto#765

Closed

cpovirk mentioned this issue Oct 4, 2019

Moe Sync google/jimfs#95

Merged

jjohannes mentioned this issue Nov 6, 2019

Publish Gradle Module Metadata with Variants #3683

Closed

OrangeDog mentioned this issue Nov 12, 2019

Get rid of guava querydsl/querydsl#2324

Closed

electrum mentioned this issue Jul 4, 2020

Exclude guava's annotations dependencies airlift/airbase#230

Closed

This was referenced Oct 4, 2020

Caffeine 2.7.0 extra dependencies ben-manes/caffeine#300

Closed

Recommend declaring annotations as api autonomousapps/dependency-analysis-gradle-plugin#296

Closed

cpovirk mentioned this issue Jan 25, 2021

Declaring that one annotation aliases or specializes another (@Implies) [working decision: no] jspecify/jspecify#35

Closed

aakoshh mentioned this issue Feb 5, 2021

[PM-2752] Throttle incoming connections from the same ip input-output-hk/scalanet#126

Merged

overheadhunter mentioned this issue Apr 4, 2022

Ensure 1.0 does not preclude supporting both retention levels well later jspecify/jspecify#234

Closed

copybara-service bot mentioned this issue Jun 28, 2023

Publish Gradle Module Metadata with Variants. #6603

Closed

This was referenced Jun 29, 2023

Write release notes for Gradle Module Metadata change #6604

Closed

Remove annotation-only Jars from runtime classpath (in Gradle) #6606

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unnecessary-looking dependencies #2824

Unnecessary-looking dependencies #2824

OrangeDog commented May 23, 2017

cpovirk commented May 23, 2017

OrangeDog commented May 23, 2017 •

edited

cpovirk commented May 23, 2017

OrangeDog commented May 23, 2017

efenderbosch commented May 23, 2017

ghost commented May 24, 2017

cpovirk commented May 25, 2017

cpovirk commented May 25, 2017

OrangeDog commented May 25, 2017

cpovirk commented May 25, 2017

guidomedina commented Aug 1, 2017 •

edited

kluever commented Aug 1, 2017

cgdecker commented Aug 2, 2017

guai commented Jan 15, 2018

jbduncan commented Jan 15, 2018 •

edited

guidomedina commented Jan 15, 2018 •

edited

cpovirk commented Jan 16, 2018

davidljuba15031979 commented Oct 19, 2019 •

edited

guidomedina commented Oct 19, 2019 •

edited

talios commented Oct 21, 2019

guidomedina commented Oct 21, 2019 •

edited

milosonator commented Oct 21, 2019

guidomedina commented Oct 22, 2019 •

edited

cpovirk commented Oct 22, 2019

guidomedina commented Oct 22, 2019 •

edited

cpovirk commented Oct 23, 2019

guidomedina commented Oct 23, 2019 •

edited

overheadhunter commented Apr 4, 2022 •

edited

jjohannes commented Jun 29, 2023

Unnecessary-looking dependencies #2824

Unnecessary-looking dependencies #2824

Comments

OrangeDog commented May 23, 2017

cpovirk commented May 23, 2017

OrangeDog commented May 23, 2017 • edited

cpovirk commented May 23, 2017

OrangeDog commented May 23, 2017

efenderbosch commented May 23, 2017

ghost commented May 24, 2017

cpovirk commented May 25, 2017

cpovirk commented May 25, 2017

OrangeDog commented May 25, 2017

cpovirk commented May 25, 2017

guidomedina commented Aug 1, 2017 • edited

kluever commented Aug 1, 2017

cgdecker commented Aug 2, 2017

guai commented Jan 15, 2018

jbduncan commented Jan 15, 2018 • edited

guidomedina commented Jan 15, 2018 • edited

cpovirk commented Jan 16, 2018

davidljuba15031979 commented Oct 19, 2019 • edited

guidomedina commented Oct 19, 2019 • edited

talios commented Oct 21, 2019

guidomedina commented Oct 21, 2019 • edited

milosonator commented Oct 21, 2019

guidomedina commented Oct 22, 2019 • edited

cpovirk commented Oct 22, 2019

guidomedina commented Oct 22, 2019 • edited

cpovirk commented Oct 23, 2019

guidomedina commented Oct 23, 2019 • edited

overheadhunter commented Apr 4, 2022 • edited

jjohannes commented Jun 29, 2023

OrangeDog commented May 23, 2017 •

edited

guidomedina commented Aug 1, 2017 •

edited

jbduncan commented Jan 15, 2018 •

edited

guidomedina commented Jan 15, 2018 •

edited

davidljuba15031979 commented Oct 19, 2019 •

edited

guidomedina commented Oct 19, 2019 •

edited

guidomedina commented Oct 21, 2019 •

edited

guidomedina commented Oct 22, 2019 •

edited

guidomedina commented Oct 22, 2019 •

edited

guidomedina commented Oct 23, 2019 •

edited

overheadhunter commented Apr 4, 2022 •

edited