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 ClassLoader to CompileOptions in order to allow for overriding the ClassLoader #72

Closed
vksiva opened this issue Nov 27, 2018 · 9 comments

Comments

@vksiva
Copy link

vksiva commented Nov 27, 2018

Expected behavior and actual behavior:

Using the example as presented in the README,

Supplier<String> supplier = Reflect.compile(
    "com.example.HelloWorld",
    "package com.example;\n" +
    "class HelloWorld implements java.util.function.Supplier<String> {\n" +
    "    public String get() {\n" +
    "        return \"Hello World!\";\n" +
    "    }\n" +
    "}\n").create().get();

// Prints "Hello World!"
System.out.println(supplier.get());

If the text has to be changed to "Hi World!" and the class is recompiled, the below code still prints "Hello World!"

 supplier = Reflect.compile(
    "com.example.HelloWorld",
    "package com.example;\n" +
    "class HelloWorld implements java.util.function.Supplier<String> {\n" +
    "    public String get() {\n" +
    "        return \"Hi World!\";\n" +
    "    }\n" +
    "}\n").create().get();

// Still prints "Hello World!"
System.out.println(supplier.get());

One way to ensure the class gets recompiled is by using different classloaders during compile. Can we use a custom classloader for the Reflect.compile() method. It was raised in #64.

Steps to reproduce the problem:

Mentioned above.

Versions:

  • jOOR: 0.9.9
  • Java: 1.8
@lukaseder
Copy link
Member

Thanks for reporting this. Indeed, we've solved this in the next release through #67

@lukaseder
Copy link
Member

lukaseder commented Nov 27, 2018

In fact, no. CompileOptions does not allow for overriding the ClassLoader yet. Will look into this soon. There are tons of open questions when doing that, of course. The current approach allows for some additional visibility to be implemented, from the very location of where the class is being compiled. With arbitrary class loaders, this will no longer be true.

Further investigation needed.

@lukaseder lukaseder changed the title Recompiled class (JDK 1.8) does not reflect the changes Add ClassLoader to CompileOptions in order to allow for overriding the ClassLoader Nov 27, 2018
lukaseder added a commit that referenced this issue Dec 4, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ing the ClassLoader
@lukaseder
Copy link
Member

The custom class loader feature has been implemented. But I will also implement a breaking change that avoids re-loading a previously compiled class by default. I don't think that's what anyone really wants. See #76

@lukaseder
Copy link
Member

Hmm, no in fact #76 cannot be implemented. The current implementation prevents LinkageError in some cases.

@lukaseder
Copy link
Member

I will revert this change. Unfortunately, providing a custom ClassLoader doesn't really help solve any problems in JDK 9+

@lukaseder
Copy link
Member

This appears to be useful for jOOQ's code generator as well, including for:

A quick attempt at re-implementing this seems to work for jOOQ (whose code generation plugin creates its own URLClassLoader). Perhaps I just made a mistake when implementing the tests at the time?

I'll try this again.

@lukaseder
Copy link
Member

Hmm, no in fact #76 cannot be implemented. The current implementation prevents LinkageError in some cases.

I'm no longer running into these errors from:
#72 (comment)

Perhaps that was a JDK bug, at the time?

@lukaseder
Copy link
Member

Fixed for jOOR 0.9.15

@lukaseder
Copy link
Member

Time for a release, too.

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

No branches or pull requests

2 participants