Upgrade to Scala 2.13.8 #12506

realvictorprm · 2022-01-20T12:33:05Z

Not completely sure this is done correctly but lets see.

changelog_begin
changelog_end

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description
If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

ghost

LGTM.

ghost · 2022-01-20T16:14:39Z

nix/nixpkgs.nix

-      version = "2.13.6";
-      name = "scala-2.13.6";
+      version = "2.13.8";
+      name = "scala-2.13.8";
      src = pkgs.fetchurl {
        url = "https://www.scala-lang.org/files/archive/${name}.tgz";
        sha256 = "0hzd6pljc8z5fwins5a05rwpx2w7wmlb6gb8973c676i7i895ps9";


Oh, my mistake, you need to update this.

If you change it to all zeroes and then try and run scala -version, Nix will print out the correct value for you.

remyhaemmerle-da

LGTM

remyhaemmerle-da · 2022-01-24T12:35:29Z

daml-lf/interface/src/main/scala/com/digitalasset/daml/lf/iface/reader/InterfaceReader.scala

      name: QualifiedName,
      tyVars: ImmArraySeq[Ast.TypeVarName],
-      enum: Ast.DataEnum,
+      `enum`: Ast.DataEnum,


Please avoid using back-tick.

Suggested change

`enum`: Ast.DataEnum,

enumeration: Ast.DataEnum,

Thanks, will apply that across the board!

Please avoid using back-tick.

Can you also add an explanation to your request?

Can you also add an explanation to your request?

Here are some of my motivations :

Simplicity is almost always better. Why using this advance feature when a simple renaming makes the trick ?

It is more difficult to type.

it makes my life more complicate when I want to use regexp to search/refactor something

intelliJ does not handle them faultless.

I prefer reserving backticks for two following usages :

In pattern machine for matching constant.

to "hide" implicits.

daml-script/export/src/main/scala/com/daml/script/export/Encode.scala

realvictorprm · 2022-01-24T13:58:18Z

...r-api-bench-tool/src/main/scala/com/daml/ledger/api/benchtool/metrics/MetricsCollector.scala

@@ -46,6 +46,7 @@ class MetricsCollector[T](exposedMetrics: Option[ExposedMetrics[T]], clock: Cloc
  import MetricsCollector.Message._
  import MetricsCollector.Response._

+  @scala.annotation.nowarn("msg=.*is unchecked since it is eliminated by erasure")


I would like to fix it but I don't know yet how I can fix it correctly here without causing big cascades of changes.

cc @mziolekda @kamil-da

Sorry, I missed this one.
I'm fine with muting this warning. I'll probably need to revisit this piece of code.

ledger/ledger-api-common/BUILD.bazel

daml-script/export/src/test/scala/com/daml/script/export/ActionsFromTreesSpec.scala

daml-script/export/src/test/scala/com/daml/script/export/EncodeInstancesSpec.scala

ghost · 2022-01-26T13:21:41Z

language-support/scala/codegen-sample-app/BUILD.bazel

@@ -52,6 +52,7 @@ da_scala_library(
    scalacopts = [
        "-Wconf:cat=unused-imports&site=com\\.daml\\.sample\\..*:s",
        "-Wconf:msg=parameter value ev.. in method ContractIdNT (Value)|(LfEncodable) is never used:s",
+        "-Wconf:msg=Wrap `enum` in backticks to use it as an identifier:s",


Do you still need this?

Yes because the generated scala code contains enum as variable name 😓

^ @stefanobaghino-da you might want to fix that in the scala codegen.

If Stefano agrees to it I can take care of that too. However I wanted to keep the scope of this PR as much limited as possible so I would do it in a different PR.

^ @stefanobaghino-da you might want to fix that in the scala codegen.

I'll add a ticket to track this, I don't think we should be working on this right now.

ghost · 2022-01-26T13:22:34Z

ledger/ledger-api-common/BUILD.bazel

@@ -21,6 +21,9 @@ da_scala_library(
        "@maven//:io_spray_spray_json",
        "@maven//:org_scalaz_scalaz_core",
    ],
+    scalacopts = [
+        "-Wconf:msg=class DeduplicationTime in object DeduplicationPeriod is deprecated:s",  # deprecated field that needs to be tested


Can't we nowarn this closer to the actual reference?

Also, prefer cat=deprecation.

That tweet seems wise. The author is clearly a brilliant individual.

cocreature

Great work! Only request from my side is to rename export to scriptExport instead of exporting.

cocreature · 2022-01-26T16:02:45Z

daml-script/export/src/main/scala/com/daml/script/export/Encode.scala

@@ -21,35 +21,40 @@ import spray.json._

 private[export] object Encode {

-  def encodeArgs(export: Export): JsObject = {
+  def encodeArgs(exporting: Export): JsObject = {


Can we call it scriptExport instead? Seems a bit clearer.

many thanks for the suggestion! Will apply that in an instant.

cocreature · 2022-01-26T16:03:34Z

language-support/scala/codegen-sample-app/BUILD.bazel

@@ -52,6 +52,7 @@ da_scala_library(
    scalacopts = [
        "-Wconf:cat=unused-imports&site=com\\.daml\\.sample\\..*:s",
        "-Wconf:msg=parameter value ev.. in method ContractIdNT (Value)|(LfEncodable) is never used:s",
+        "-Wconf:msg=Wrap `enum` in backticks to use it as an identifier:s",


^ @stefanobaghino-da you might want to fix that in the scala codegen.

cocreature · 2022-01-26T16:04:21Z

...r-api-bench-tool/src/main/scala/com/daml/ledger/api/benchtool/metrics/MetricsCollector.scala

@@ -46,6 +46,7 @@ class MetricsCollector[T](exposedMetrics: Option[ExposedMetrics[T]], clock: Cloc
  import MetricsCollector.Message._
  import MetricsCollector.Response._

+  @scala.annotation.nowarn("msg=.*is unchecked since it is eliminated by erasure")


cc @mziolekda @kamil-da

stefanobaghino-da · 2022-01-31T09:42:41Z

This PR has been open for 11 days now. Please either merge it or close it.

realvictorprm · 2022-01-31T10:10:04Z

This PR has been open for 11 days now. Please either merge it or close it.

already on my list, need to do a last rebase and then the CI should pass.

realvictorprm · 2022-01-31T17:02:46Z

we're slowly getting there, only Linux is left! Hopefully it succeeds this time! 😅

ghost · 2022-02-02T14:44:45Z

...ger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/CommandsValidator.scala

  /** We validate only using current time because we set the currentTime as submitTime so no need to check both
    */
+  @scala.annotation.nowarn(


Import nowarn, please.

I don't think it's necessary, especially if it's used only once. But as mentioned there's no need to to include scala in the fully qualified name.

ghost · 2022-02-02T15:59:44Z

I think the code is fixed now; please merge/rebase main.

changelog_begin changelog_end

stefanobaghino-da · 2022-02-03T09:18:40Z

...ger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/CommandsValidator.scala

  /** We validate only using current time because we set the currentTime as submitTime so no need to check both
    */
+  @nowarn(
+    "msg=class DeduplicationTime in object DeduplicationPeriod is deprecated"


Please use cat=deprecation&origin=^com\.daml\.ledger\.api\.v1\.commands\.Commands\.DeduplicationPeriod\.DeduplicationTime$ in the future.

Now I finally understand how that one works, thanks!

realvictorprm · 2022-02-03T09:32:42Z

Yaaay finally merged, took long enough lol

realvictorprm requested a review from cocreature January 20, 2022 12:33

realvictorprm requested a review from garyverhaegen-da as a code owner January 20, 2022 12:33

ghost approved these changes Jan 20, 2022

View reviewed changes

ghost reviewed Jan 20, 2022

View reviewed changes

realvictorprm force-pushed the upgradeScala branch from d246e58 to a6e59d0 Compare January 24, 2022 11:29

realvictorprm requested review from remyhaemmerle-da, S11001001 and stefanobaghino-da as code owners January 24, 2022 12:16

remyhaemmerle-da approved these changes Jan 24, 2022

View reviewed changes

realvictorprm requested review from a team as code owners January 24, 2022 13:52

realvictorprm commented Jan 24, 2022

View reviewed changes

daml-script/export/src/main/scala/com/daml/script/export/Encode.scala Outdated Show resolved Hide resolved

realvictorprm commented Jan 24, 2022

View reviewed changes

ledger/ledger-api-common/BUILD.bazel Outdated Show resolved Hide resolved

realvictorprm force-pushed the upgradeScala branch from 7abefc6 to 7748ae6 Compare January 26, 2022 12:26

ghost approved these changes Jan 26, 2022

View reviewed changes

cocreature approved these changes Jan 26, 2022

View reviewed changes

realvictorprm enabled auto-merge (squash) January 26, 2022 17:01

realvictorprm force-pushed the upgradeScala branch from 61e3995 to abb2ca8 Compare January 31, 2022 10:51

realvictorprm force-pushed the upgradeScala branch from abb2ca8 to 6371430 Compare February 1, 2022 10:54

ghost reviewed Feb 2, 2022

View reviewed changes

realvictorprm added 5 commits February 2, 2022 18:21

Upgrade to Scala 2.13.8

705e648

changelog_begin changelog_end

Update hash for scala in nixpkgs

acfebeb

update more hashes for scala upgrade

bddb6d8

Fix most warnings etc.

b3675b9

Fix remaining errors etc.

8e3b206

realvictorprm added 9 commits February 2, 2022 18:21

Fix formatting

2ac7a4b

Resolve last errors hopefully

8d45354

Fix ledger api common build file

4a77c8d

Combine imports & revert accidental change that broke the CI run

9a9d807

Rename exporting vals to scriptExport & minimize diff

ec0e4ab

Remove more wrong changes

2826ab3

moved warning around

1f96c7d

Also repin the compat folder stuff

071bd10

Apply review comments

aa200e7

realvictorprm force-pushed the upgradeScala branch from 6371430 to aa200e7 Compare February 2, 2022 17:22

realvictorprm merged commit aa45f48 into main Feb 3, 2022

realvictorprm deleted the upgradeScala branch February 3, 2022 09:05

stefanobaghino-da reviewed Feb 3, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Scala 2.13.8 #12506

Upgrade to Scala 2.13.8 #12506

realvictorprm commented Jan 20, 2022

ghost left a comment

ghost Jan 20, 2022

remyhaemmerle-da left a comment

remyhaemmerle-da Jan 24, 2022

realvictorprm Jan 24, 2022

stefanobaghino-da Jan 28, 2022

remyhaemmerle-da Jan 28, 2022 •

edited

Loading

realvictorprm Jan 24, 2022

cocreature Jan 26, 2022

kamil-da Jan 31, 2022

ghost Jan 26, 2022

realvictorprm Jan 26, 2022

cocreature Jan 26, 2022

realvictorprm Jan 26, 2022

stefanobaghino-da Jan 28, 2022

stefanobaghino-da Jan 28, 2022

ghost Jan 26, 2022

ghost Jan 26, 2022

stefanobaghino-da Jan 28, 2022

cocreature left a comment

cocreature Jan 26, 2022

realvictorprm Jan 26, 2022

cocreature Jan 26, 2022

cocreature Jan 26, 2022

stefanobaghino-da commented Jan 31, 2022

realvictorprm commented Jan 31, 2022

realvictorprm commented Jan 31, 2022

ghost Feb 2, 2022

stefanobaghino-da Feb 3, 2022

ghost commented Feb 2, 2022

stefanobaghino-da Feb 3, 2022

realvictorprm Feb 3, 2022

realvictorprm commented Feb 3, 2022

Upgrade to Scala 2.13.8 #12506

Upgrade to Scala 2.13.8 #12506

Conversation

realvictorprm commented Jan 20, 2022

Pull Request Checklist

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyhaemmerle-da left a 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

remyhaemmerle-da Jan 28, 2022 • 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

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

Choose a reason for hiding this comment

cocreature left a 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

stefanobaghino-da commented Jan 31, 2022

realvictorprm commented Jan 31, 2022

realvictorprm commented Jan 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Feb 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

realvictorprm commented Feb 3, 2022

remyhaemmerle-da Jan 28, 2022 •

edited

Loading