RFR: 8287828: Fix so that one can select jtreg test case by ID from make
One can select a testcase by ID when running a jtreg test case directly from jtreg (using the testcase.java#testID syntax). However, this has not been possible to do when launching jtreg indirectly from make. This fix attempts to address this issue. I have not tested this thoroughly yet, I wanted to show the code to get feedback first. The idea is to *not* emulated destructive imperative assignments through the use of eval. I instead try to handle it in a functional style without reassigning variables. I have tried to make the change as small as possible. I am not used to change the build system, so I would appreciate thorough review. - Commit messages: - 8287828: Fix so that one can select jtreg test case by ID from make Changes: https://git.openjdk.java.net/jdk/pull/9028/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9028=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287828 Stats: 48 lines in 1 file changed: 24 ins; 21 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 22:40:52 GMT, Tim Prinzing wrote: >> make/test/JtregNativeJdk.gmk line 67: >> >>> 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) >>> jvm.lib >>> 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib >>> 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc >> >> Does this test need this flag? or the default exception handling behavior >> is adequate? > > iostreams don't compile without it. While the tests currently have printf, > the 8281001 restores back to using iostream and this file doesn't have to > change. Ok, thanks. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 18:16:53 GMT, Mandy Chung wrote: >> Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest > > make/test/JtregNativeJdk.gmk line 67: > >> 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) jvm.lib >> 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib >> 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc > > Does this test need this flag? or the default exception handling behavior is > adequate? iostreams don't compile without it. While the tests currently have printf, the 8281001 restores back to using iostream and this file doesn't have to change. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Thanks for doing the run, Dan. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest I did a Mach5 Tier1 test run on the v00 version of this patch. There were no failures and all 6 runs of jni/nullCaller/NullCallerTest.java passed. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Marked as reviewed by mchung (Reviewer). make/test/JtregNativeJdk.gmk line 67: > 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) jvm.lib > 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib > 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc Does this test need this flag? or the default exception handling behavior is adequate? - PR: https://git.openjdk.java.net/jdk/pull/9010
Integrated: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint
On Thu, 2 Jun 2022 20:59:26 GMT, Jonathan Gibbons wrote: > Please review a moderate change to enable (most) doclet warnings even if > doclint is enabled. > > Since the introduction of doclint, there was some (small) overlap between the > small set of warnings generated by the doclet and the new larger set of > diagnostics that could be generated by doclint. The solution, until now, has > been to disable doclet warnings when doclint is enabled. But, neither set > contains the other, and the policy has inappropriately suppressed some > warnings and inhibited writing new code to generate new warnings by the > doclet that could only be done by the doclet, and not doclint. > > One notable group of warnings that has been inappropriately suppressed is the > warnings generated by using the `-serial warn` option. > > The fundamental core of the change is to remove the conditional checks in the > doclet `Messages.java`, which would suppress messages when doclint was > enabled. A consequence is that some specific messages have to disabled if > they are duplicate checks if doclint is enabled. (The messages cannot be > removed altogether because doclint might _not_ be enabled.) A test is added > to verify that either one message or the other is generated, but never both. > > As previously noted, an issue with the earlier policy is that warnings > generated by using the `-serial warn` option were inappropriately suppressed, > and this change fixes that ... revealing a number of latent warnings in the > JDK API that need to be addressed. The short term fix here is to temporarily > remove the use of the `-serialwarn` option. See > [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749). This pull request has now been integrated. Changeset: 59e9700c Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/59e9700c4e0ae892f15607bcaa267e5868eb0512 Stats: 267 lines in 11 files changed: 228 ins; 21 del; 18 mod 8252717: Integrate/merge legacy standard doclet diagnostics and doclint Reviewed-by: erikj, prappo - PR: https://git.openjdk.java.net/jdk/pull/9006
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint [v2]
On Thu, 2 Jun 2022 23:33:13 GMT, Pavel Rappo wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> fix whitespace > > test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java > line 190: > >> 188: checkExit(expect.exit); >> 189: >> 190: checkOutput(Output.OUT, true,expect.message); > > Add whitespace after `true,` ✅ - PR: https://git.openjdk.java.net/jdk/pull/9006
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint [v2]
> Please review a moderate change to enable (most) doclet warnings even if > doclint is enabled. > > Since the introduction of doclint, there was some (small) overlap between the > small set of warnings generated by the doclet and the new larger set of > diagnostics that could be generated by doclint. The solution, until now, has > been to disable doclet warnings when doclint is enabled. But, neither set > contains the other, and the policy has inappropriately suppressed some > warnings and inhibited writing new code to generate new warnings by the > doclet that could only be done by the doclet, and not doclint. > > One notable group of warnings that has been inappropriately suppressed is the > warnings generated by using the `-serial warn` option. > > The fundamental core of the change is to remove the conditional checks in the > doclet `Messages.java`, which would suppress messages when doclint was > enabled. A consequence is that some specific messages have to disabled if > they are duplicate checks if doclint is enabled. (The messages cannot be > removed altogether because doclint might _not_ be enabled.) A test is added > to verify that either one message or the other is generated, but never both. > > As previously noted, an issue with the earlier policy is that warnings > generated by using the `-serial warn` option were inappropriately suppressed, > and this change fixes that ... revealing a number of latent warnings in the > JDK API that need to be addressed. The short term fix here is to temporarily > remove the use of the `-serialwarn` option. See > [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749). Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: fix whitespace - Changes: - all: https://git.openjdk.java.net/jdk/pull/9006/files - new: https://git.openjdk.java.net/jdk/pull/9006/files/2f4bc7fb..55b5576f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9006=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9006=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/9006.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9006/head:pull/9006 PR: https://git.openjdk.java.net/jdk/pull/9006
RFR: 8287811: JFR: jfr configure error message should not print stack trace
Could I have a review of PR that removes a printStackTrace() for the jfr tool. Testing: jdk/jfr/tool Thanks Erik - Commit messages: - Initial Changes: https://git.openjdk.java.net/jdk/pull/9018/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9018=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287811 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9018.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9018/head:pull/9018 PR: https://git.openjdk.java.net/jdk/pull/9018
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint
On Fri, 3 Jun 2022 15:59:04 GMT, Pavel Rappo wrote: > > I did a visual inspection over the Checker class. > > > The test is set up to be a framework to accommodate new cases if any should > > arise. > > This is good. It would be even better if we had a mechanism to reduce risk of > diagnostic duplication in the future. I don't have any concrete proposal, but > unique internal error codes shared between doclint and doclet come to mind. I would also like to have a policy to align the user-facing content of the diagnostic message. This includes the severity (warning or error) as well as the position and message text, including substituted values. - PR: https://git.openjdk.java.net/jdk/pull/9006
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint
On Thu, 2 Jun 2022 20:59:26 GMT, Jonathan Gibbons wrote: > Please review a moderate change to enable (most) doclet warnings even if > doclint is enabled. > > Since the introduction of doclint, there was some (small) overlap between the > small set of warnings generated by the doclet and the new larger set of > diagnostics that could be generated by doclint. The solution, until now, has > been to disable doclet warnings when doclint is enabled. But, the sets do not > overlap, and the policy has inappropriately suppressed some warnings and > inhibited the generation of new warnings by the doclet that could only be > done by the doclet, and not doclint. > > One notable group of warnings that has been inappropriately suppressed is the > warnings generated by using the `-serial warn` option. > > The fundamental core of the change is to remove the conditional checks in the > doclet `Messages.java`, which would suppress messages when doclint was > enabled. A consequence is that some specific messages have to disabled if > they are duplicate checks if doclint is enabled. (The messages cannot be > removed altogether because doclint might _not_ be enabled.) A test is added > to verify that either one message or the other is generated, but never both. > > As previously noted, an issue with the earlier policy is that warnings > generated by using the `-serial warn` option were inappropriately suppressed, > and this change fixes that ... revealing a number of latent warnings in the > JDK API that need to be addressed. The short term fix here is to temporarily > remove the use of the `-serialwarn` option. See > [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749). Looks good. > I did a visual inspection over the Checker class. > The test is set up to be a framework to accommodate new cases if any should > arise. This is good. It would be even better if we had a mechanism to reduce risk of diagnostic duplication in the future. I don't have any concrete proposal, but unique internal error codes shared between doclint and doclet come to mind. - Marked as reviewed by prappo (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9006
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint
On Fri, 3 Jun 2022 07:48:15 GMT, Pavel Rappo wrote: > > Since the introduction of doclint, there was some (small) overlap between > > the small set ... and the new larger set ... But, the sets do not overlap > > Do or do they not overlap? :) Ooops. Wrong word. I was meaning that neither set contains the other. > > > the policy has inappropriately suppressed some warnings and inhibited the > > generation of new warnings > > What's the difference between "suppressed" and "inhibited" here? I was using "suppressed" with respect to existing doclet warnings, which have not been displayed if doclint was enabled, and was using "inhibited" with respect to writing code to generate new warnings, meaning that there was not much benefit to adding new warnings if they were typically going to be suppressed when doclint was enabled. > > Generally, how can we be sure that we haven't missed any double-reported > cases? I did a visual inspection over the Checker class. Generally, Checker does more in the area of "missing" comments and HTML issues, with less focus on "reference" issues, while the doclet does not report missing comments and does not even attempt to detect HTML issues. The overlap is "syntax" issues and "reference" issues, which are covered here. The test is set up to be a framework to accommodate new cases if any should arise. - PR: https://git.openjdk.java.net/jdk/pull/9006
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9010
RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest - Commit messages: - 8287745 Fix JtregNativeJdk.gmk to include c++ libs for NullCallerTest Changes: https://git.openjdk.java.net/jdk/pull/9010/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9010=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287745 Stats: 4 lines in 2 files changed: 1 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/9010.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9010/head:pull/9010 PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint
On Thu, 2 Jun 2022 20:59:26 GMT, Jonathan Gibbons wrote: > Please review a moderate change to enable (most) doclet warnings even if > doclint is enabled. > > Since the introduction of doclint, there was some (small) overlap between the > small set of warnings generated by the doclet and the new larger set of > diagnostics that could be generated by doclint. The solution, until now, has > been to disable doclet warnings when doclint is enabled. But, the sets do not > overlap, and the policy has inappropriately suppressed some warnings and > inhibited the generation of new warnings by the doclet that could only be > done by the doclet, and not doclint. > > One notable group of warnings that has been inappropriately suppressed is the > warnings generated by using the `-serial warn` option. > > The fundamental core of the change is to remove the conditional checks in the > doclet `Messages.java`, which would suppress messages when doclint was > enabled. A consequence is that some specific messages have to disabled if > they are duplicate checks if doclint is enabled. (The messages cannot be > removed altogether because doclint might _not_ be enabled.) A test is added > to verify that either one message or the other is generated, but never both. > > As previously noted, an issue with the earlier policy is that warnings > generated by using the `-serial warn` option were inappropriately suppressed, > and this change fixes that ... revealing a number of latent warnings in the > JDK API that need to be addressed. The short term fix here is to temporarily > remove the use of the `-serialwarn` option. See > [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749). > Since the introduction of doclint, there was some (small) overlap between the > small set ... and the new larger set ... But, the sets do not overlap Do or do they not overlap? :) > the policy has inappropriately suppressed some warnings and inhibited the > generation of new warnings What's the difference between "suppressed" and "inhibited" here? Generally, how can we be sure that we haven't missed any double-reported cases? test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java line 136: > 134: var doclintResult = new Result(Exit.OK, "C.java:8: warning: > @param \"x\" has already been specified"); > 135: var docletResult = new Result(Exit.OK, "C.java:8: warning: > Parameter \"x\" is documented more than once."); > 136: Both are warnings, hence two `Exit.OK` results; I see. test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java line 190: > 188: checkExit(expect.exit); > 189: > 190: checkOutput(Output.OUT, true,expect.message); Add whitespace after `true,` - PR: https://git.openjdk.java.net/jdk/pull/9006
RE: Warning about git from 'make test' on Windows
Hi, I see the same on my windows build. I verified that 8.3 filenames are enabled. I took a little closer look. In my PATH env in Cygwin I have: ...:/cygdrive/c/Program Files/Git/cmd:... The git path is resolved via UTIL_LOOKUP_PROGS(GIT, git) in basic_tools.m4. UTIL_LOOKUP_PROGS, defined in util_paths.m4 must somehow not be capable to correctly resolve the path with spaces. The output in configure is: checking for git... Files/Git/cmd/git Maybe this gives a hint on how to fix it? I guess I can configure my path to use the 8.3 notation. But maybe UTIL_LOOKUP_PROGS could be improved? Cheers Christoph > -Original Message- > From: build-dev On Behalf Of Magnus Ihse > Bursie > Sent: Freitag, 3. Juni 2022 01:10 > To: Andrey Turbanov ; build-dev@openjdk.java.net > Subject: Re: Warning about git from 'make test' on Windows > > > On 2022-06-02 21:26, Andrey Turbanov wrote: > > Hello. > > I noticed strange warnings produced by 'make test' recently on my > > Win11 installation. For example: > > > > $ make test TEST="tier1" > > Building target 'test' in configuration 'windows-x86_64-server-release' > > /usr/bin/bash: Files/Git/cmd/git: No such file or directory > > /usr/bin/bash: Files/Git/cmd/git: No such file or directory Test > > selection 'tier1', will run: > > > > > > Also found this in > > build\windows-x86_64-server-release\configure-support\config.log > > > > configure:29274: checking for git > > configure:29502: result: Files/Git/cmd/git > > > > Seems like space in git path is incorrectly handled. > > Is it a known issue or is it a limitation of autoconf? > Spaces in paths is always tricky and is best avoided. That being said, we try > to > make autoconf robust for tools in directories with space in them. > > Are you running in Cygwin? > > Can you verify that your C: drive can use short paths. See > https://openjdk.java.net/groups/build/doc/building.html#spaces-in-path > > /Magnus > > > > Andrey Turbanov
