Adapt addons to core watch service changes

[J-N-K]

Depends on openhab/openhab-core#3004

[J-N-K]

@jpg0 @florian-h05 Can you have a look at the changes in JS Scripting?
@jimtng @ccutrer an you have a look at the changes in JRuby Scripting?

Unfortunately it's not so easy to test because it requires a full distribution build with the core changes.

[openhab-bot]

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

https://community.openhab.org/t/openhab-4-0-snapshot-discussion/142322/22

[florian-h05]

Having a glance (at GitHub web) at JS Scripting, the changes look reasonable and good to me, but I will try to have a look at this in IntelliJ tomorrow.
I guess since it is not easy to test this, we should merge if it looks good and then see what happens. We are in the early days of a new openHAB major release, IMO no one should expect the SNAPSHOTs to work as fine as usual.

[ccutrer]

Unfortunately it's not so easy to test because it requires a full distribution build with the core changes.

I see, cause of the change in the BOM's pom.xml.

Are there any docs on how to do a full dist build? I've run into this before when I was trying to move the Jinja dependency for Home Assistant. I failed, and have never gotten back to it (and I've kept myself plenty busy with other things).

[ccutrer]

JRuby portion looks fine to me, minus the minor typo. I'll try to figure out how to test it within the next few days.

[J-N-K]

@ccutrer I do a local openhab-core build, then an openhab-addons and openhab-webui build (only if changes may affect UI). The last step is an openhab-distro build which produces a .zip for the core installation and a .kar for add-ons. I have a modified update script which updates my the test installation on my dev machine from a file (instead of the remote repository).

[florian-h05]

@J-N-K Any chance you can provide the core ZIP to us?
AFAIK addons KAR shouldn’t be required, an addon jar should be enough and I can build the addon on my own.

[florian-h05]

Apart from one bug, the JS Scripting code looks good to me, thanks!
I trust you that you tested it, I won't test myself unless someone provides me the distro build (or just the core ZIP).

Please have a look at my comments.

[florian-h05]

Suggested change

	if (Files.isRegularFile(this.libraryPath)) {
	if (Files.isRegularFile(Path.of(this.LIB_PATH))) {

[J-N-K]

Debatable if it is more clear if we use libraryPath which is what is used by the WatchService itself or Path.of(LIB_PATH). Technically it is the same. The constructor calls super(watchService, LIB_PATH) and there libraryPath is assigned to Path.of(LIB_PATH).

[ccutrer]

@ccutrer I do a local openhab-core build, then an openhab-addons and openhab-webui build (only if changes may affect UI). The last step is an openhab-distro build which produces a .zip for the core installation and a .kar for add-ons. I have a modified update script which updates my the test installation on my dev machine from a file (instead of the remote repository).

Hmm, a full core build is failing for me with

[ERROR] Failed to execute goal on project org.openhab.core.model.core: Could not resolve dependencies for project org.openhab.core.bundles:org.openhab.core.model.core:jar:4.0.0-SNAPSHOT: Failed to collect dependencies at org.openhab.core.bom:org.openhab.core.bom.compile-model:pom:4.0.0-SNAPSHOT -> org.eclipse.xtext:org.eclipse.xtext.generator:jar:2.29.0 -> org.eclipse.xtext:org.eclipse.xtext.xtext.generator:jar:2.29.0 -> org.eclipse.emf:org.eclipse.emf.mwe.utils:jar:1.8.0 -> org.eclipse.emf:org.eclipse.emf.mwe.core:jar:[1.8.0]: No versions available for org.eclipse.emf:org.eclipse.emf.mwe.core:jar:[1.8.0] within specified range

[J-N-K]

Can you try building with mvn clean install -U? I would suggest to wait until I fixes the Nashorn missing issue in AbstractScriptFileWatcher.

[wborn]

full core build is failing for me

I had the same issue yesterday and fixed it by removing all cached Eclipse artifacts from my local Maven repo in ~/.m2/repository/org/eclipse

[jpg0]

Awesome that this has been refactored! When working on this to support more advanced watching for JSScripting, it was pretty obvious that the existing code was really struggling to incorporate the changes. As long as the tests are passing then I'm happy (as they were already pretty extensive as I remember).

[wborn]

Can you fix the merge conflicts and address the review comment @J-N-K?

[wborn]

Thanks!