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

Introducing Java 9 support! (Fixes #576) #649

Merged
merged 1 commit into from
Dec 29, 2017

Conversation

smartboyathome
Copy link
Contributor

@smartboyathome smartboyathome commented Dec 29, 2017

  • Glowstone has a new GlowBootstrap main() method, in charge of downloading any depndencies before starting the application.
  • GlowMain now contains the contents of the old GlowServer.main() method.
  • LibraryManager was rewritten to use the Jar Class Loader library, rather than relying on URLClassLoader.addURL().
  • Still works on Java 8!

- Glowstone has a new GlowBootstrap main() method, in charge of downloading any depndencies before starting the application.
- GlowMain now contains the contents of the old GlowServer.main() method.
- LibraryManager was rewritten to use the Jar Class Loader library, rather than relying on URLClassLoader.addURL().
@Pr0methean
Copy link
Contributor

NB: Until projectlombok/lombok#985 can at least be worked around, we'll still have to build on JDK 8 and can't do a multirelease jar.

@hibo98
Copy link
Member

hibo98 commented Dec 29, 2017

Hey, I yesterday also tried to figure out a fix for this problem.
But I think my fix is one of the quick and dirty ones.
Also because logging is not correctly initialized in the agent.
hibo98@d04866c

So that looks quiet good

@smartboyathome
Copy link
Contributor Author

Good note. It's true that it won't build on Java 9, but I was more focused on getting it to run. The key to this was minimizing the classes referenced in the bootstrap phase. That's one reason I created GlowMain and reference it only as a runnable, trying hard to avoid triggering any static variables from being loaded.

@mastercoms mastercoms merged commit 3f96747 into GlowstoneMC:dev Dec 29, 2017
@hibo98
Copy link
Member

hibo98 commented Dec 29, 2017

I test this pull-request on my server and it did not work!
When the server trys to connect the database then it throws a SQLException.

java.sql.SQLException: No suitable driver found for jdbc:mysql://*******
        at java.sql/java.sql.DriverManager.getConnection(Unknown Source)
        at java.sql/java.sql.DriverManager.getConnection(Unknown Source)
...

So the mysql driver is not loaded!
Verison is 2018.0.0-SNAPSHOT.3f96747

--- EDITED ---

This Problem occurs under Java 8 too.

java.sql.SQLException: No suitable driver found for jdbc:mysql://*******
        at java.sql.DriverManager.getConnection(DriverManager.java:689)
        at java.sql.DriverManager.getConnection(DriverManager.java:247)
...

mastercoms added a commit that referenced this pull request Dec 29, 2017
mastercoms added a commit that referenced this pull request Dec 29, 2017
@mastercoms
Copy link
Member

I have reverted it. @smartboyathome please remake the PR.

@smartboyathome
Copy link
Contributor Author

@hibo98 I cannot reproduce this. I ran it on my Win10 machine and it starts up just fine, both under Java 8 and Java 9. What OS are you using? Any specific configuration I should be aware of?

@hibo98
Copy link
Member

hibo98 commented Dec 29, 2017

Like you I am using Windows 10, with installed Oracle Java 8 or 9. And the start up also fine.
The Problem occurs, when my plugin try to connect to the database.
So the loading of the classes probably don't work.
You have to test for example with a plugin that uses a mysql connection.

@smartboyathome
Copy link
Contributor Author

Thanks. I'll play around some tonight, but https://stackoverflow.com/questions/42052856/java-9-classpath-and-library-path-extension worries me, seems like plugins that rely on JDBC just may not work with how Glowstone downloads its libraries anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants