[GraalJS] Add support for ES6+ to automation via GraalJS

[jpg0]

Signed-off-by: Jonathan Gilbert jpg@trillica.com

Bundle to add ES6+ support for automation via GraalJS

OSGi bundle to provide a GraalJS runtime for automation before Graal is bundled with the JDK (e.g. using Java 8 in this case). GraalJS is significantly more capable than Nashorn, which now deprecated.

This is a drop-in replacement for Nashorn, the only incompatibilities potentially being existing scripts which use Nashorn-specific features (which will need to be addressed at some point regardless). It does NOT replace the scripting engine for transformations (although this would be relatively straightforward to add).

Note that this bundle also enables CommonJS module support by default (although can be disabled), and therefore allows npm-installation of most of the existing available 3rd party JS libraries.

Discussion originally started here: https://community.openhab.org/t/graalvm-for-automation/84116/11

Binary available for testing.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/graalvm-for-automation/84116/13

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/fluent-js-api-for-rules/85524/1

[davidgraeff]

This is +83,314 lines of added code. The chance for this going through review is low to be honest.
I have checked and you are commiting all kind of javascript test files of the GraalVM project? If so, maybe reduce to one integration test. The How's and Why's of the engine is already tested in their own repo I guess.

If you have further ideas to cut down on lines please apply those. A regular binding has around 1.5k SLOC.

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[jpg0]

@davidgraeff you're completely right; apologies for that. The large amount of code was testing for specific edge-cases in loading 3rd party libraries' modules. I've removed it entirely as it's mostly tested elsewhere via mocks. Down to a much more manageable size.

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[kaikreuzer]

Thanks @jpg0, 2200 LoC look much better than before ;-)

I am wondering, whether this is the right repo for the PR, though. We don't have a add-on category for scripting engines and it probably does not make much sense to introduce such a category.

