Switch to a multiproject/modular source layout

[carlosame]

As a step towards fully modularising Rhino (see https://github.com/mozilla/rhino/issues/1075#issuecomment-950907384), the source layout should be changed to a Gradle multiproject layout. I'd suggest adhering to the Maven/Gradle layout conventions that many projects follow, and the layout would then be something like:

org.mozilla.rhino/
                  src/
                      main/
                           java/
                                org/
                                    mozilla/
                           resources/
                                     org/
                                         mozilla/
                      test/
                           java/
                                org/
                                    mozilla/
                           resources/
                                     org/
                                         mozilla/
                      lc/
                           java/
                                com/
                                    netscape/
                                             javascript/
org.mozilla.rhino.engine/
                         src/
                             main/
                                  java/
                                       org/
                                           mozilla/
                                  resources/
                                            META-INF/
org.mozilla.rhino.runtime/
                          src/
                              main/
                                   java/
                                        org/
                                            mozilla/
                                   resources/
                                             org/
                                                 mozilla/

The "main" rhino jar would only contain the org.mozilla.javascript.tools package and would depend on the rhino-runtime jar, which is currently unusable in modular Java because it contains packages which are already present in the main rhino jar.

As to why I'm using full names like org.mozilla.rhino.runtime instead of just runtime or rhino-runtime: because the javadoc tool currently requires that convention, if one wants to produce a multi-module javadoc. BTW as an alternative one could use a smaller name, then rebuild the source tree elsewhere under buildGradle with the full names before building the Javadocs from that directory, it's just more complexity being added to the build.

The next step would be the modularisation itself, adding a module-info.java to each subproject (now module), also compiling classes that are specific to Java 11 and 18. There is more than one way to achieve that, and that could be discussed in #1075: one possibility would be to add two sourcesets:

org.mozilla.rhino.runtime/
                          src/
                              java11/
                                     java/
                                          org/
                                              mozilla/
                                     resources/
                                               org/
                                                   mozilla/
                              java18/
                                     java/
                                          org/
                                              mozilla/
                                     resources/
                                               org/
                                                   mozilla/

The Java 18 classes would not have any SecurityManager-related code (issue #1053), that being an approach alternative to #1068. However, one could wait until Java 19 or 20 to remove the SecurityManager stuff: the current Rhino code would run fine on Java 18, it is just that some permissions would not have any effect there.

Finally, two multi-release jar files would be produced:

• rhino-runtime jar with classes compiled for Java 8, 11 and 18.
• rhino jar with classes compiled for Java 8 and 18.

The other jar file (rhino-engine) does not need specific Java 11/18 classes AFAIK, other than the module-info.java.

[p-bakker]

Not being familiar at all with modular Java, I'm trying to understand what all this would mean.

Besides changes in the folder structure in the repo, it seems that the other part means NOT creating jars containing overlapping content. Hence your suggestion The "main" rhino jar would only contain the org.mozilla.javascript.tools package, correct?

Personally I wouldn't mind, as I'm a consumer of the rhino-runtime jar, so not affected. But I do see challenges for Shell users, as they would then have to get at least 2 jars and use the correct command line arguments to make things work.

Wondering if we should maybe also build a rhino-shell jar for just shell users that is a single jar containing everything (thus not adhering to modular Java principles): if you're using Rhino as a repl, I think Java modularity might not be an issue of concern?

As for The "main" rhino jar would only contain the org.mozilla.javascript.tools package: I'd rename this jar to rhino-tools or something like that in this setup

[carlosame]

Besides changes in the folder structure in the repo, it seems that the other part means NOT creating jars containing overlapping content. Hence your suggestion The "main" rhino jar would only contain the org.mozilla.javascript.tools package, correct?

Yes, otherwise users would run into the "split packages" problem, and the JVM would not even start. And that's one of the reasons why I believe that #1076 should be addressed in 1.7.14 (it is fixed by the first two commits in #1072).

Personally I wouldn't mind, as I'm a consumer of the rhino-runtime jar, so not affected. But I do see challenges for Shell users, as they would then have to get at least 2 jars and use the correct command line arguments to make things work.

Wondering if we should maybe also build a rhino-shell jar for just shell users that is a single jar containing everything (thus not adhering to modular Java principles): if you're using Rhino as a repl, I think Java modularity might not be an issue of concern?

Instead of building a rhino-shell jar, what would be of help is to build an executable using jlink. That's one of the things that could be done once the project is being compiled with Java 11+ (it is possible to build an executable using Java 8, but better use jlink if the project is going to compile with 11+).

As for The "main" rhino jar would only contain the org.mozilla.javascript.tools package: I'd rename this jar to rhino-tools or something like that in this setup

Yes the rhino-tools name fits better, but downstream projects using the shell would have to adapt. And if the current set of packages offered by rhino.jar would become rhino-tools.jar (plus the runtime dependency), one then could have a plain rhino jar without the tools, replacing the rhino-runtime that only a few people use?

I do not know enough about the Rhino shell users to assess whether this would be an inconvenient or not.

[p-bakker]

And that's one of the reasons why I believe that #1076 should be addressed in 1.7.14

Why does it need addressing in 1.7.14, instead of doing it together with switching over to a modular layout? Given that 1.7.14 is already in RC and close to being released as final

Instead of building a rhino-shell jar, what would be of help is to build an executable using jlink

I doubt that can work for the Shell, which allows dynamic class loading through importPackages. So we dont know which packages/modules/... to include

[carlosame]

Why does it need addressing in 1.7.14, instead of doing it together with switching over to a modular layout? Given that 1.7.14 is already in RC and close to being released as final

Because people seem to expect or assume that 1.7.14 is ready for modular Java, and the first two commits of that PR do not alter the API (other than moving a class to a new location and using a new package for customizations).

I looked at downstream open source projects that use Rhino, found one of them where they put an add-opens that could have been avoided with that PR applied. Admittedly it was only one project among quite a few.

That said, if a 1.7.15 was published with the fix in a reasonable timeframe, that would be fine as well (what scares me is a 1.7.15 being released in late 2022 or 2023).

I doubt that can work for the Shell, which allows dynamic class loading through importPackages. So we dont know which packages/modules/... to include

An example of how to build an executable with the specified dependencies could be offered in the documentation.

[tonygermano]

I usually just run java -jar rhino.jar to start up the shell. I'm not against having instructions for building a custom bin with jlink, but I don't think we should lose the ability to run the jar with that simple command. Sometimes it's necessary to add more jars to the classpath, and then you need to specify the shell Main class, but I wouldn't want to make that a requirement even if you only need the basic shell. In most cases, I'd rather add an add-opens than rebuild the program, and I'm sure that's true of most end users.

[carlosame]

I usually just run java -jar rhino.jar to start up the shell.

In that case, the binary executable that ./gradlew build created would be even more compact.

I don't think we should lose the ability to run the jar with that simple command. Sometimes it's necessary to add more jars to the classpath, and then you need to specify the shell Main class, but I wouldn't want to make that a requirement even if you only need the basic shell.

Then you should be fine with the simple basic binary.

In most cases, I'd rather add an add-opens than rebuild the program, and I'm sure that's true of most end users.

No add-opens would help here (that's for another issue, the point "2" below). If both rhino.jar and rhino-runtime.jar appear in someone's modulepath, they'll be unable to start the JVM. And while there are people who are fine with Java 8 (notably Android people), almost everyone else in the server JVM world seems to be trying to migrate to Java 11 at least.

Oracle is ending its premier support for Java 8 in March 2022, and a few widely-used libraries —like Jetty— are migrating to Java 11 (triggering an upgrade of dependent projects). I'd still not call it a "stampede" but the transition is ongoing.

I'll summarize the pending modularity issues (perhaps I should open a meta-issue with the following):

1) The two kinds of issues mentioned in #1076: split packages in VMBridge customizations, proxy object instantiation. Fixed by the first two commits in #1072.

2) Protected fields and methods (like 95% of the cases are fixed by #1072, once that PR is applied only the accesses to protected fields would still require an add-opens, as explained in the README).

3) "Split packages" problem with rhino.jar and rhino-runtime.jar. That's the reason why I did not add a module name to rhino-runtime.jar. The problem hasn't been observed out there because almost nobody uses rhino-runtime.jar, but if this project keeps telling people that "you should use rhino-runtime.jar" they will be hit (impact: unable to start the JVM). It depends on the current issue to be fixed.

4) org.mozilla.javascript.engine.RhinoScriptEngineFactory is not being advertised via module-info. Depends on the current issue to be fixed.

