On Tue, 4 Oct 2022 16:19:24 GMT, Jorn Vernee <jver...@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
