On Tue, 3 Mar 2026 21:30:45 GMT, Erik Joelsson <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one >> additional commit since the last revision: >> >> better but still not working > > make/CompileJavaModules.gmk line 77: > >> 75: # Temporarily restrict this to java.base, but it can be expanded later. >> 76: >> 77: ifeq ($(MODULE), java.base) > > If this is only ever for java.base, then consider putting the whole thing in > `make/modules/java.base/Java.gmk`, along with the ToolsJdk import. If it's > intended to be expanded to a limited set of modules, consider making the > conditional on a suitably named variable that can be set from > `make/modules/<module>/Java.gmk` to activate this feature. If it's intended > to cover all modules eventually, then leaving it here makes sense. Restricting it to java.base is because it's used for every module, there's a build failure for one of the modules. Conceptually it should be applied to all modules which can participate in preview behaviour (which is not all modules, but is more than java.base). I just wanted to set the pieces in the right place, even if I only enable it for java.base for now. > make/CompileJavaModules.gmk line 176: > >> 174: SRC := $(wildcard $(MODULE_PREVIEW_SRC_DIRS)), \ >> 175: INCLUDES := $(JDK_USER_DEFINED_FILTER), \ >> 176: EXTRA_DEPS := $(VALUETYPE_GENSRC_PROCESSOR_DEP), \ > > EXTRA_DEPS looks to be internal in SetupJavaCompilation and not meant to be > used as a parameter. There is a documented parameter `DEPENDS` which seems to > do what you need. Yeah, I wasn't completely clear if DEPENDS or EXTRA_DEPS was the right thing. It wasn't clear that one was internal. The "depends" thing is interesting to me, because there's a conceptual distinction between "the code I'm compiling depends on this library" and "the action I'm performing should be re-performed if this thing was rebuilt". Because the output from the compiler is the same, it's the output of the AP that differs when the processor is recompiled. I don't think this distinction is actually pulled out anywhere, so I guessed at "EXTRA_DEPS" as the right thing because "DEPENDS" feels like library dependencies etc. But they do appear to be used in exactly the same places, so this distinction was always tenuous. I'll make it use DEPENDS unless we find something better. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2180#discussion_r2895249749 PR Review Comment: https://git.openjdk.org/valhalla/pull/2180#discussion_r2895241475
