RFR: 8237467: effect of jlink plugins for vendor information and command-line options should be sticky

Mandy Chung mchung at openjdk.org
Tue Sep 27 19:16:11 UTC 2022


On Tue, 27 Sep 2022 11:12:57 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

> This PR adds a new jlink plugin (`--save-jlink-argfiles=<filenames>`) to support persisting jlink options.
> 
> 
>> echo "--add-modules jdk.internal.vm.ci --add-options=-Dfoo=xyzzy --vendor-version="XyzzyVM 3.14.15" --vendor-bug-url=https://bugs.xyzzy.com/" > my_image.argfile
>> export ALL_MODULES=$(java --list-modules | sed 's:@.*::g' | tr '\n' ',')
>> jlink --keep-packaged-modules=my_image/jmods --add-modules $ALL_MODULES --output=my_image --save-jlink-argfiles my_image.argfile
>> my_image/bin/java -XshowSettings:properties --version 2>&1 | grep yzzy
>> my_image/bin/jlink --add-modules=java.base --output=my_image2
>> my_image2/bin/java -XshowSettings:properties --version 2>&1 | grep yzzy
>     foo = xyzzy
>     java.vendor.url.bug = https://bugs.xyzzy.com/
>     java.vendor.version = XyzzyVM 3.14.15
> OpenJDK Runtime Environment XyzzyVM 3.14.15 (build 20-internal-2022-09-22-0951036.dnsimon...)
> OpenJDK 64-Bit Server VM XyzzyVM 3.14.15 (build 20-internal-2022-09-22-0951036.dnsimon..., mixed mode)
>> my_image2/bin/java -d jdk.internal.vm.ci | head -1
> jdk.internal.vm.ci at 20-internal

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/CommandLine.java line 2:

> 1: /*
> 2:  * Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.

This is a new file and so I would just have the copyright start year.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/CommandLine.java line 43:

> 41:  * Various utility methods for processing Java tool command line arguments.
> 42:  *
> 43:  *  <p><b>This is NOT part of any supported API.

This note was prior to module system.  We could drop this comment since it's encapsulated and it's not an exported package.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 234:

> 232:         Path outputPath = null;
> 233:         try {
> 234:             try (InputStream savedOptions = JlinkTask.class.getModule().getResourceAsStream("jdk/tools/jlink/internal/options")) {

nit: wrap this long line that helps future side-by-side review.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 236:

> 234:             try (InputStream savedOptions = JlinkTask.class.getModule().getResourceAsStream("jdk/tools/jlink/internal/options")) {
> 235:                 if (savedOptions != null) {
> 236:                     List<String> newArgs = new ArrayList<>();

`s/newArgs/prependArgs/` to make it clearer.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SaveJlinkArgfilesPlugin.java line 96:

> 94:     public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) {
> 95:         in.transformAndCopy(Function.identity(), out);
> 96:         byte[] savedOptions = argfiles.stream().

Formatting consistent with jlink files:

Suggestion:

        byte[] savedOptions = argfiles.stream()
                                      .collect(Collectors.joining("\n"))
                                      .getBytes(StandardCharsets.UTF_8);

test/jdk/tools/jlink/plugins/SaveJlinkArgfilesPluginTest.java line 81:

> 79:         // Check that the primary image creation ignored the saved args
> 80:         oa.shouldHaveExitValue(0);
> 81:         oa.shouldNotMatch("yzzy");

Can this check for `java.vendor.version = XyzzyVM 3.14.15`, that will also help the reader to understand what the test tries to verify.

test/jdk/tools/jlink/plugins/SaveJlinkArgfilesPluginTest.java line 94:

> 92:         oa = ProcessTools.executeProcess(launcher.toString(), "-XshowSettings:properties", "--version");
> 93:         oa.shouldHaveExitValue(0);
> 94:         expectNMatchingLines(oa.getStdout(), "yzzy", 2);

Same for line 94-95: check for `java.vendor.version = XyzzyVM 3.14.15` instead

-------------

PR: https://git.openjdk.org/jdk/pull/10445


More information about the core-libs-dev mailing list