Skip to content

Inconsistent behavior of -release (JEP 247) #13015

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

Open
szeiger opened this issue Jul 4, 2024 · 2 comments
Open

Inconsistent behavior of -release (JEP 247) #13015

szeiger opened this issue Jul 4, 2024 · 2 comments
Labels
Milestone

Comments

@szeiger
Copy link

szeiger commented Jul 4, 2024

Given these equivalent Scala and Java sources, and compilers running on JDK 17:

$ cat Main.scala
import jdk.jfr.internal.Repository
class Main {}

$ cat Main.java
import jdk.jfr.internal.Repository;
public class Main {}

$ java -version
java version "17.0.11" 2024-04-16 LTS
Java(TM) SE Runtime Environment Oracle GraalVM 17.0.11+7.1 (build 17.0.11+7-LTS-jvmci-23.0-b34)
Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 17.0.11+7.1 (build 17.0.11+7-LTS-jvmci-23.0-b34, mixed mode, sharing)

javac does not allow the use of the the internal API, with or without an explicit --release 17 because it always limits access based on modules:

$ javac  Main.java
Main.java:1: error: package jdk.jfr.internal is not visible
import jdk.jfr.internal.Repository;
              ^
  (package jdk.jfr.internal is declared in module jdk.jfr, which does not export it)
1 error

$ javac --release 17  Main.java
Main.java:1: error: package jdk.jfr.internal is not visible
import jdk.jfr.internal.Repository;
              ^
  (package jdk.jfr.internal is declared in module jdk.jfr, which does not export it)
1 error

Without --release the module can be made accessible with --add-exports but this is rejected when used with --release (whether the requested version matches the current JDK version or not):

$ javac --add-exports=jdk.jfr/jdk.jfr.internal=ALL-UNNAMED Main.java

$ javac --release 17 --add-exports=jdk.jfr/jdk.jfr.internal=ALL-UNNAMED Main.java
error: exporting a package from system module jdk.jfr is not allowed with --release
1 error

$ javac --release 11 --add-exports=jdk.jfr/jdk.jfr.internal=ALL-UNNAMED Main.java
error: exporting a package from system module jdk.jfr is not allowed with --release
1 error

Instead of using --release it is possible to manually set both -source and -target version and override the system module path (the module equivalent of the bootclasspath) to get the same effect as --release while allowing the use of --add-exports:

$ javac -source 11 -target 11  --system /Users/stefan.zeiger/.sdkman/candidates/java/11-zulu-local Main.java --add-exports=jdk.jfr/jdk.jfr.internal=ALL-UNNAMED

scalac has several inconsistencies with this scheme:

  • By default scalac is not module-aware. It allows access to private APIs without warnings or errors:

    $ scalac Main.scala
    
  • Setting an explicit release version enforces module access but only if the version is lower than the current JDK version, otherwise it is treated the same as not using modules:

    $ scalac -release:17 Main.scala
    
    $ scalac -release:11 Main.scala
    Main.scala:1: error: object internal is not a member of package jdk.jfr
    import jdk.jfr.internal.Repository
                   ^
    1 error
    
  • There are no equivalents of --add-exports and --system and thus no way to target an older release without enforcing strict module access.

(This is about Scala 2; I tested it with 2.13.14 but AFAICT there are no changes in this area in any recent 2.13 or 2.12 version since the feature was added; I have not tried it with Scala 3)

@som-snytt
Copy link

The ticket for module support scala/scala-dev#529

@szeiger
Copy link
Author

szeiger commented Jul 4, 2024

Simply adding support for --system without touching any of the other stuff looks straight-forward.

$ /Users/stefan.zeiger/code/scala/build/quick/bin/scalac -target 11 --system /Users/stefan.zeiger/.sdkman/candidates/java/11-zulu-local  Main.scala

It still ignores module access but now you can compile against an older image version like with javac. Since javac does not support the combination of --add-exports and --release it is reasonable (and probably unavoidable) the scalac does the same, so if scalac ever gets full module support, module access will be enforced for the loaded image with or without --system and a new --add-exports option has to be added to allow exceptions.

