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