On Tue, 27 Sep 2022 11:12:57 GMT, Doug Simon <dnsi...@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@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
