Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

AOT compiled jar causing problems #339

Closed
Deraen opened this issue Oct 18, 2015 · 9 comments
Closed

AOT compiled jar causing problems #339

Deraen opened this issue Oct 18, 2015 · 9 comments
Labels

Comments

@Deraen
Copy link

Deraen commented Oct 18, 2015

Hi,

Based on the fact that you are aot compiling onyx.interop namespace, I believe the intent of aot compilation is to have some Java classes available in Jar for Java applications?

However, the side effect of AOT is that ALL namespaces that interop namespace transitively depends on are AOT compiled, including clojure.core, timbre, tools.reader... This will cause problems with applications which use need different versions of these dependencies as AOT compiled class files in the classpath will shadow all .clj sources. This completely breaks Clojurescript in project with Onyx dependency.

It should be easy to check what is happening by checking the contents of the jar file.

If the intent of AOT compilation is to provide some Java classes in JAR, I believe the interop ns should lazily load other namespaces so that it doesn't have transitive dependencies to other namespaces.

Deraen added a commit to Deraen/onyx that referenced this issue Oct 18, 2015
This avoids AOT compilation of complete dependency tree (including
clojure.core, tools.reader...) which causes problems with projects which
use other versions of these libraries.

Fixes onyx-platform#339
Deraen added a commit to Deraen/onyx that referenced this issue Oct 18, 2015
This avoids AOT compilation of complete dependency tree (including
clojure.core, tools.reader...) which causes problems with projects which
use other versions of these libraries.

Fixes onyx-platform#339
@lbradstreet
Copy link
Member

Thank you. You are correct that this is the intent, and I was wondering if this would come back to bite us.

I definitely want to fix this, though I'll have to spend some time looking at the solution and will discuss it with @MichaelDrogalis. We do something similar with the messaging layer, so your solution is not out of the question.

@nha
Copy link

nha commented Oct 18, 2015

@lbradstreet Maybe you remember we had a chat about an exception thrown when using ClojureScript, and it turns out this is the reason.
@Deraen thanks for diagnosing the problem and submitting a patch !

@lbradstreet
Copy link
Member

Oh, great! It was the first thing I thought of when I saw this issue.

I really appreciate you both spending the time to track this down.

On 18 Oct 2015, at 8:59 PM, Nicolas Ha [email protected] wrote:

@lbradstreet Maybe you remember we had a chat about an exception thrown when using ClojureScript, and it turns out this is the reason.
@Deraen thanks for diagnosing the problem and submitting a patch !


Reply to this email directly or view it on GitHub.

@MichaelDrogalis
Copy link
Contributor

Thanks for sending in a patch for this @Deraen / @nha. Looking at it now. :)

@MichaelDrogalis
Copy link
Contributor

There's not been a whole lot of work around transitive AOT for the last few years. Not sure we have many other options on the table. http://dev.clojure.org/display/design/Transitive+AOT+Compilation

MichaelDrogalis pushed a commit that referenced this issue Oct 18, 2015
@MichaelDrogalis
Copy link
Contributor

Possible fix in c7a08d8.

This essentially does the same thing, but elides the costly require statement invoked every time. I believe this should work because onyx.system will invoke the task lifecycle namespace, which brings on the function namespace that we need. Repeated calls to resolve, while non-zero, are relatively small. Thoughts?

PS - tested this on our Java input plugin and things continued to work, and it also fixed my reproduction of this bug.

@lbradstreet
Copy link
Member

Assuming it's fixed, I think we should do a new 0.7.x release with just this fix in. I'd hate others to hit this issue over the next month.

@MichaelDrogalis
Copy link
Contributor

Agreed. Let's cut one today.

MichaelDrogalis pushed a commit that referenced this issue Oct 19, 2015
@MichaelDrogalis
Copy link
Contributor

@nha & @Deraen: the patch that excludes require ended up doing the trick. Thank you both immensely for tracking this down - I've added both of you to the contributors list on the README. Will release today with the fix.

andrewberls added a commit to framed-data/overseer that referenced this issue Nov 25, 2015
overseer.runner is used by the launcher script included with Overseer.
It used to be AOT-compiled, but this was disabled as AOT compilation
causes issues with applications that need to modify dependency versions
- see onyx-platform/onyx#339. This re-enables
AOT-compilation for that namespace, but uses the same fix strategy of
requiring namespaces at runtime and resolving symbols which should
address the issue. This probably comes at some minor perf cost, but is
only run once at runtime.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants