RFR: 7903136: Don't require jdk18_home to be set in jextract gradle build [v4]
Brice Dutheil
duke at openjdk.java.net
Wed Mar 30 11:12:05 UTC 2022
On Tue, 29 Mar 2022 18:29:55 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
>> Drop the requirement to set `jdk18_home` in the gradle build, and rely instead on `JAVA_HOME` being set to JDK 18.
>>
>> I've also updated the build instructions. The `-Pjtreg_home` flag seems to have been dropped by accident from the test command. I've re-added it. Also renamed `libclang_dir` -> `llvm_dir` which was missed in an earlier change.
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>
> Tabs to spaces
build.gradle line 39:
> 37: }
> 38:
> 39: String foundVersionStr = props.getProperty("JAVA_VERSION");
Just for information: In my experience checking the version in the release file is unfortunately not enough to ascertain a valid JDK. In particular when playing with multiple variant / builds. This might be a remote problem, but I think getting system properties is better.
So executing something like (pseudo code)
def file = Files.createTempFile("sysprop", ".java").toFile()
file.deleteOnExit()
def proc = new ProcessBuilder(['java', '--source', '11', file.absolutePath]).start()
proc.waitFor()
if (0 != proc.exitValue()) {
throw new RuntimeException("can't read system properties: ${proc.exitValue()}")
}
Properties props = new Properties();
try (InputStream input = proc.inputStream) {
props.load(input);
}
The command could be replaced by: `jshell -s - <<< "System.getProperties().forEach((k, v) -> System.out.println(k + "=" + v))"` thus avoiding the temporary file.
build.gradle line 59:
> 57:
> 58: def String downloadJDK() {
> 59: String jdkUrl = "https://download.java.net/java/GA/jdk18/43f95e8614114aeaa8e8a5fcf20a682d/36/GPL/openjdk-18_";
You should put this in a property so gradle won't have to recompile/reconfigure the script if this value changes.
build.gradle line 92:
> 90: if (project.hasProperty("jdk")) {
> 91: println("using JDK from 'jdk' project property");
> 92: jdkHome = project.property("jdk");
I think the property assignment can be replaced by the following, if the property is not found then it will return `null`
Suggestion:
String jdkHome = project.findProperty("jdk");
if (jdkHome != null) {
logger.lifecycle("using JDK from 'jdk' project property");
Also you may want to replace `println` by logger statements, although that may be a bit out of scope to do that for the whole file in this PR.
-------------
PR: https://git.openjdk.java.net/jextract/pull/9
More information about the jextract-dev
mailing list