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

Respect user target always #10356

Merged
merged 2 commits into from
Mar 30, 2023
Merged

Conversation

som-snytt
Copy link
Contributor

Reverts the commit cc60058 in which I seem to have changed my mind whether to respect explicit --target in the presence of explicit --release.

The possibly dubious use case is to work around scala/bug#12761.

Note that if you're going to ignore my compiler option, you should at least tell me so. The deprecation is ambiguous.

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

@scala-jenkins scala-jenkins added this to the 2.13.12 milestone Mar 27, 2023
@som-snytt
Copy link
Contributor Author

"How could the unit test fail?"

[error] Test scala.tools.nsc.settings.TargetTest.respect user target failed: java.lang.AssertionError: Error output: '11' is not a valid choice for '-release', took 0.002 sec

I forgot to break out the old JDK 8, I think it's still in a box on a shelf in the closet.

@som-snytt som-snytt force-pushed the issue/12761-release-v-target branch from 6d21ff0 to c44b8c6 Compare March 29, 2023 02:59
@SethTisue SethTisue modified the milestones: 2.13.12, 2.13.11 Mar 29, 2023
@SethTisue SethTisue added the prio:hi high priority (used only by core team, only near release time) label Mar 29, 2023
@som-snytt
Copy link
Contributor Author

Maybe consider contributing lately and eightly to testkit.

@som-snytt som-snytt marked this pull request as ready for review March 29, 2023 06:20
@som-snytt
Copy link
Contributor Author

The settings cannot warn (!) as they only have an error function, but it could warn in the backend if user tried to do this:

$ skalac -d /tmp --release:11 --target:8 t7977.scala
warning: target platform version 8 is older than the release version 11
warning: 1 deprecation; re-run with -deprecation for details
2 warnings

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Thanks, this makes sense 👍

@lrytz lrytz merged commit 1ddce7e into scala:2.13.x Mar 30, 2023
@SethTisue SethTisue removed the prio:hi high priority (used only by core team, only near release time) label Mar 30, 2023
@som-snytt som-snytt deleted the issue/12761-release-v-target branch March 30, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants