8294368: Java incremental builds broken on Windows after JDK-8293116

[JornVernee]

This patch fixes incremental builds on Windows.

There are 2 parts to this:

1. the build system needs to run the paths in the modified file list through fixpath. I've added a convert mode to fixpath.sh for that. There's an extra target for generating the file with fixed paths. On non-windows platforms this is just a simple cp of the file.
2. the dependency plugin of javac was using string-based path comparison. But, the paths fed by the build system and the paths used internally by javac could be in slightly different formats, meaning that files were not detected properly as changed. I switched to Path-based comparison instead and that fixes the issue.

Testing:
tested this manually by doing the following:

1. make clean
2. make images
3. put garbage in one of the files in java.base
4. make images (incremental)
5. verify that the build reported an error
6. verify the contents of <build>\jdk\modules\java.base\_the.java.base_batch.modfiles.fixed
7. revert the changes of 3, and do the same for another file
8. make images (incremental)
9. verify that the build reported an error
10. verify the contents of <build>\jdk\modules\java.base\_the.java.base_batch.modfiles.fixed
11. remove garbage from file modified by 9 again
12. make images (incremental)
13. verify that build succeeds as in 2

I've tested the build on Windows and Linux (WSL) using the above steps.

---

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-8294368: Java incremental builds broken on Windows after JDK-8293116

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)
• Daniel Jeliński (@djelinski - Committer)
• Jan Lahoda (@lahodaj - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10560

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

Using diff file

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

[bridgekeeper]

👋 Welcome back jvernee! 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]

@JornVernee The following labels will be automatically applied to this pull request:

• build
• compiler

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

[erikj79]

This looks like a good fix. I would probably have done it slightly differently to avoid the redundant copy when it's not needed, but I think this is good enough. Thanks for fixing it!

[JornVernee]

Thanks for the review, I've address your comments (35cc226).

WRT the redundant copy. I wanted to avoid the use site having to have some if that checks whether we're running on Windows to decide which file $1_COMPILE_TARGET should depend on. Copying allows always depending on the fixed file.

[mlbridge]

Webrevs

• 00: Full (35cc2269)

[erikj79]

WRT the redundant copy. I wanted to avoid the use site having to have some if that checks whether we're running on Windows to decide which file $1_COMPILE_TARGET should depend on. Copying allows always depending on the fixed file.

You are indeed correct and having a single code path is also a desirable attribute. Had this been in a more performance sensitive location, I would have still preferred to avoid the copy, but I think once per Java compilation unit is rare enough to not be an issue, especially since it's just unnecessary on non-windows.

[openjdk]

@JornVernee 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:

8294368: Java incremental builds broken on Windows after JDK-8293116

Reviewed-by: erikj, djelinski, jlahoda

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 11 new commits pushed to the master branch:

• 4bdd1c9: 8290964: C2 compilation fails with assert "non-reduction loop contains reduction nodes"
• b4e74ae: 8294514: Wrong initialization of nmethod::_consts_offset for native nmethods
• 953ce8d: 8293701: jdeps InverseDepsAnalyzer runs into NoSuchElementException: No value present
• be82cff: 8294748: Cleanup unneeded references to hg
• 43dbf58: 8186765: Speed up test sun/net/www/protocol/https/HttpsClient/ProxyAuthTest.java
• 755958e: 8294376: Minimize disabled warnings in java.base
• 1dafbe3: 8294539: Augment discussion of equivalence relations on floating-point values
• b2e86a6: 8294255: Add link to DEFAULT_WAIT_TIME in javadoc for SunToolKit.realsSync
• b22a38d: 8292309: Fix java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java test
• 121d4a5: 8293579: tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java fails on Japanese Windows platform
• ... and 1 more: https://git.openjdk.org/jdk/compare/3644e26cef71c00e1a2638d2b8bed9c1bda965ca...master

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.

[djelinski]

Also verified that:

• changes to public interface trigger full recompilation
• changes that do not affect public interface cause minimal recompilation.

This was on Cygwin. LGTM!

[lahodaj]

Thanks a lot for fixing this, and sorry for trouble!

[JornVernee]

/integrate

[openjdk]

Going to push as commit 8ebebbc.
Since your change was applied there have been 11 commits pushed to the master branch:

• 4bdd1c9: 8290964: C2 compilation fails with assert "non-reduction loop contains reduction nodes"
• b4e74ae: 8294514: Wrong initialization of nmethod::_consts_offset for native nmethods
• 953ce8d: 8293701: jdeps InverseDepsAnalyzer runs into NoSuchElementException: No value present
• be82cff: 8294748: Cleanup unneeded references to hg
• 43dbf58: 8186765: Speed up test sun/net/www/protocol/https/HttpsClient/ProxyAuthTest.java
• 755958e: 8294376: Minimize disabled warnings in java.base
• 1dafbe3: 8294539: Augment discussion of equivalence relations on floating-point values
• b2e86a6: 8294255: Add link to DEFAULT_WAIT_TIME in javadoc for SunToolKit.realsSync
• b22a38d: 8292309: Fix java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java test
• 121d4a5: 8293579: tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java fails on Japanese Windows platform
• ... and 1 more: https://git.openjdk.org/jdk/compare/3644e26cef71c00e1a2638d2b8bed9c1bda965ca...master

Your commit was automatically rebased without conflicts.

[openjdk]

@JornVernee Pushed as commit 8ebebbc.

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