(My prototype implementation closes the system image after each compilation which is probably not good for performance in long-running compile servers. I'll have to run some benchmarks and possibly add caching.)

szeiger added a commit to szeiger/scala that referenced this issue Jul 4, 2024
This PR adds a new `-system` / `--system` setting to scalac which mimics its counterpart in javac.

This fixes the biggest problem (at least for us) of scala/bug#13015. It is now possible to compile code against an older system image without enforcing strict module access. scalac generally does not enforce modules (scala/scala-dev#529) but it does when using `-release` (with class lookup based on `ct.sym`) and there was no way to opt out of these restrictions.

The usual opt-out in javac is `--add-exports` but it is not supported for system modules in combination with `--release` (https://bugs.openjdk.org/browse/JDK-8178152) so there is no expectation that scalac could support it. Instead the solution for javac is to replace `--release` with a combination of `-source`, `-target` and a system image for the target version via `--system`. This combination, unlike `--release`, can be used with `--add-exports`. If scalac adds full module support at a later time (with access restrictions enabled by default) it will also need to support `--add-exports` and allow its use in combination with `--system`.

I am also un-deprecating `-target` (which was deprecated in favor of `-release`) because it now has a legitimate use in combination with an alternative system image (just like in javac where it is serves the same purpose and is not deprecated, either).
szeiger added a commit to szeiger/scala that referenced this issue Jul 10, 2024
This PR adds a new `-system` / `--system` setting to scalac which mimics its counterpart in javac.

This fixes the biggest problem (at least for us) of scala/bug#13015. It is now possible to compile code against an older system image without enforcing strict module access. scalac generally does not enforce modules (scala/scala-dev#529) but it does when using `-release` (with class lookup based on `ct.sym`) and there was no way to opt out of these restrictions.

The usual opt-out in javac is `--add-exports` but it is not supported for system modules in combination with `--release` (https://bugs.openjdk.org/browse/JDK-8178152) so there is no expectation that scalac could support it. Instead the solution for javac is to replace `--release` with a combination of `-source`, `-target` and a system image for the target version via `--system`. This combination, unlike `--release`, can be used with `--add-exports`. If scalac adds full module support at a later time (with access restrictions enabled by default) it will also need to support `--add-exports` and allow its use in combination with `--system`.

I am also un-deprecating `-target` (which was deprecated in favor of `-release`) because it now has a legitimate use in combination with an alternative system image (just like in javac where it serves the same purpose and is not deprecated, either).
szeiger added a commit to databricks/scala that referenced this issue Jul 10, 2024
This PR adds a new `-system` / `--system` setting to scalac which mimics its counterpart in javac.

This fixes the biggest problem (at least for us) of scala/bug#13015. It is now possible to compile code against an older system image without enforcing strict module access. scalac generally does not enforce modules (scala/scala-dev#529) but it does when using `-release` (with class lookup based on `ct.sym`) and there was no way to opt out of these restrictions.

The usual opt-out in javac is `--add-exports` but it is not supported for system modules in combination with `--release` (https://bugs.openjdk.org/browse/JDK-8178152) so there is no expectation that scalac could support it. Instead the solution for javac is to replace `--release` with a combination of `-source`, `-target` and a system image for the target version via `--system`. This combination, unlike `--release`, can be used with `--add-exports`. If scalac adds full module support at a later time (with access restrictions enabled by default) it will also need to support `--add-exports` and allow its use in combination with `--system`.

I am also un-deprecating `-target` (which was deprecated in favor of `-release`) because it now has a legitimate use in combination with an alternative system image (just like in javac where it is serves the same purpose and is not deprecated, either).
szeiger added a commit to databricks/scala that referenced this issue Jul 10, 2024
This PR adds a new `-system` / `--system` setting to scalac which mimics its counterpart in javac.

This fixes the biggest problem (at least for us) of scala/bug#13015. It is now possible to compile code against an older system image without enforcing strict module access. scalac generally does not enforce modules (scala/scala-dev#529) but it does when using `-release` (with class lookup based on `ct.sym`) and there was no way to opt out of these restrictions.

The usual opt-out in javac is `--add-exports` but it is not supported for system modules in combination with `--release` (https://bugs.openjdk.org/browse/JDK-8178152) so there is no expectation that scalac could support it. Instead the solution for javac is to replace `--release` with a combination of `-source`, `-target` and a system image for the target version via `--system`. This combination, unlike `--release`, can be used with `--add-exports`. If scalac adds full module support at a later time (with access restrictions enabled by default) it will also need to support `--add-exports` and allow its use in combination with `--system`.

I am also un-deprecating `-target` (which was deprecated in favor of `-release`) because it now has a legitimate use in combination with an alternative system image (just like in javac where it is serves the same purpose and is not deprecated, either).
szeiger added a commit to databricks/scala that referenced this issue Jul 11, 2024
This PR adds a new `-system` / `--system` setting to scalac which mimics its counterpart in javac.

This fixes the biggest problem (at least for us) of scala/bug#13015. It is now possible to compile code against an older system image without enforcing strict module access. scalac generally does not enforce modules (scala/scala-dev#529) but it does when using `-release` (with class lookup based on `ct.sym`) and there was no way to opt out of these restrictions.

The usual opt-out in javac is `--add-exports` but it is not supported for system modules in combination with `--release` (https://bugs.openjdk.org/browse/JDK-8178152) so there is no expectation that scalac could support it. Instead the solution for javac is to replace `--release` with a combination of `-source`, `-target` and a system image for the target version via `--system`. This combination, unlike `--release`, can be used with `--add-exports`. If scalac adds full module support at a later time (with access restrictions enabled by default) it will also need to support `--add-exports` and allow its use in combination with `--system`.

I am also un-deprecating `-target` (which was deprecated in favor of `-release`) because it now has a legitimate use in combination with an alternative system image (just like in javac where it serves the same purpose and is not deprecated, either).

# Conflicts:
#	src/compiler/scala/tools/nsc/Global.scala
#	src/compiler/scala/tools/nsc/classpath/DirectoryClassPath.scala
#	src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala
@SethTisue SethTisue added the jdk11 label Nov 6, 2024
@SethTisue SethTisue added this to the Backlog milestone Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants