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

Fixes java mode loading #3444

Merged
merged 4 commits into from Jul 9, 2015

Conversation

Projects
None yet
3 participants
@Manindra29
Member

Manindra29 commented Jul 1, 2015

If the NoClassDefFoundError is caught for JavaMode, reinits the urlclassloader. (fixes #3443)

Manindra29 added some commits Jun 30, 2015

reloading with java mode
Signed-off-by: Manindra Moharana <mkmoharana29@gmail.com>

@Manindra29 Manindra29 changed the title from Fixes java mode loading #3443 to Fixes java mode loading Jul 1, 2015

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Jul 3, 2015

Member

This is a really hacky approach and only gets it working with Java Mode...

We already have the idea of the imports entry in the .properties file, a better option would likely be to include the import for Java Mode in there, and then use that to figure out which classes need to be loaded. So you'd include:

imports=processing.mode.java

in your mode.properties file, and we'd iterate through the modes to find the right classes, then read those JARs from the new mode's ClassLoader.

On a quick glance, it also looks like your approach is stealing the ClassLoader of JavaMode and polluting it with the subclassing Mode's classes, which is also a bad idea, because it would mean other Modes would be capable of breaking the default mode, and therefore completely hosing Processing.

Member

benfry commented Jul 3, 2015

This is a really hacky approach and only gets it working with Java Mode...

We already have the idea of the imports entry in the .properties file, a better option would likely be to include the import for Java Mode in there, and then use that to figure out which classes need to be loaded. So you'd include:

imports=processing.mode.java

in your mode.properties file, and we'd iterate through the modes to find the right classes, then read those JARs from the new mode's ClassLoader.

On a quick glance, it also looks like your approach is stealing the ClassLoader of JavaMode and polluting it with the subclassing Mode's classes, which is also a bad idea, because it would mean other Modes would be capable of breaking the default mode, and therefore completely hosing Processing.

@Manindra29

This comment has been minimized.

Show comment
Hide comment
@Manindra29

Manindra29 Jul 7, 2015

Member

@benfry I wasn't aware of the imports field in mode.properties. I've updated the PR as you suggested. Now dependencies are loaded by classname. If mode.properties has:

imports=processing.mode.java.JavaMode

The installed modes are searched for a class name matching processing.mode.java.JavaMode. If such a Mode is found, all of its jars are added to the urlclassloader. This method feels much cleaner.

(This works with REPLMode)

Member

Manindra29 commented Jul 7, 2015

@benfry I wasn't aware of the imports field in mode.properties. I've updated the PR as you suggested. Now dependencies are loaded by classname. If mode.properties has:

imports=processing.mode.java.JavaMode

The installed modes are searched for a class name matching processing.mode.java.JavaMode. If such a Mode is found, all of its jars are added to the urlclassloader. This method feels much cleaner.

(This works with REPLMode)

@codeanticode

This comment has been minimized.

Show comment
Hide comment
@codeanticode

codeanticode Jul 8, 2015

Member

Tested the android mode, it works now!

Member

codeanticode commented Jul 8, 2015

Tested the android mode, it works now!

benfry added a commit that referenced this pull request Jul 9, 2015

@benfry benfry merged commit 85184bb into master Jul 9, 2015

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Jul 9, 2015

Member

Thanks for fixing.

Member

benfry commented Jul 9, 2015

Thanks for fixing.

@benfry benfry deleted the fixJavaModeLoading branch Aug 14, 2015

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