Removed Guava, bump Gradle and all deps. #3

soberich · 2018-11-29T05:47:56Z

Hi,
We really want to remove Guava as it is on of the largest deps in war though required by this dependency (and json-patch). This would ease the choice to depend or not on Guava.
Please kindly review.
No "optimizations" made. Just a plain migration with practically coping and pasting logic from Guava's methods (Guava is a great library but we just have a JAX-RS web-app and Guava is a too big contributor to classpath).

Overview.

Removed Guava
Bump gradlew to 5.0 and adjust scripts for compatibility
Builds and tests run now on Java 6-11
Bump all versions to top available.

ar · 2019-03-12T23:48:15Z

There's also CVE-2018-10237 that hits 16.0.1.

If this PR is not accepted, would you consider using Guava 27.1-jre instead of 16.0.1 ?

while we wait on java-json-tools/jackson-coreutils#3

Capstan · 2019-08-30T07:47:18Z

I am starting to look at CVEs in this and its dependents. I want to sever the Gradle changes from the Guava changes, so I'll probably crib from this to get it building, commit, and then evaluate code changes separately. /cc @huggsboson

Capstan · 2019-08-30T13:34:27Z

Guava has been updated in pr #8 so the Guava removal can be considered on its own. /cc @ar

soberich · 2019-08-30T13:43:21Z

@Capstan Guys, wait. What is the reason to leave Guava? Again, let's remove it. It is not needed. I see this as simply much better approach. Why do half-action when we should go full. @Capstan Why making PR #8?

Capstan · 2019-08-30T14:02:55Z

Bumping the version was, IMO, the safest, quickest way to mitigate the CVE that @ar mentioned. It is not meant to represent a final decision on the "whether Guava" front.

Capstan · 2019-08-30T14:08:27Z

@soberich If you rebase and squash your change, we can take a look at it isolation. Thanks!

soberich · 2019-08-30T16:08:22Z

I will. Can we also backport this to fge coordinates afterwords? This woul be really really in demand I believe.

Capstan

Overall, this seems like a fairly reasonable change to reduce the weight of this library without making it unduly complex. I'd like to ensure that it's backwards compatible or that we're happy making another major version bump before we take it.

My knowledge of OSGi is near zilch, and I'm not sure if there are other implications I'm not considering.

/cc @huggsboson

Capstan · 2019-09-02T08:39:34Z

src/main/java/com/github/fge/jackson/JsonNumEquals.java

@@ -41,21 +41,50 @@
 * kind of equality.</p>
 */
 public final class JsonNumEquals
-    extends Equivalence<JsonNode>
 {


Removing this extension seems like a breaking change.

Why?
Do you believe anyone relies in it in hierarchy while working with JsonNumEquals?

I don't believe anything either way. I do know I cannot survey all users of the library. I do know it's part of the surface, and any user could have legitimately started relying on it. By semver conventions, that implies removing it is a breaking change and would necessitate a major version bump.

I'm also not averse to major version bumps for such changes, but I'm apparently an outlier, as others want the major version to be more associated with a collection of features than with API compat. As the new kid on this block, I'm going to defer to the more senior maintainers, if they express a preference.

Let me start looking at it. I will today/tomorrow.

No rush. Just to expectation set, as soon as I have permissions, I'm going to cut a 1.10 release with just the dependency version bumps. If this commit is accepted, it'll be in a following release.

@Capstan I am confused a bit. But I didn't remove that I shaded it. Which changes were you looking at? Basically it was all good and you consern already was resolved originally. I will squash it.

I am not sure what "shaded" means in this context. Removing the extends Equivalence<JsonNode> removes the Equivalents parentage, which is absolutely necessary if we're removing Guava references. The remaining question is do we bump the major version for this change or not.

I also probably need to make sure that at least the dependent projects in the java-json-tools don't rely on this. Not sure exactly best way to do that without using snapshot releases.

Out of curiosity, what other java-json-tools modules do you want to de-Guava to re: issue #11 ? What gets pulled into your JAX-RS app today?

I ask because while this removal is totally feasible, if you're including other libraries for which it is less feasible, this might not be worth finishing.

Ah, now I'm seeing your re-add of Equivalence. And now I understand "shading" to mean "shadowing", and no, I'm not excited to fork parts of Guava into this project. Either we should remove references to it outright, or keep the Guava references, but having this will cause code collisions for the people who include this and Guava.

Capstan · 2019-09-02T08:41:34Z

src/main/java/com/github/fge/jackson/jsonpointer/JsonPointerException.java

@@ -22,6 +22,8 @@
 public final class JsonPointerException
    extends Exception
 {
+    static final long serialVersionUID = -1835450876546372005L;


Out of curiosity, where did this come from? Does this allow JsonPointerExceptions serialized in the old system to be deserialized in the new system?

It was long ago, but I think it was just a quick refactor for Sonar satisfaction.

soberich · 2019-10-12T13:56:11Z

@Capstan
I have rebased.
I have seen this #11, but I don't see Guava now in a dependency tree of my fork (this PR). Maybe I didn't get what you mean there.
Anyway now we have a green bar here to proceed.
Thanks.

soberich · 2019-10-18T07:40:12Z

@Capstan ping.

Capstan

Overall looks good. I have a couple outstanding questions around Equivalence and change scope beyond this library.

Capstan

Please remove the shadow copy of Guava's Equivalence (i.e., drop commit 9700917).

Capstan · 2019-10-18T12:10:49Z

src/main/java/com/github/fge/jackson/JsonNumEquals.java

@@ -41,21 +41,50 @@
 * kind of equality.</p>
 */
 public final class JsonNumEquals
-    extends Equivalence<JsonNode>
 {


Ah, now I'm seeing your re-add of Equivalence. And now I understand "shading" to mean "shadowing", and no, I'm not excited to fork parts of Guava into this project. Either we should remove references to it outright, or keep the Guava references, but having this will cause code collisions for the people who include this and Guava.

soberich · 2019-10-18T17:18:34Z

All true. Well, the request was waiting for review and feedback, and as now we are there, we may get to a key question -
Are going to bump the (major) version or not?Because if yes, I will simply properly shadow those classes by prefixing "com.google..." package name with library name-space.
Initially PR was "for discussion" in my mind on this matter, as if maybe we would want to somehow keep the version.
@Capstan about your previous comment. If we will bump the version I will change package. This should be a solution. Right?

soberich · 2019-11-02T16:33:17Z

@Capstan this should work now. I prefixed as posted in previous commit.
I can squash all after you review if needed.

Capstan · 2019-11-05T08:48:33Z

I screwed up CI. Give me a second.

Capstan · 2019-11-05T08:52:27Z

Ah, no, CI should be fixed as of commit ce6f3a8. Sync and try again?

soberich · 2019-12-07T20:31:52Z

@Capstan WDYT?

soberich · 2019-12-07T20:40:37Z

@Capstan I rebased it multiple times, so now you probably proceed with some javadoc linting and javadoc fails. Can we finish the work and merge. You can make minor changes youself such as javadoc, I it is hard to follow those minors and rebasing everytime. The PR intention is to remove Guava which is done. Let's do it then. Again feel free to make any adjustments.

soberich · 2019-12-19T21:16:55Z

@Capstan just a ping

Capstan · 2020-01-06T15:19:32Z

I committed most of this in #42.

soberich mentioned this pull request Nov 29, 2018

Removed Guava (left in tests), bump Gradle and all deps. (this is 2nd part of https://github.com/java-json-tools/jackson-coreutils/pull/3) java-json-tools/json-patch#47

Closed

ar added a commit to jpos/jPOS-EE that referenced this pull request Mar 13, 2019

bump up Guava version

8816904

while we wait on java-json-tools/jackson-coreutils#3

Capstan mentioned this pull request Aug 30, 2019

Update to Gradle 5.6.1. #5

Merged

Capstan referenced this pull request Aug 30, 2019

Bump Guava to 28.1-jre.

61f98e1

Capstan reviewed Sep 2, 2019

View reviewed changes

Capstan mentioned this pull request Sep 5, 2019

Consider removal of Guava from java-json-tools repos #11

Open

soberich force-pushed the master branch from 5812632 to 9700917 Compare October 12, 2019 13:49

Capstan reviewed Oct 18, 2019

View reviewed changes

Capstan suggested changes Oct 18, 2019

View reviewed changes

Capstan mentioned this pull request Oct 30, 2019

Replace OSGi plugin with biz.aQute.bnd. #14

Merged

soberich force-pushed the master branch from 9700917 to ab12e68 Compare November 2, 2019 16:29

soberich requested a review from Capstan November 2, 2019 16:30

soberich force-pushed the master branch 3 times, most recently from b857462 to dbe3357 Compare November 2, 2019 16:50

Remove Guava. Build and tests now run on Java 6-11.

f36641a

soberich force-pushed the master branch from f916ce4 to f36641a Compare November 20, 2019 08:14

Merge branch 'master' into master

ee61209

Capstan closed this Jan 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed Guava, bump Gradle and all deps. #3

Removed Guava, bump Gradle and all deps. #3

soberich commented Nov 29, 2018

ar commented Mar 12, 2019

Capstan commented Aug 30, 2019

Capstan commented Aug 30, 2019 •

edited

Loading

soberich commented Aug 30, 2019 •

edited

Loading

Capstan commented Aug 30, 2019 •

edited

Loading

Capstan commented Aug 30, 2019

soberich commented Aug 30, 2019 •

edited

Loading

Capstan left a comment •

edited

Loading

Capstan Sep 2, 2019

soberich Sep 2, 2019

Capstan Sep 2, 2019

soberich Sep 3, 2019

Capstan Sep 3, 2019

soberich Sep 3, 2019 •

edited

Loading

Capstan Oct 18, 2019 •

edited

Loading

Capstan Oct 18, 2019

Capstan Oct 18, 2019

Capstan Oct 18, 2019

Capstan Sep 2, 2019

soberich Sep 2, 2019

soberich commented Oct 12, 2019

soberich commented Oct 18, 2019

Capstan left a comment

Capstan left a comment

Capstan Oct 18, 2019

soberich commented Oct 18, 2019

soberich commented Nov 2, 2019

Capstan commented Nov 5, 2019

Capstan commented Nov 5, 2019

soberich commented Dec 7, 2019

soberich commented Dec 7, 2019 •

edited

Loading

soberich commented Dec 19, 2019

Capstan commented Jan 6, 2020

Removed Guava, bump Gradle and all deps. #3

Removed Guava, bump Gradle and all deps. #3

Conversation

soberich commented Nov 29, 2018

ar commented Mar 12, 2019

Capstan commented Aug 30, 2019

Capstan commented Aug 30, 2019 • edited Loading

soberich commented Aug 30, 2019 • edited Loading

Capstan commented Aug 30, 2019 • edited Loading

Capstan commented Aug 30, 2019

soberich commented Aug 30, 2019 • edited Loading

Capstan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soberich Sep 3, 2019 • edited Loading

Choose a reason for hiding this comment

Capstan Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soberich commented Oct 12, 2019

soberich commented Oct 18, 2019

Capstan left a comment

Choose a reason for hiding this comment

Capstan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soberich commented Oct 18, 2019

soberich commented Nov 2, 2019

Capstan commented Nov 5, 2019

Capstan commented Nov 5, 2019

soberich commented Dec 7, 2019

soberich commented Dec 7, 2019 • edited Loading

soberich commented Dec 19, 2019

Capstan commented Jan 6, 2020

Capstan commented Aug 30, 2019 •

edited

Loading

soberich commented Aug 30, 2019 •

edited

Loading

Capstan commented Aug 30, 2019 •

edited

Loading

soberich commented Aug 30, 2019 •

edited

Loading

Capstan left a comment •

edited

Loading

soberich Sep 3, 2019 •

edited

Loading

Capstan Oct 18, 2019 •

edited

Loading

soberich commented Dec 7, 2019 •

edited

Loading