Skip to content

Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target#10109

Merged
lrytz merged 1 commit intoscala:2.12.xfrom
lrytz:unlink-release-target
Aug 16, 2022
Merged

Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target#10109
lrytz merged 1 commit intoscala:2.12.xfrom
lrytz:unlink-release-target

Conversation

@lrytz
Copy link
Member

@lrytz lrytz commented Aug 15, 2022

Unlike Scala 2.13, Scala 2.12 cannot emit valid class files for target bytecode versions newer than 8. Thus, this PR deprecates -target on 2.12.

Regardless, you may still use -release to compile against a specific platform API version.

For more information on -release, see the Scala 2.13 PR #9982. This PR, taken together with #10089, effectively backports 9982 to 2.12, with the exception that on 2.12, bytecode version 8 is always emitted.

@scala-jenkins scala-jenkins added this to the 2.12.17 milestone Aug 15, 2022
@lrytz lrytz marked this pull request as ready for review August 15, 2022 09:33
@SethTisue SethTisue requested a review from som-snytt August 15, 2022 12:15
@som-snytt
Copy link
Contributor

Why isn't target tied to 8, with an explanatory message if they try to change it?

I almost wrote 'exoplanetary". That is for messages from aliens.

@lrytz lrytz force-pushed the unlink-release-target branch from 0ae79da to 9648941 Compare August 16, 2022 08:48
@lrytz
Copy link
Member Author

lrytz commented Aug 16, 2022

👍 good suggestion, I added that

Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

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

That .withPreSetHook(normalizeTarget) really shines when you just want to value.toInt.

} else if (setting.value.toInt < MinSupportedTargetVersion) {
setting.withDeprecationMessage(s"${setting.name}:${setting.value} is deprecated, forcing use of $DefaultTargetVersion")
setting.value = DefaultTargetVersion // triggers this hook
setting.value = DefaultTargetVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the comment falsified? I see L73 as why it doesn't loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, no, I think I just found it distracting...

.withAbbreviation("--target")
def targetValue: String = releaseValue.getOrElse(target.value)
// Unlike 2.13, don't use `releaseValue.getOrElse(target.value)`, because 2.12 doesn't have a fix for scala-dev#408
def targetValue: String = target.value
Copy link
Contributor

Choose a reason for hiding this comment

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

If they use just -release then they get default target 8. If they -release 8 -target 6 they will get the existing deprecation and target 8. If they are building on JDK 8, they can only use -release 8 or use -target. The changed behavior is -release 11 where they get target 8 as a limitation with no warning to fail a build. Only explicit -target 9 would warn.

@lrytz lrytz merged commit 5d7d7e8 into scala:2.12.x Aug 16, 2022
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Aug 16, 2022
@SethTisue SethTisue changed the title [nomerge] on 2.12, -release should not set -target [nomerge] Always emit Java 8 bytecode, regardless of -release; deprecate -target Aug 31, 2022
@SethTisue SethTisue changed the title [nomerge] Always emit Java 8 bytecode, regardless of -release; deprecate -target Always emit Java 8 bytecode, regardless of -release; deprecate -target Aug 31, 2022
@SethTisue SethTisue changed the title Always emit Java 8 bytecode, regardless of -release; deprecate -target Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target Aug 31, 2022
@SethTisue SethTisue changed the title Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target on 2.12 Aug 31, 2022
@SethTisue SethTisue changed the title Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target on 2.12 Always emit Java 8 bytecode on 2.12, regardless of -release; deprecate -target Aug 31, 2022
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Sep 17, 2022
### What changes were proposed in this pull request?
This PR aims to upgrade Scala to 2.12.17
- https://www.scala-lang.org/news/2.12.17

### Why are the changes needed?
The main [change](https://github.com/scala/scala/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A2.12.17+is%3Amerged+label%3Arelease-notes) fo this version as follows:

- scala/scala#10109
- scala/scala#10075
- scala/scala#10108
- scala/scala#10045
- scala/scala#10063
- scala/scala#10042
- scala/scala#10040
- scala/scala#10095

### Does this PR introduce _any_ user-facing change?
Yes, this is a Scala version change.

### How was this patch tested?
Existing Test

Closes #37892 from LuciferYang/SPARK-40436.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
LuciferYang added a commit to LuciferYang/spark that referenced this pull request Sep 20, 2022
### What changes were proposed in this pull request?
This PR aims to upgrade Scala to 2.12.17
- https://www.scala-lang.org/news/2.12.17

### Why are the changes needed?
The main [change](https://github.com/scala/scala/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A2.12.17+is%3Amerged+label%3Arelease-notes) fo this version as follows:

- scala/scala#10109
- scala/scala#10075
- scala/scala#10108
- scala/scala#10045
- scala/scala#10063
- scala/scala#10042
- scala/scala#10040
- scala/scala#10095

### Does this PR introduce _any_ user-facing change?
Yes, this is a Scala version change.

### How was this patch tested?
Existing Test

Closes apache#37892 from LuciferYang/SPARK-40436.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes worth highlighting in next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants