RFR: 8294368: Java incremental builds broken on Windows after JDK-8293116
Erik Joelsson
erikj at openjdk.org
Tue Oct 4 17:33:10 UTC 2022
On Tue, 4 Oct 2022 16:19:24 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
> 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.
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!
make/common/JavaCompilation.gmk line 473:
> 471: $$(eval $$(call ListPathsSafely, $1_MODFILES, $$($1_MODFILELIST)))
> 472:
> 473: $$($1_MODFILELIST_FIXED): $$($1_MODFILELIST)
Could you add a comment about why this is needed?
make/common/MakeBase.gmk line 449:
> 447: FixPath = \
> 448: $(strip $(subst \,\\, $(shell $(FIXPATH_BASE) print $(patsubst $(FIXPATH), , $1))))
> 449: FixPathFile = \
Could you add a comment on what FixPathFile does exactly?
make/scripts/fixpath.sh line 361:
> 359: outfile="$2"
> 360: if [[ -e $outfile ]] ; then
> 361: rm $outfile
Please try to match indent length with the rest of the file.
-------------
PR: https://git.openjdk.org/jdk/pull/10560
More information about the compiler-dev
mailing list