On Thu, 8 Sep 2022 13:54:02 GMT, Erik Joelsson <er...@openjdk.org> wrote:
>> I looked up what kind of dependencies we are passing in through `DEPEND`. >> None, it turns out. It is not used. So that is not a worry. We might >> consider removing it. Sending in exceptional dependencies like that is never >> a good design decision. If it ever happens again that this would be the only >> acceptable solution, we can re-add the `DEPEND` argument. >> >> But that basically mean that Jan's patch might be okay, after all. `DEPENDS` >> does not exist, and I want to remove it. `EXTRA_DEPS` is only modified if we >> change the `depend` javac plugin. I am willing to sacrifice automatic >> recompilation of all Java classes if that is changed. > > Ok I was wrong about what EXTRA_DEPS was used for, I read the code too > hastily. However, the dependency declaration for the pubapi files from other > modules is added in make/CompileJavaModules.gmk like this: > > > $($(MODULE)_COMPILE_TARGET): $(foreach d, $(call FindDepsForModule, > $(MODULE)), \ > $(call SetupJavaCompilationApiTarget, $d, \ > $(if $($d_BIN), $($d_BIN), $(JDK_OUTPUTDIR)/modules/$d))) > > > So this still doesn't change my main point. The pubapi files only appear on > the dependency declaration for `$($(MODULE)_COMPILE_TARGET)`. This means that > with the current patch, we will not recompile the full module when a parent > module has a change to the pubapi. The Depend plugin needs to see the pubapi > files in `$$?` for that to happen. > > One could also make the case for using DEPEND when adding such a dependency > instead of referencing `$($(MODULE)_COMPILE_TARGET)` in explicit rules. It > would be more inline with more recent design patterns. @erikj79 I agree that setting DEPEND would be better. I hate those weird "injection attacks" where we fake arguments to functions, they make the code hopeless to understand (and I'd like to forbid them in `NamedParamsMacroTemplate`, but I can't just now since we still use it in so many places). But can we really do that? I don't think we have that dependency information available when setting up the module java compilation. (In any case, that is a separate cleanup) ------------- PR: https://git.openjdk.org/jdk/pull/10104