sky-uk / cqlmigrate

Cassandra schema migration library
BSD 3-Clause "New" or "Revised" License
47 stars 29 forks source link

Fully remove Guava dependency #75

Closed mkobit closed 5 years ago

mkobit commented 5 years ago

https://github.com/sky-uk/cqlmigrate/issues/13 looked to remove the dependency, but it still must be part of the transitives.

For example, at https://github.com/sky-uk/cqlmigrate/blob/1aa621e3d7dbb099759dd52a89b127f8a673edf7/src/main/java/uk/sky/cqlmigrate/CqlFileParser.java#L3-L4 still shows usage.

This means this library can't be used with Guava 26.0+ due to those static final CharMatchers being removed - see https://github.com/google/guava/releases/tag/v26.0

adamdougal commented 5 years ago

Unfortunately this is being pulled in by com.datastax.cassandra:cassandra-driver-core:3.1.0

./gradlew dependencies
Configuration on demand is an incubating feature.
:dependencies

------------------------------------------------------------
Root project
------------------------------------------------------------

archives - Configuration for archive artifacts.
No dependencies

compile - Compile classpath for source set 'main'.
+--- com.datastax.cassandra:cassandra-driver-core:3.1.0
|    +--- io.netty:netty-handler:4.0.37.Final
|    |    +--- io.netty:netty-buffer:4.0.37.Final
|    |    |    \--- io.netty:netty-common:4.0.37.Final
|    |    +--- io.netty:netty-transport:4.0.37.Final
|    |    |    \--- io.netty:netty-buffer:4.0.37.Final (*)
|    |    \--- io.netty:netty-codec:4.0.37.Final
|    |         \--- io.netty:netty-transport:4.0.37.Final (*)
|    +--- com.google.guava:guava:16.0.1
|    +--- io.dropwizard.metrics:metrics-core:3.1.2
|    |    \--- org.slf4j:slf4j-api:1.7.7 -> 1.7.25
|    +--- com.github.jnr:jnr-ffi:2.0.7
|    |    +--- com.github.jnr:jffi:1.2.10
|    |    +--- org.ow2.asm:asm:5.0.3
|    |    +--- org.ow2.asm:asm-commons:5.0.3
|    |    |    \--- org.ow2.asm:asm-tree:5.0.3
|    |    |         \--- org.ow2.asm:asm:5.0.3
|    |    +--- org.ow2.asm:asm-analysis:5.0.3
|    |    |    \--- org.ow2.asm:asm-tree:5.0.3 (*)
|    |    +--- org.ow2.asm:asm-tree:5.0.3 (*)
|    |    +--- org.ow2.asm:asm-util:5.0.3
|    |    |    \--- org.ow2.asm:asm-tree:5.0.3 (*)
|    |    \--- com.github.jnr:jnr-x86asm:1.0.2
|    \--- com.github.jnr:jnr-posix:3.0.27
|         +--- com.github.jnr:jnr-ffi:2.0.7 (*)
|         \--- com.github.jnr:jnr-constants:0.9.0
+--- org.slf4j:slf4j-api:1.7.25
\--- com.google.code.findbugs:jsr305:3.0.0

Even if we updated to the latest version it'd still be pulling in version 19.0 of Guava https://github.com/datastax/java-driver/blob/3.7.1/pom.xml#L53 so we won't be able to get rid of the dependency, just our direct usage of it.

jsravn commented 5 years ago

@adamdougal I think it is okay, as long as cqlmigrate doesn't depend on the removed static fields. So it should be updated to remove those usages.

I'm not sure if the datastax driver is using the deprecated fields, but it should be easy to check this.

mkobit commented 5 years ago

Ahh, good catch with the transitive dependency.

Maybe as an immediate fix, could swap to [CharMatcher.whitespace()](https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/CharMatcher.html#whitespace()) which exists in both?

Also, the Throwables.propagate is deprecated and can be easily swapped, too.

Would you accept a pull request that changes those pieces?

mkobit commented 5 years ago

I also think that would include bumping the driver version because the current driver seems to resolve to 18.0 which doesn't have the whitespace() method.

|    +--- com.google.guava:guava:16.0.1
|    +--- com.google.guava:guava:16.0.1
     |    +--- com.google.guava:guava:18.0
     |         +--- com.google.guava:guava:16.0 -> 18.0
     +--- com.google.guava:guava:18.0
|    +--- com.google.guava:guava:16.0.1
|    +--- com.google.guava:guava:16.0.1 -> 18.0
|    |    +--- com.google.guava:guava:17.0 -> 18.0
|    |    +--- com.google.guava:guava:17.0 -> 18.0
     |    +--- com.google.guava:guava:18.0
     |         +--- com.google.guava:guava:16.0 -> 18.0
     +--- com.google.guava:guava:18.0
     |    +--- com.google.guava:guava:18.0
     |         +--- com.google.guava:guava:16.0 -> 18.0
     +--- com.google.guava:guava:18.0
|    +--- com.google.guava:guava:16.0.1 -> 18.0
|    |    +--- com.google.guava:guava:17.0 -> 18.0
|    |    +--- com.google.guava:guava:17.0 -> 18.0
     |    +--- com.google.guava:guava:18.0
     |         +--- com.google.guava:guava:16.0 -> 18.0
     +--- com.google.guava:guava:18.0
jsravn commented 5 years ago

@mkobit yes PRs are welcome :). I no longer help maintain this project but I can poke the people who do.

mkobit commented 5 years ago

Also worth noting that the Cassandra 4.0 driver was recently released - https://github.com/datastax/java-driver/tree/4.x/upgrade_guide

The driver now requires Java 8 or above. It does not depend on Guava anymore (we still use it internally but it's shaded).

lw346 commented 5 years ago

The latest PRs (#76 and #77) were released as version 0.10.0.

lw346 commented 5 years ago

Having done a more comprehensive test, it looks like cqlmigrate itself is now usable with later versions of Guava on the classpath. I'm going to close this issue as fixed. Feel free to reopen if you feel otherwise.

mkobit commented 5 years ago

We just tested it in ours and things look good so far. Thanks!

steliosAnastasakis commented 4 years ago

We were also having issues with the CharMatcher.WHITESPACE in CqlFileParser::findStatement. We had guava:25.1-jre as dependency. By forcing the module to use guava:25.0-jre, we resolved the issue. CharMatcher.WHITESPACE is deprecated. We tried to switch this to CharMatcher.BREAKING_WHITESPACE, using guava:25.1-jre and it worked!

hope this helps.