As you mention, this is rather a replacement for the Nashorn integration, so it should be considered a core component (maybe one that isn't installed by default due to its size, though).

@openhab/core-maintainers What's your view on this? Where should such code best live?

[jpg0]

I just realised that somehow an old factory file was left in there which broke classloading order. I've push a new commit to fix this (and also simplified logging).

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[cweitkamp]

What's your view on this? Where should such code best live?

IMO it depends on how much other scripting engines we expect to become part of OH 2 in the future. In general it is an extension for the automation engine. On the short run it would be be okay to move it to OHC but in the long run ... Does anything else speak against a new add-on category? Or is this another topic for OH 3 architecture?

[wborn]

What's your view on this? Where should such code best live?

All these scripting engines would be useful for OHC derivatives. But it will have a big impact on the download size if they are all included by default. I see that several of the Maven dependencies are 10MB or more.

[5iver]

But it will have a big impact on the download size if they are all included by default.

I plan to submit a similar PR for a Jython this weekend (:tada:). Once closer to OH3, I plan to open an issue to discuss the possibility of removing the rule engines, scripted automation, script engine factories, module types, rule templates, etc. from OHC and create a new repo for them. Due to this, my suggestion is to put everything automation related into OHC for now. This is where I've built the Jython script engine factory, although it also worked as an addon. Jython is ~40MB.

Once I have the PR in for Jython, I plan to review this PR, which I see several issues with.

[jpg0]

which I see several issues with

Are there any issues that I can begin to work on now?

[TravisBuddy]

Travis tests have failed

Hey @jpg0,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

2nd Build

Expand here

Single addon pull request: Building org.openhab.automation.module.script.graaljs
[WARNING] The POM for com.sun.xml.bind:jaxb-core:jar:2.2.11 is invalid, transitive dependencies (if any) will not be available, enable debug logging for more details
[WARNING] The POM for com.sun.xml.bind:jaxb-impl:jar:2.2.11 is invalid, transitive dependencies (if any) will not be available, enable debug logging for more details
[WARNING] Could not find file feature file features/karaf/openhab-addons/src/main/feature/feature.xml
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.sun.xml.bind.v2.runtime.reflect.opt.Injector (file:/home/travis/.m2/repository/com/sun/xml/bind/jaxb-impl/2.2.11/jaxb-impl-2.2.11.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int)
WARNING: Please consider reporting this to the maintainers of com.sun.xml.bind.v2.runtime.reflect.opt.Injector
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
[INFO] Empty report will not be appended to the summary report.
[WARNING] Manifest not present in the first entry of the zip - truffle-api-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - js-scriptengine-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - js-launcher-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - launcher-common-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - graal-sdk-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - regex-19.2.0.1.jar
[WARNING] Manifest not present in the first entry of the zip - js-19.2.0.1.jar
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/main/java/org/openhab/automation/module/script/graaljs/commonjs/internal/Module.java:[110,3302] Redundant null check: comparing '@NonNull Map<@NonNull String,@NonNull Value>' against null
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/main/java/org/openhab/automation/module/script/graaljs/commonjs/internal/Module.java:[110,3326] Dead code
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/main/java/org/openhab/automation/module/script/graaljs/commonjs/internal/Module.java:[118,3582] Redundant null check: The variable cachedExports cannot be null at this location
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/main/java/org/openhab/automation/module/script/graaljs/commonjs/internal/Module.java:[121,3702] Dead code
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/main/java/org/openhab/automation/module/script/graaljs/commonjs/internal/Module.java:[354,12437] Null type mismatch (type annotations): 'null' is not compatible to the free type variable 'T'
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/bnd.bnd [6:108]: Invalid property key: `osgi.serviceloader;`: <<    osgi.serviceloader;
      filter:="(osgi.serviceloader=org.graalvm.polyglot.impl.AbstractPolyglotImpl)";>>
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/bnd.bnd [6:108]: No value specified for key: osgi.serviceloader;. An empty value should be specified as 'osgi.serviceloader;:' or 'osgi.serviceloader;=': <<    osgi.serviceloader;
      filter:="(osgi.serviceloader=org.graalvm.polyglot.impl.AbstractPolyglotImpl)";>>
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/bnd.bnd [0:0]: Unused Export-Package instructions: [org.openhab.*] 
[WARNING] Can't extract module name from publisher-5.3.1.jar: Provider class org.glassfish.jersey.server.internal.RuntimeDelegateImpl not in module
[WARNING] Can't extract module name from geronimo-activation_1.1_spec-1.1.jar: geronimo.activation.1.1.spec: Invalid module name: '1' is not a Java identifier
[WARNING] Can't extract module name from xpp3_min-1.1.4c.jar: Provider class org.xmlpull.mxp1.MXParser,org.xmlpull.mxp1_serializer.MXSerializer not in module
[WARNING] Can't extract module name from geronimo-stax-api_1.0_spec-1.0.1.jar: geronimo.stax.api.1.0.spec: Invalid module name: '1' is not a Java identifier
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ResourceFolderTest.java:[22,606] The import org.junit.Assert.assertNotNull is never used
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ResourceFolderTest.java:[23,652] The import org.junit.Assert.assertNull is never used
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ModuleTest.java:[307,12024] ArrayList is a raw type. References to generic type ArrayList<E> should be parameterized
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ModuleTest.java:[328,13050] ArrayList is a raw type. References to generic type ArrayList<E> should be parameterized
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ModuleTest.java:[352,14261] ArrayList is a raw type. References to generic type ArrayList<E> should be parameterized
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ModuleTest.java:[376,15479] ArrayList is a raw type. References to generic type ArrayList<E> should be parameterized
[WARNING] /home/travis/build/openhab/openhab2-addons/bundles/org.openhab.automation.module.script.graaljs/src/test/java/org/openhab/automation/module/script/graaljs/commonjs/internal/ModuleTest.java:[399,16824] ArrayList is a raw type. References to generic type ArrayList<E> should be parameterized
[WARNING] Could not find file feature file features/karaf/openhab-addons/src/main/feature/feature.xml
     [java] JVM args ignored when same JVM is used.
     [java] Warnings generated: 2
[INFO] Individual report appended to summary report.
[DEBUG] /home/travis/build/openhab/openhab2-addons/target/old_Merge.xml  > report/create_html.xslt  null >  /home/travis/build/openhab/openhab2-addons/target/summary_report.html
[ERROR] Code Analysis Tool has found: 
 12 error(s)! 
 8 warning(s) 
 24 info(s)
[DEBUG] org.openhab.automation.module.script.graaljs.internal..java:[40]
Class contains more than one logger.
[DEBUG] .automation.module.script.graaljs/README.md:[0]
File does not end with a newline.
[WARNING] .automation.module.script.graaljs/README.md:[43]
The line before code formatting section must be empty.
[WARNING] .automation.module.script.graaljs/README.md:[48]
The line before code formatting section must be empty.
[WARNING] .automation.module.script.graaljs/README.md:[60]
The line before a Markdown list must be empty.
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[34]
File contains tab characters (this is the first instance).
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[55]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[91]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[154]
Remove empty line before closing brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[168]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[195]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[220]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[239]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[278]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.Module.java:[299]
Remove empty line after opening brace
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.package-info.java:[0]
File does not end with a newline.
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.package-info.java:[4]
Header line doesn't match pattern ^ \* See the NOTICE file\(s\) distributed with this work for additional$
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.Folder.java:[26]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.Paths.java:[24]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.Require.java:[28]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.ModuleCache.java:[27]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.AbstractFolder.java:[26]
File contains tab characters (this is the first instance).
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.AbstractFolder.java:[43]
Remove empty line after opening brace
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.RequireFunction.java:[29]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.FilesystemFolder.java:[30]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.commonjs.internal.ResourceFolder.java:[30]
File contains tab characters (this is the first instance).
[ERROR] org.openhab.automation.module.script.graaljs.internal.GraalJSScriptEngineFactory.java:[49]
File contains tab characters (this is the first instance).
[DEBUG] org.openhab.automation.module.script.graaljs.internal.GraalJSScriptEngineFactory.java:[73]
Remove empty line
[DEBUG] org.openhab.automation.module.script.graaljs.internal.GraalJSScriptEngineFactory.java:[163]
Remove empty line
[DEBUG] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[]
Logger should be non-static field. Change this field (logger) to non-static field.
[ERROR] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[42]
File contains tab characters (this is the first instance).
[DEBUG] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[42]
Name 'logger' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
[DEBUG] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[43]
Name 'stackLogger' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
[DEBUG] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[52]
Name 'EVAL_WITH_READER_METHOD' must match pattern '^[a-z][a-zA-Z0-9]*$'.
[DEBUG] org.openhab.automation.module.script.graaljs.internal.DebuggingGraalScriptEngine.java:[]
Logger should be non-static field. Change this field (stackLogger) to non-static field.
[WARNING] org.openhab.automation.module.script.graaljs.commonjs.internal.FilesystemFolderTest.java:[32]
Classes/Interfaces should be annotated with @NonNullByDefault
[WARNING] org.openhab.automation.module.script.graaljs.commonjs.internal.FilesystemFolderTest.java:[81]
Throwing 'Throwable' is not allowed.
[WARNING] org.openhab.automation.module.script.graaljs.commonjs.internal.AssertUtils.java:[27]
Classes/Interfaces should be annotated with @NonNullByDefault
[WARNING] org.openhab.automation.module.script.graaljs.commonjs.internal.ModuleTest.java:[42]
Classes/Interfaces should be annotated with @NonNullByDefault
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.ModuleTest.java:[562]
Remove empty line
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.ModuleTest.java:[575]
Remove empty line
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.ResourceFolderTest.java:[22]
Unused import - org.junit.Assert.assertNotNull.
[DEBUG] org.openhab.automation.module.script.graaljs.commonjs.internal.ResourceFolderTest.java:[23]
Unused import - org.junit.Assert.assertNull.
[WARNING] org.openhab.automation.module.script.graaljs.commonjs.internal.ResourceFolderTest.java:[34]
Classes/Interfaces should be annotated with @NonNullByDefault
[ERROR] Failed to execute goal org.openhab.tools.sat:sat-plugin:0.8.0:report (sat-all) on project org.openhab.automation.module.script.graaljs: 
[ERROR] Code Analysis Tool has found 12 error(s)! 
[ERROR] Please fix the errors and rerun the build.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :org.openhab.automation.module.script.graaljs

[jpg0]

@martinvw I formatted using the default Eclipse formatter, and it used tabs instead of spaces, which has failed the build. Is there a specific formatter documented or downloadable anywhere, or should I just switch tabs to (4x) spaces?

Edit: I have now reformatted as Eclipse w/o tabs.

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @jpg0,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[jpg0]

I have two features that I had added after opening this PR:

• Enforcing single-thread per Graal JS context for rule executions
• Allowing host preset imports via standard JS module system

Currently these are not included; I'm happy to keep going on this PR without them (I'll submit as subsequent ones), but if you'd prefer to include them, let me know.

