Inline Guava #125
Comments
|
We can "shade" the guava dep, I think. That seems reasonable.
…On Fri, 23 Jun 2017 at 15:52 Florent Biville ***@***.***> wrote:
As you already probably know, Guava is notoriously known for causing
conflicts because of its aggressive pruning policy. And well, that's
exactly the blocker for me.
As I briefly mentioned in #124
<#124>, the last released
version of compile-testing pulls Guava 20.0 which is incompatible with
Neo4j (that pulls Guava 18.0).
Would you consider releasing an extra artefact that inlines Guava (or
removing Guava altogether but that's probably not that easy).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#125>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAUN4hOuQjVARlxA27YjSZt-9de_uBwCks5sHEHBgaJpZM4OEHo8>
.
|
|
When you use Guava, you agree to its lifecycle policy, which implies that you commit to follow Guava releases by no more than a 2-years lag and never use Guava 18 was published on 2014-08-25, which means that you cannot safely use it with libraries that have been compiled against Guava 20 (released 2016-10-28) or later. This is a constraint that you bought into. So either update Neo4j to a newer (less that 2 years old) Guava version, or stick with libraries and library versions that are guaranteed to work with Guava 18 (and this indeed applies transitively to all. Or if you disagree with Guava's policy, then update Neo4j to no longer use Guava (many projects blacklist Guava because of that policy). Or possibly shade Guava into Neo4j to free yourself from the policy constraints. But asking others to shade Guava or remove a dependency on Guava because you fail/refuse to follow the contract you (implicitly) bought into is just plain wrong (and to some extent disdainful/disrespectful). compile-testing is not the problem here. |
|
Thanks @tbroyer for the detailed explanation. I understand the rationale, and I personally avoid shipping Guava in libraries (or shade the few classes the lib relies on). In the context of Neo4j, I can at least raise the issue (I'm not a core contributor, just an occasional one working on one specific area). FYI, I just ran a little experiment on Truth 0.30, 0.31 and master. Then, I could list the Guava classes required by Truth. Truth core 0.30 depends on 15 classes, 0.31 on 16 and master on 17 (details below). While this may seem like a lot, shading these classes looks feasible (and the rate of addition seems limited, based on this small version sample). What do you think @cgruber? In any case, you call the shots, I'll raise an issue on upgrading Guava in Neo4j. If you ever decide to shade Guava, that'll push more work on your plate but also provide more options to your users (who love your work btw) :) Guava classes required by Truth core, per version0.30
0.31
Master (7d038d601a9cfab5828c440c5faef98c683b6929)
|
|
Guava types are in Truth's public API which prevents shading.
…On Sat, Jun 24, 2017, 5:36 AM Florent Biville ***@***.***> wrote:
Thanks @tbroyer <https://github.com/tbroyer> for the detailed explanation.
I understand the rationale, and I personally avoid shipping Guava in
libraries (or shade the few classes the lib relies on). In the context of
Neo4j, I can at least raise the issue (I'm not a core contributor, just an
occasional one working on one specific area).
FYI, I just ran a little experiment on Truth 0.30, 0.31 and master.
I removed Guava dependency altogether in order to how much Truth
production code is adhering to Guava.
@@ -14,11 +14,6 @@
</properties>
<dependencies>
<dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- <version>${guava.version}</version>
- </dependency>
- <dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>3.0.1</version>
@@ -88,6 +83,9 @@
<excludes>
<exclude>**/super/**/*.java</exclude>
</excludes>
+ <compilerArguments>
+ <Xmaxerrs>10000</Xmaxerrs>
+ </compilerArguments>
</configuration>
</plugin>
Then, I could list the Guava classes required by Truth.
$> mvn -fae -am -pl core package | grep -E 'symbol\:(\ )+class' | sed 's/\[ERROR\] //' | tr -s ' ' | cut -s -d' ' -f4 | sort | uniq
Truth core 0.30 depends on 15 classes, 0.31 on 16 and master on 17
(details below).
While this seems like a lot, shading these classes looks feasible (and the
rate of addition seems limited, based on this small version sample). What
do you think @cgruber <https://github.com/cgruber>?
In any case, you call the shots, I'll raise an issue on upgrading Guava in
Neo4j.
If you ever decide to shade Guava, that'll push more work on your plate
but also provide more options to your users (who love your work btw) :)
Guava classes required by Truth core, per version 0.30
- AtomicLongMap
- BiMap
- Function
- GwtIncompatible
- ImmutableBiMap
- ImmutableList
- ImmutableMultimap
- ImmutableSetMultimap
- ListMultimap
- Multimap
- Multiset
- Optional
- Range
- SetMultimap
- Table
0.31
- AtomicLongMap
- BiMap
- ForwardingSortedMap
- ForwardingSortedSet
- Function
- GwtIncompatible
- ImmutableBiMap
- ImmutableMultimap
- ImmutableSetMultimap
- ListMultimap
- Multimap
- Multiset
- Optional
- Range
- SetMultimap
- Table
Master (7d038d601a9cfab5828c440c5faef98c683b6929)
- AtomicLongMap
- BiMap
- Cell
- ForwardingSortedMap
- ForwardingSortedSet
- Function
- GwtIncompatible
- ImmutableBiMap
- ImmutableMultimap
- ImmutableSetMultimap
- ListMultimap
- Multimap
- Multiset
- Optional
- Range
- SetMultimap
- Table
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#125 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEZKNnIpCDSkbLHl9C5z2o9w6s_oFks5sHQKpgaJpZM4OEHo8>
.
|
|
Oops, I should probably have taken a look at that first 😅 |
(I had been thinking like we could wait for 1.0 for this, but that feels like hurting people in the meantime for no good reason. And we should really set a good example. Credit to google/compile-testing#125 for prompting me to do this now.) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=160149591
(I had been thinking like we could wait for 1.0 for this, but that feels like hurting people in the meantime for no good reason. And we should really set a good example. Credit to google/compile-testing#125 for prompting me to do this now.) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=160149591
|
Woops. I flaked on the public exposure of types in Truth. That would
make shading nearly worthless as we'd have to exclude so much, and I doubt
what we could shade is even used by CompileTesting.
I think that Neo4j will have to upgrade its dep.
…On Sun, 25 Jun 2017 at 05:59 Florent Biville ***@***.***> wrote:
Oops, I should probably have taken a look at that first 😅
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#125 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAUN4o57jv8pX_qESdlilC1lyqRUFCbzks5sHlmWgaJpZM4OEHo8>
.
|
|
I filed the issue on their side. Thanks for your patience and for this so useful lib!! |
As you already probably know, Guava is notoriously known for causing conflicts because of its aggressive pruning policy. And well, that's exactly the blocker for me.
As I briefly mentioned in #124, the last released version of compile-testing pulls Guava 20.0 which is incompatible with Neo4j (that pulls Guava 18.0).
Would you consider releasing an extra artefact that inlines Guava (or removing Guava altogether but that's probably not that easy).
The text was updated successfully, but these errors were encountered: