Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

=sbt Bump Scala version to 2.12.17 & 2.13.10, sbt to 1.7.2 #31648

Merged
merged 1 commit into from Oct 11, 2022
Merged

=sbt Bump Scala version to 2.12.17 & 2.13.10, sbt to 1.7.2 #31648

merged 1 commit into from Oct 11, 2022

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Oct 9, 2022 •
edited

The regression is fixed.
Draft release note of 2.13.10 : scala/scala-dev#820
Before: #31615

@He-Pin He-Pin force-pushed the scalaupdate branch 5 times, most recently from b7b4221 to 4294f24 Compare October 9, 2022 04:48
@He-Pin
Copy link
Member Author

He-Pin commented Oct 9, 2022 

[10-09 04:53:37.035] [error] /home/runner/work/akka/akka/akka-actor-tests/src/test/scala/akka/util/ByteIteratorSpec.scala:19:23: a type was inferred to be `AnyVal`; this may indicate a programming error.
[10-09 04:53:37.035] [error]       freshIterator().indexOf(0x20) should be(0)

and

[10-09 04:53:38.583] [error] /home/runner/work/akka/akka/akka-actor-tests/src/test/scala/akka/util/ByteStringSpec.scala:634:24: a type was inferred to be `AnyVal`; this may indicate a programming error.
[10-09 04:53:38.583] [error]       ByteString.empty.indexOf(5) should ===(-1)

@SethTisue cc.

@He-Pin He-Pin mentioned this pull request Oct 9, 2022
Closed
@som-snytt
Copy link

som-snytt commented Oct 9, 2022 •
edited

I commented on the linked ticket that

ByteIteratorSpec.this.convertToAnyShouldWrapper[Int](freshIterator().indexOf[AnyVal](32))((Position.apply("ByteIteratorSpec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 19): org.scalactic.source.Position), scalactic.this.Prettifier.default).should(ByteIteratorSpec.this.be.apply(0));

Previously, the warning was incorrectly dropped.

Maybe something like this will silence the warning only in Spec classes.

Compile / scalacOptions ++= Seq(raw"-Wconf:cat=lint-infer-any&site=.*Spec\..*:s"),

My next guess:

Compile / scalacOptions ++= Seq(raw"-Wconf:cat=lint-infer-any&src=akka-actor-tests/.*scala:s"),

I wasn't successful at suppressing, but I'm a mere student of -Wconf.

@He-Pin
Copy link
Member Author

He-Pin commented Oct 9, 2022

Thanks @som-snytt for the detail hint.

@He-Pin He-Pin marked this pull request as ready for review October 9, 2022 10:07
He-Pin
He-Pin commented Oct 10, 2022
View reviewed changes
project/AkkaDisciplinePlugin.scala
@@ -67,7 +67,7 @@ object AkkaDisciplinePlugin extends AutoPlugin {
"akka-stream-tests-tck",
"akka-testkit")

val defaultScalaOptions = "-Wconf:cat=unused-nowarn:s,any:e"
val defaultScalaOptions = "-Wconf:cat=unused-nowarn:s,cat=lint-infer-any:s,any:e"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to workaround the new linter.

He-Pin
He-Pin commented Oct 10, 2022
View reviewed changes
akka-actor/src/main/scala/akka/actor/dungeon/Children.scala
@@ -23,7 +24,7 @@ private[akka] trait Children { this: ActorCell =>

import ChildrenContainer._

@nowarn("msg=never used")
@nowarn("msg=never")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To work both in 2.12 and 2.13, or should this be just @nowarn?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to use explicit message in case something else comes up later. Shorter never is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's never updated now and in 2.12 it's never used.

@He-Pin He-Pin marked this pull request as draft October 11, 2022 05:18
@He-Pin
Copy link
Member Author

He-Pin commented Oct 11, 2022 •
edited 

[error] -target is deprecated: Use -release instead to compile against the correct platform API.

When compile with java 8

scala/bug#11814
#27079

deprecate -target :scala/scala#9982

@He-Pin
Copy link
Member Author

He-Pin commented Oct 11, 2022 •
edited

With -release:8 and java 11.
image

@He-Pin He-Pin marked this pull request as ready for review October 11, 2022 08:26
@He-Pin He-Pin marked this pull request as draft October 11, 2022 08:37
@He-Pin He-Pin marked this pull request as ready for review October 11, 2022 08:58
patriknw
patriknw reviewed Oct 11, 2022
View reviewed changes
akka-actor/src/main/scala/akka/actor/dungeon/Children.scala
@@ -23,7 +24,7 @@ private[akka] trait Children { this: ActorCell =>

import ChildrenContainer._

@nowarn("msg=never used")
@nowarn("msg=never")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to use explicit message in case something else comes up later. Shorter never is fine.

project/JdkOptions.scala
@@ -48,7 +48,9 @@ object JdkOptions extends AutoPlugin {
targetSystemJdk,
jdk8home,
fullJavaHomes,
Seq(if (scalaVersion.startsWith("3.")) "-Xtarget:8" else "-target:jvm-1.8"),
if (scalaVersion.startsWith("3.")) Seq("-Xtarget:8")
else if (scalaVersion.startsWith("2.13.")) Seq("-release", "8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where can I read about the reason for this change?
It's somewhat scary given the below comment "-release 8' is not enough, for some reason we need the 8 rt.jar"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In scala/scala#9982, I have compiled it with jdk 11 and checked the bytecode, is there anything I missed out?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. All good!

@patriknw patriknw added the 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding label Oct 11, 2022
patriknw
patriknw approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

johanandren
johanandren approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @He-Pin

@johanandren johanandren merged commit 3a35b10 into akka:main Oct 11, 2022
@johanandren johanandren added this to the 2.7.0-M4 milestone Oct 11, 2022
@He-Pin He-Pin mentioned this pull request Oct 13, 2022
Merged
@patriknw patriknw removed this from the 2.7.0-M4 milestone Oct 19, 2022
@patriknw patriknw added this to the 2.7.0 milestone Oct 19, 2022
@He-Pin He-Pin mentioned this pull request Mar 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patriknw patriknw patriknw approved these changes

@johanandren johanandren johanandren approved these changes

Assignees
No one assigned
Labels
2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding dependency-change
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@He-Pin @som-snytt @patriknw @johanandren