Restrict access to internal packages

[Marcono1234]

Resolves #94

Changes the module declarations to not export internal packages publicly anymore to prevent users from accidentally (or intentionally) relying on implementation details.

The module declarations now use qualified exports ... to ... to only export the packages to other sibling Endive modules which are permitted to use those packages.

Unfortunately this leads to the compiler warning "module not found" because the exports now contain a forward reference to another module which has not yet been compiled. For now this has been solved through @SuppressWarnings("module") but that unfortunately also suppresses other module-related warnings. It seems there is currently not better solution to this, see also https://stackoverflow.com/q/53670052.

[Marcono1234]

This is necessary, otherwise the tests (and probably also user code) fails with:

IllegalAccessError: class run.endive.$gen.CompiledMachine (in unnamed module @0x72e34f77) cannot access class run.endive.runtime.internal.CompilerInterpreterMachine (in module run.endive.runtime) because module run.endive.runtime does not export run.endive.runtime.internal to unnamed module @0x72e34f77

This cannot be solved using exports ... to because the generated code might be in an arbitrary user-defined module I assume (or even in the unnamed module, as seen here).

So using exports ... to + opens might be the only solution which prevents access at compile-time but allows access at runtime.

Unfortunately opens might give more permissions to reflection at runtime than needed (see JLS, "It also grants reflective access to all types in the package, and all their members, for code in other modules."). But it should be pretty clear to users that they are messing with Endive internals in case they have to rely on reflection.

[Marcono1234]

In case you are merging this, please let me know whether I should squash the commits first or if you are doing that on merge.

[andreaTP]

Hey @Marcono1234, thanks for putting this together and for caring about API boundaries!

After looking at this more carefully, I think the internal naming convention here serves more as a signal to API users ("this is not public API, don't rely on it") than as an actual JPMS encapsulation boundary. The vast majority of our users consume Endive on the classpath, where module-info declarations don't restrict access at all.

Adding exports ... to with forward references introduces @SuppressWarnings("module") (which silences all module warnings), and we still need opens run.endive.runtime.internal anyway since the AOT compiler generates code that references those classes at runtime, so the package isn't truly hidden even in JPMS mode.

I think the current approach of just having internal in the package name is the right level of protection for now, it clearly communicates intent without adding complexity. Does that make sense?

[Marcono1234]

The vast majority of our users consume Endive on the classpath, where module-info declarations don't restrict access at all.

Oh you are right. I had thought the module restrictions were still enforced in that case, but apparently I misremembered it.

The "internal" in the name certainly helps a bit, but it is quite possible that there will be users who accidentally or intentionally are going to ignore it.

But I can understand your concerns, so feel free to close this PR and the linked issue if you don't think this is worth it.

[andreaTP]

Thanks for the good exchange, feel free to re-open when appropriate!