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.

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

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

Merged
merged 1 commit into from
Aug 16, 2022

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.

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 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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