8295176: some langtools test pollutes source tree

[lahodaj]

The ReleaseOptionThroughAPI.java test compiles another test source, ReleaseOption.java. But it does not specify the output directory, so javac generates the classfile next to the source files into the source directory, causing trouble.

The proposed fix is to add -d . to the test, so that the classfile is not generated into the working directory.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8295176: some langtools test pollutes source tree

Reviewers

• Srikanth Adayapalam (@sadayapalam - Reviewer) ⚠️ Review applies to 8d452c71
• Joe Darcy (@jddarcy - Reviewer)
• Vicente Romero (@vicente-romero-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10724/head:pull/10724
$ git checkout pull/10724

Update a local copy of the PR:
$ git checkout pull/10724
$ git pull https://git.openjdk.org/jdk pull/10724/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10724

View PR using the GUI difftool:
$ git pr show -t 10724

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10724.diff

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj The following label will be automatically applied to this pull request:

• compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (34eb4ed5)
• 00: Full (8d452c71)

[sadayapalam]

Thanks so much for fixing this. It was beginning to irritate me.

The fix is ok, but doing a bit of archaeology, it looks like the fix for JDK-8173605 introduced the problem. JDK-8173605 attempts to remove support for source and target 1.7 option in javac. In so doing, the test ReleaseOption.java
whose objective is to "Verify that javac rejects Java 8 program with --release 7" becomes irrelevant and perhaps the whole test could have been deleted rather than making the negative test (that would fail to compile) into a positive test that compiles fine.

In particular the Summary line in ReleaseOption.java is stale.

It is not clear to me that the test serves any real purpose - compiled directly or through the API.

[sadayapalam]

I confirm that the proposed fix solves the problem.

I suggest a push either after

• cleaning up the Summary of ReleaseOption.java test OR
• deleting both the tests ReleaseOption.java and ReleaseOptionThroughAPI.java

Looks good to me.

[openjdk]

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8295176: some langtools test pollutes source tree

Reviewed-by: sadayapalam, darcy, vromero

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 8 new commits pushed to the master branch:

• ae60599: 8295023: Interpreter(AArch64): Implement -XX:+PrintBytecodeHistogram and -XX:+PrintBytecodePairHistogram options
• 4d37ef2: 8295262: Build binutils out of source tree
• 0919a3a: 8294186: AArch64: VectorMaskToLong failed on SVE2 machine with -XX:UseSVE=1
• ec2981b: 8293711: Factor out size parsing functions from arguments.cpp
• 5d273b9: 8295278: Add parallel class loading tests
• 172006c: 8295333: G1: Remove unnecessary check in G1Policy::calculate_desired_eden_length_by_mmu
• 7743345: 8294314: Minimize disabled warnings in hotspot
• 552d8a2: 8295192: Use original configure command line when called from a script

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[sadayapalam]

The proposed change restoring the negative nature of the test is better than my suggestions. Thanks, Look good to me.

[lahodaj]

Thanks for the reviews!

[lahodaj]

/integrate

[openjdk]

Going to push as commit 71aa821.
Since your change was applied there have been 18 commits pushed to the master branch:

• bca7ab3: 8295412: support latest VS2022 MSC_VER in abstract_vm_version.cpp
• c33ca0c: 6229853: BasicTextAreaUI:create incompletely documents the possible returned View types
• 358ac07: 8294254: [macOS] javax/swing/plaf/aqua/CustomComboBoxFocusTest.java failure
• 490fcd0: 8293833: Error mixing types with -XX:+UseCMoveUnconditionally -XX:+UseVectorCmov
• 529cc48: 8295396: RISC-V: Cleanup useless CompressibleRegions
• 692cdab: 8295016: Make the arraycopy_epilogue signature consistent with its usage
• 21a825e: 8288387: GetLocalXXX/SetLocalXXX spec should require suspending target thread
• 8d751de: 8295231: Move all linking of native libraries to make
• f300ec8: 8294546: document where javac differs when invoked via launcher and ToolProvider
• b269c51: 8295395: Linux Alpha Zero builds fail after JDK-8292591
• ... and 8 more: https://git.openjdk.org/jdk/compare/cf07eaeb9291da725181832b8bb1dc54957ba886...master

Your commit was automatically rebased without conflicts.

[openjdk]

@lahodaj Pushed as commit 71aa821.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.