[cpmeister]

2.5.x is our current working branch for openhab-addons. Please change your PR to merge to that branch instead.

[jpg0]

TBH I've assumed that this PR is dead as it's been open for 4 months. My code has moved on as well as OH, although I know that it's still compatible as I'm still actively using and maintaining it.

If this PR has a hope of actually being merged I'm happy to update, but if it's going nowhere then I can close it.

[kaikreuzer]

@jpg0 Definitely not dead or forgotten!
I'd hope that it can move forward as soon as we have a decision on openhab/openhab-core#1319

[kaikreuzer]

@cpmeister This PR is definitely for 3.0 and not 2.5, so having the PR against master is just fine in this case!

[jpg0]

@kaikreuzer I'll update this PR once that decision has been made, as it will affect this code too. There are quite a few improvements since this was originally submitted.

[kaikreuzer]

Thanks 👍

[5iver]

As I mentioned much much earlier, there are several issues that I see in this PR that I believe should be addressed before it is merged. The openhab-addons and openhab-core maintainers may feel differently.

• GraaljsScriptEngineFactory should extend AbstractScriptEngineFactory (this is now possible after openhab-core/#1251).
• Remove everything other than the ScriptEngineFactory and submit the other functionality in another bundle(s). I'll also be removing the helper libraries from the JythonScriptEngineFactory. I've put a lot of thought into this and it will be cleaner to have bundles for just the ScriptEngineFactories, but even more so, it will be better to have separate bundles for the "other stuff".
• This addon conflicts with org.openhab.core.automation.module.script.internal.NashornScriptEngineFactory, which will require changes in OHC to cleanup. In testing this bundle, things got messy when installing and uninstalling, and when transitioning to/from Nashorn. I know this is vague, but my testing was months ago 😄. In an attempt to resolve the issue at that time, I had split NashornScriptEngineFactory from OHC and created a separate addon for it.
• That's all I can remeber ATM. After openhab-core/#1319 is resolved, I plan to do a full review of this PR.

[jpg0]

Thanks for letting me know the issues that you found! I'll respond here whilst I've got a second...

1. I see no issue with using AbstractScriptEngineFactory, assuming that the existing functionality can be replicated
2. Happy to split things up; the only thing that I believe should remain with the actual script engine in the module loading. This is because it's so core to the language (e.g. without scripts suck!) and matches other languages which include it (such as Python).
3. I agree; there were some other issues with reloading too (some classes were leaking in to the system classloader). I did some work on this in a later commit (after this PR) and believe that I fixed this (as it would also break transforms, which continue to run on Nashorn).

Anyway, I'll look forward to the full review.

[kaikreuzer]

For your replacement PR, it would be great if you could go directly for the namespace suggested in openhab/openhab-core#1319 (comment).

[jpg0]

Sure!

[jpg0]

Superceded by #8441

I'll close this and work on that one; there have been more changes and the add-on is now more mature, but requires some changes in code it depends on.