[tonygermano]

What happens when someone wants to call a java library from rhino shell that uses modules not included with the binary produced by jlink? (I admit my knowledge of modules and jlink is limited.)

[carlosame]

What happens when someone wants to call a java library from rhino shell that uses modules not included with the binary produced by jlink? (I admit my knowledge of modules and jlink is limited.)

One possibility that could be studied would be to pass a project property to the Gradle build, that then could use jlink/jpackage to build a new executable with the new modules. Like ./gradlew -pshellDeps="foo". But that could be a mess if you have to rebuild the executable often.

To me, it looks simpler to just run java with the additional jar(s) in the modulepath. After all, that would be almost the same that you do now when you want to add more jars, the difference being that now you would have to add the new rhino-runtime.jar to the classpath or modulepath (in addition to the new rhino.jar or rhino-tools.jar which would contain only the tools).

What's important about this issue is to agree on the need of the new source layout. Once that is cleared out, at the time of discussing the PR we could see which approach is more convenient about executing the shell. And this includes the possibility of having a module-free rhino-all.jar uberjar which has all the packages, like I do in EchoSVG, that one takes only about 17 lines of Gradle.

[p-bakker]

Closing as done today through the merge of #1479

[carlosame]

PR #1479 breaks backwards compatibility with the automatic module name that this project has been providing since 1.7.14. Instead, it could have followed what I had been proposing about a full modularisation since 2021, see for example https://github.com/mozilla/rhino/issues/870#issue-863048072:

