On Sun, 21 Nov 2021 12:39:17 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which proposes to add more > diagnostics when a failure occurs in the jlink tool during the jpackage tests? > > As noted in https://bugs.openjdk.java.net/browse/JDK-8277507, so far 3 > failures have been reported in jpackage tests (across different test cases) > with the same underlying cause coming from the jlink tool. Since this failure > isn't specific to one or two tests and seems to be keep happening across > these tests in `test/jdk/tools/jpackage/`, I decided to set this system > property from one central location in the `HelloApp` which gets used by these > tests. These failures have only been reported on macos (and specifically > aarch64), but the commit here doesn't do any OS name checks, to keep this > change simple. Furthermore, looking at the > `jdk.tools.jlink.internal.JlinkTask` code, enabling this system property will > _not_ generate any additional logs unless there's an exception thrown by the > `jlink` tool, in which case it prints the exception stacktrace. So enabling > this by default won't end up increasing the log output or flooding the logs. > > With this change, I have run the 3 tests noted in those issues locally to > make sure this doesn't break anything else. I have also verified manually > that enabling this system property does indeed get propagated to the > `JlinkTask` and checked the logs to see that the command line does pass it: > >> >> [17:51:07.123] TRACE: exec: Execute >> [macosx-x86_64-server-release/images/jdk/bin/jpackage --input ./test/input >> --dest ./test/output --name "Name With Space" --type pkg >> -J-Djlink.debug=true --main-jar hello.jar --main-class Hello --verbose](15); >> inherit I/O... >> [17:51:07.364] Building PKG package for Name With Space. >> [17:51:19.191] Command [PID: -1]: >> jlink --output >> /var/folders/7v/cnkwrnx154926cr3289w4rd80000gp/T/jdk.jpackage13608930262477285540/images/image-3713034571561734902/Name >> With Space.app/Contents/runtime/Contents/Home --module-path >> macosx-x86_64-server-release/images/jdk/jmods --add-modules >> java.rmi,jdk.management.jfr,jdk.jdi,jdk.charsets,jdk.xml.dom,java.xml,java.datatransfer,jdk.jstatd,jdk.httpserver,java.desktop,java.security.sasl,jdk.zipfs,java.base,jdk.crypto.ec,jdk.javadoc,jdk.management.agent,jdk.jshell,jdk.editpad,jdk.jsobject,java.sql.rowset,jdk.sctp,jdk.unsupported,jdk.jlink,java.smartcardio,java.security.jgss,jdk.nio.mapmode,java.compiler,jdk.dynalink,jdk.unsupported.desktop,jdk.accessibility,jdk.security.jgss,jdk.incubator.vector,java.sql,java.xml.crypto,java.logging,java.transaction.xa,jdk.jfr,jdk.crypto.cryptoki,jdk.net,jdk.random,java.naming,jdk.internal.ed,java.prefs,java.net.http,jdk.compiler,jdk.internal.opt,jdk.naming.rmi,jdk.jconsole,jdk.attach,jdk.internal.le,java.management,jdk.jdwp.agent,jd k.incubator.foreign,jdk.internal.jvmstat,java.instrument,jdk.management,jdk.security.auth,java.scripting,jdk.jdeps,jdk.jartool,jdk.jpackage,java.management.rmi,jdk.naming.dns,jdk.localedata --strip-native-commands --strip-debug --no-man-pages --no-header-files >> [17:51:19.192] Output: >> WARNING: Using incubator modules: jdk.incubator.foreign, >> jdk.incubator.vector >> >> [17:51:19.192] Returned: 0 >> > > These tests get run in `tier2` and I have no way of running the entire > `tier2` locally or relying of GitHub actions which only runs `tier1`. If this > change looks OK and is approved, then I would like to request for help > running the entire tests against this PR before merging it. Thank you Alan. I'll wait for someone from jpackage area to take a look at this change. ------------- PR: https://git.openjdk.java.net/jdk/pull/6491