-
Notifications
You must be signed in to change notification settings - Fork 65
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
7903928: update build.gradle to use JDK 23 #271
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back sundar! A progress list of the required criteria for merging this PR into |
@sundararajana This change is no longer ready for integration - check the PR body for details. |
Webrevs
|
changed Python sample so that it does not fail with unsupported type _Float16
@@ -29,6 +29,7 @@ def static checkPath(String p) { | |||
} | |||
|
|||
def llvm_home = project.property("llvm_home") | |||
def jdk_home = project.property("jdk23_home") |
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.
We should rename this property simply to jdk_home
.
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.
I agree. Especially now that we only have one "master" (tip) version.
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.
Gradle's jdk is set via JAVA_HOME. It is usual have gradle support for the latest JDK to come bit "late". If we call JDK needed for jextract to be "jdk_Home" and the one needed for gradle as "JAVA_HOME", we may be in a situation where jdk_home points latest JDK (say JDK 25) and JAVA_HOME points whatever then latest Gradle supports. Can we keep jextract use specific JDK version for its build/test as a separately distinctly named variable?
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.
I think you can run gradle's Java compilation and the gradle tool with different JDKs; my previous strategy for running JMH through gradle was to define my local openjdk build as org.gradle.java.home
project property, either set in gradle.properties or passed as -Porg.gradle.java.home=xx
on command line. JAVA_HOME
is used to launch the gradle tool, so it cannot be the local openjdk build, which uses a class file format too new for gradle to generate abstract method implementations.
The |
This PR has been superseded by #275. Please continue the discussion and review there. |
@sundararajana this pull request can not be integrated into git checkout CODETOOLS-7903928
git fetch https://git.openjdk.org/jextract.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
@sundararajana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
We've updated native makefile to use JDK 23 (
0b89a46 )
While build.gradle works & expects jdk 23, it still refers to JDK var as "jdk22_home" and passes --release to be 22. Also jextract version is "22".
Progress
Issue
Backport <hash>
with the hash of the original commit. See Backports.Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jextract.git pull/271/head:pull/271
$ git checkout pull/271
Update a local copy of the PR:
$ git checkout pull/271
$ git pull https://git.openjdk.org/jextract.git pull/271/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 271
View PR using the GUI difftool:
$ git pr show -t 271
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jextract/pull/271.diff
Using Webrev
Link to Webrev Comment