-
Notifications
You must be signed in to change notification settings - Fork 117
Support exploded JDK. #118
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
Conversation
If you believe this is an error, please contact graal-dev@openjdk.java.net. |
|
|
Why is this necessary? Are you building a JDK8 with a non-standard layout? Or setting |
|
I'm using |
|
Ok, sounds reasonable. Can you please bump the patch level number of the |
|
What happened here? It was merged and then reverted?!? The patch is even gone in my branch now... |
|
Sorry, it had broke our continuous integration builds with JDK 8. I had to take it out immediately. I'll provide a fix for this issues and re-merge. |
|
What about the version bump, as @dougxc requested? |
|
Yes, I've seen that later, but it is not related. I'm working right now to fix the issue. |
|
Ok, thanks. |
|
|
||
| def get_jdk_path(self, jdk, path): | ||
| # Exploded JDKs don't have a jre directory. | ||
| if exists(join(jdk.home, 'jre')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue here is, that mx.graal-core/suites.py links to the jre directory
"JVMCI_API" : {
"path" : "jre/lib/jvmci/jvmci-api.jar",
....
},
in that case, following path is created: $JAVA_HOME/jre/jre/lib/jvmci/jvmci-api.jar.
We could do a bit of fuzzying like:
def get_jdk_path(self, jdk, path):
# Exploded JDKs don't have a jre directory.
if exists(join(jdk.home, path)):
return join(jdk.home, path)
elif exists(join(jdk.home, 'jre')):
return join(jdk.home, 'jre', path)
else:
return join(jdk.home, path)
A simpler version would be:
def get_jdk_path(self, jdk, path):
# Exploded JDKs don't have a jre directory.
if exists(join(jdk.home, path)):
return join(jdk.home, path)
else:
return join(jdk.home, 'jre', path)
this is not beautilful but in order to get this feature and have backwards compatibility we have to do it.
In the long run, we may remove the jre prefix from the suites.py dependencies.
Just have noticed, you suggested to remove the jre prefix anyway. However, the backwards compatibility must be ensured. The second suggestion would do the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I didn't think of being backwards compatible.
|
Ok, I've prepeared an internal review with the second (simpler) version of |
|
I haven't tried the patch but it looks ok. Did you try it? |
In this scenario, which JDK version are you building? Based on your mention of |
|
No, it's 8u; it also has |
|
Ok, thanks for the clarification. I guess you're deploying |
|
Yes. By doing that we have |
|
Yeah, this doesn't work right now. Are you guys pushing a change to |
|
Thanks! |
…OM/mx:NetBeansBuildDir to master * commit 'b0808a94c0ec6b19519341d68fb0ffdbf795fe39': Point NetBeans build.dir to the directory where classes are really created
Using an exploded JDK results in:
This also needs a change in the graal-core suite file: