Update scrooge-generator, scrooge-core, util-core and util-logging to version 20.9.0

[cattibrie]

Description and Motivation

Changes related to issue #1131

Updated scrooge-generator, scrooge-core, util-core and util-logging to version 20.9.0.
Also, updated their dependencies to recommended versions in maven repository.

For testing run the script test_version.sh.
All passed except test of scala version 2.12. Some of the thrift related tests didn't pass.
Would be glad for help with fixing test.

[liucijus]

I think your upgrades are correct, but there are tests which depend on some binary jars which are incompatible with this 20.9.0 version.

@ittaiz any idea how are these jars generated, where is the source? https://github.com/bazelbuild/rules_scala/tree/master/test/src/main/scala/scalarules/test/twitter_scrooge/thrift

[cattibrie]

I think your upgrades are correct, but there are tests which depend on some binary jars which are incompatible with this 20.9.0 version.

@liucijus Thank you
This is also was my thought.
I couldn't find info on how the jars in tests were generated.

[ittaiz]

I have no idea.
I found the PR that introduced them by digging through history and such.
It sounds like these jars are needed and the question is how can they be compatible to both.
Maybe the need is to extract them, change the sources and repackage them.
If so then it's probably valuable to add in the sources without a scrooge target just for next time.
It would also help understand why the sources needed the change.

[cattibrie]

@ittaiz thank you

It looks like in the target test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3:thrift3_import thrift3_scrooge.jar is a jar that was generated and compiled from Thrift3.thrift file.
I built a new jar with test/src/main/scala/scalarules/test/twitter_scrooge:scrooge3 and replaced the old one.

I looked at the content of the jar:
jar tf test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3/thrift3_scrooge.jar

The content of the thrift3_scrooge.jar that was in rules_scala before my changes:
https://gist.github.com/cattibrie/702c51e60577df71478eedc541e928c8

The content of the thrift3_scrooge.jar that was generated by running with scrooge version 18.6.0:
bazel build test/src/main/scala/scalarules/test/twitter_scrooge:scrooge3
https://gist.github.com/cattibrie/2c5331742aae30d52e8c35c9d55dc545

The content of the thrift3_scrooge.jar that was generated by running with scrooge version 20.9.0:
bazel build test/src/main/scala/scalarules/test/twitter_scrooge:scrooge3
https://gist.github.com/cattibrie/c086d3479e37e42d2c8e2f1738e37a5e

[liucijus]

Small nits:

• is ScalaTest update required with this Scrooge update? I guess it currently causes coverage tests to fail. In theory ScalaTest can impact other rules_scala users who do not use Scrooge, and I would go with such update with a separate PR
• why not version 20.10.0?
• it would be great to have an issue registered to improve situation with these binary jars committed. Ideally they should be consumed as outputs of source targets, or at least have a readme describing how to "fix" the tests for a next Scrooge update.

[cattibrie]

@liucijus thank you 🙂

• scrooge version 20.9.0 has a dependency of scalatest version 3.1.2. I will create a separate PR for scalatest update.
• when I started the PR the latest version was 20.9.0. Also, the changelog for 20.10.0 says there are no changes. Thus, I would prefer to update to version 20.9.0 for now.
• an issue created: Improve process of updating binary jars in twitter_scrooge tests #1141

[ittaiz]

Looks good. Really small comment inside.

[ittaiz]

Why is this needed?

[cattibrie]

With the newer version of scrooge-generator, a name clash occurs.
I got the error running bazel test test/src/main/scala/scalarules/test/twitter_scrooge/...:

ERROR: /Users/etyurina/workspace/rules_scala_fork/test/src/main/scala/scalarules/test/twitter_scrooge/thrift/BUILD:3:15: scala //test/src/main/scala/scalarules/test/twitter_scrooge/thrift:thrift failed (Exit 1)
bazel-out/darwin-fastbuild/bin/test/src/main/scala/scalarules/test/twitter_scrooge/thrift/tmp4946029676322863219/scalarules/test/twitter_scrooge/thrift/Union.scala:85: error: value Struct1 is not a member of org.apache.thrift.protocol.TStruct
          _result = Union.Struct1({
                          ^
one error found

The generated file is here.

[ittaiz]

Got you. Next time I think there's value in either adding a comment or better yet making the rename more explicit (maybe unionWhichIsntReservedWord or something).Thanks for the contribution!

[cattibrie]

Thank you @ittaiz
Will comment or give a better naming next time 🙂

Thank you for the help and review @ittaiz, @liucijus!

[ittaiz]

Thank you for your time and effort! We definitely need an active thrift scrooge champion around here :)