On Thu, 22 Jan 2026 20:51:53 GMT, Erik Joelsson <[email protected]> wrote:
>> make/CompileJavaModules.gmk line 182: >> >>> 180: $(MODULE_OUTPUTDIR)/_the.java.base.valueclasses: >>> $($(MODULE)-$(VALUECLASSES_STR)) >>> 181: $(MKDIR) -p $(@D)/META-INF/preview >>> 182: $(MV) $(TEMP_OUTPUTDIR)/$(MODULE)/*/ >>> $(@D)/META-INF/preview >> >> This could also be a `$(CP) -r` (with slight adjustment), but I'd want to >> still avoid copying the marker files. >> The root module directory shouldn't contain any files we care about, so >> using '*/' suffix just copies directories and ignores marker files. >> >> I *think* that because TARGETS doesn't have the compilation marker added to >> it, once the compilation marker files are created, they only serve to >> support other targets that depend upon them, which is only the valueclasses >> target. >> >> If however, the Java compilation macro does have side effects visible to the >> rest of the build, this might not be a safe assumption. Please let me know >> if you think I've made an error here, and what your preferences are. > > By moving the class files instead of copying them, you are disabling any > incremental build logic provided by javac and the plugins we use when > compiling the value classes. There are most likely other potential issues > that I'm not thinking about right now. In my experience maintaining these > makefiles since 2012, moving files, or deleting "temporary" files in the > build output almost always leads to problems with incremental or interrupted > builds. Please leave them be and copy them to where they need to go. > Excluding the marker files is certainly a good idea. If this was a review for > mainline, I would not accept moving the files, but since this is a project > repo, I can assume this is a work in progress and you are free to experiment > any which way you like. The final review happens when it goes into mainline. > > Yes, it's correct to not add `$($(MODULE)-$(VALUECLASSES_STR))` to `TARGETS` > since it's an intermediate build step. We only need to add targets to > `TARGETS` when they are end targets for the makefile. Okay, cool, I'll switch it to a copy and leave the intermediate marker NOT added to TARGETS. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1949#discussion_r2718608599
