On Tue, 23 Nov 2021 04:57:57 GMT, Alexander Matveev <almat...@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,j dk.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. > > Marked as reviewed by almatvee (Reviewer). Thank you for the review @sashamatveev. For a moment I was thinking whether I should withdraw this PR because your manual run already narrowed down the exception stacktrace. But I will go ahead with the merge so as to allow any future failures to generate this root cause information and see if it consistently fails due to the same error. ------------- PR: https://git.openjdk.java.net/jdk/pull/6491