so instead of the automatic name a full JPMS modularization could be performed. That would imply to split the code in a way that two or three modules (with their own module-info files) can be produced, e.g. org.mozilla.rhino.runtime, org.mozilla.rhino.engine and org.mozilla.rhino

Or the modules proposed in the present issue, which are backwards-compatible.

I mean, the two previous releases of Rhino provided an org.mozilla.rhino module which implicitly exported all packages, but this full modularisation does not provide any module with that name. This means that any project that required the org.mozilla.rhino module is now broken. Not good at all.

Please do either of:

a) Rename module org.mozilla.rhino.tools as org.mozilla.rhino. b) Rename org.mozilla.rhino.runtime as org.mozilla.rhino.

Due to security considerations (avoid providing a shell with the module org.mozilla.rhino which is what 99% of the people shall use), my preference would be for b), although a) gives full backwards compatibility.

[gbrail]

Thanks for digging in to this! Can you please look at this new issue that I created and add your perspective? Thanks!

https://github.com/mozilla/rhino/issues/1488

On Wed, Jun 12, 2024 at 4:34 AM carlosame @.***> wrote:

PR #1479 https://github.com/mozilla/rhino/pull/1479 breaks backwards compatibility with the automatic module name that this project has been providing since 1.7.14. Instead, it could have followed what I had been proposing about a full modularisation since 2021, see for example #870 (comment) https://github.com/mozilla/rhino/issues/870#issue-863048072:

so instead of the automatic name a full JPMS modularization https://www.oracle.com/corporate/features/understanding-java-9-modules.html could be performed. That would imply to split the code in a way that two or three modules (with their own module-info files) can be produced, e.g. org.mozilla.rhino.runtime, org.mozilla.rhino.engine and org.mozilla.rhino

Or the modules proposed in the present issue, which are backwards-compatible.

I mean, the two previous releases of Rhino provided an org.mozilla.rhino module which implicitly exported all packages, but this full modularisation does not provide any module with that name. This means that any project that required the org.mozilla.rhino module is now broken. Not good at all.

Please do either of:

a) Rename module org.mozilla.rhino.tools as org.mozilla.rhino. b) Rename org.mozilla.rhino.runtime as org.mozilla.rhino.

Due to security considerations (avoid providing a shell with the module org.mozilla.rhino which is what 99% of the people shall use), my preference would be for b), although a) gives full backwards compatibility.

— Reply to this email directly, view it on GitHub https://github.com/mozilla/rhino/issues/1092#issuecomment-2162782461, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD7I25EZBK3PDLQXJQD753ZHAW2RAVCNFSM6AAAAABJCDFNCGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG44DENBWGE . You are receiving this because you are subscribed to this thread.Message ID: @.***>