Skip to content
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

Add missing bundles to Demo App for Java 11 compatibility #1007

Merged
merged 1 commit into from
Dec 22, 2019

Conversation

wborn
Copy link
Member

@wborn wborn commented Nov 23, 2019

The Demo App can be used with both Java 8 and Java 11 when adding the same runrequires as used with itest projects.
The PR also makes the indentation more consistent by using tabs instead of spaces everywhere in app.bndrun.

Related to #768

@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/solved-setup-eclipse-on-ubuntu-observed-problems-with-java-version/85873/3

@maggu2810
Copy link
Contributor

I do not know if the setting comes from Eclipse IDE itself or from the Bndtools plugin but for me it seems that with the recent version spaces are used.
At least if I edit the bndrun files in the OpenHAB IDE setup and press tab, spaces are inserted.

@wborn
Copy link
Member Author

wborn commented Nov 23, 2019

I opted for tabs because when dragging/dropping bundles or hitting the Resolve button in the Bnd editor it replaces spaces with tabs on my machine. I also see spaces being used when hitting tab on a new line without indentation. But when I press enter at the end of a line that already has tabs it will indent using tabs.

@maggu2810
Copy link
Contributor

Okay, thanks for clarification.
Perhaps I will open an issue on Bndtools some time, that a consistent indentation would be nice and if it would be possible to pick up a special formatter.

@cweitkamp
Copy link
Contributor

Both are working for me.

When running from IDE on Java 11 I saw a similar warning like you fixed in #1001 for Java 9.

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/home/christoph/.m2/repository/org/apache/servicemix/bundles/org.apache.servicemix.bundles.xstream/1.4.7_1/org.apache.servicemix.bundles.xstream-1.4.7_1.jar) to field java.util.TreeMap.comparator
WARNING: Please consider reporting this to the maintainers of com.thoughtworks.xstream.core.util.Fields
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

@wborn
Copy link
Member Author

wborn commented Nov 26, 2019

The warnings will be gone when adding some add-opens using:

-runvm: \
	--add-opens java.base/java.io=ALL-UNNAMED,\
	--add-opens java.base/java.lang=ALL-UNNAMED,\
	--add-opens java.base/java.lang.reflect=ALL-UNNAMED,\
	--add-opens java.base/java.net=ALL-UNNAMED,\
	--add-opens java.base/java.security=ALL-UNNAMED,\
	--add-opens java.base/java.text=ALL-UNNAMED,\
	--add-opens java.base/java.util=ALL-UNNAMED,\
	--add-opens java.desktop/java.awt.font=ALL-UNNAMED,\
	--add-opens java.naming/javax.naming.spi=ALL-UNNAMED,\
	--add-opens java.rmi/sun.rmi.transport.tcp=ALL-UNNAMED

However that doesn't work with Java 8. It would be nice to keep it all in one file instead of creating a second file for Java 9+. I see there's a if macro instruction that may help. Perhaps it can check the value of osgi.ee in some way but I haven't been successful with that so far. Maybe you or @maggu2810 have some ideas regarding this matter?

@maggu2810
Copy link
Contributor

Did you see

ee
The name of the highest execution environment found in the current JAR

https://bnd.bndtools.org/chapters/855-macros-ref.html

@wborn
Copy link
Member Author

wborn commented Nov 28, 2019

Yes I tried that but unfortunately it didn't work.

I've also asked this as question in the bndtools-users group:

https://groups.google.com/forum/#!topic/bndtools-users/GQb-50si8cc

@wborn
Copy link
Member Author

wborn commented Nov 29, 2019

Apparently the Java version isn't available so the only remaining option is to move the extra VM parameters to a separate file for Java 9+.

@wborn wborn requested a review from a team November 29, 2019 23:03
The Demo App can be used with both Java 8 and Java 11 when adding the same runrequires as used with itest projects.
The PR also makes the indentation more consistent by using tabs instead of spaces everywhere in app.bndrun.
Add add-opens for Java 9+ in separate bndrun file.

Signed-off-by: Wouter Born <[email protected]>
@wborn
Copy link
Member Author

wborn commented Dec 22, 2019

Can we merge this @openhab/distro-maintainers?

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 The warnings are gone when running from app-java9plus.bndrun. I am fine with merging.

@cweitkamp cweitkamp merged commit 6b33069 into openhab:master Dec 22, 2019
@wborn
Copy link
Member Author

wborn commented Dec 22, 2019

Thanks! Now I can spend my time on other things instead of fixing merge conflicts. :-)

@wborn wborn deleted the demo-app-java-11 branch December 22, 2019 15:47
wborn added a commit that referenced this pull request Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants