RFR: JDK-8069261: Create make dependencies on make variable values

[Magnus Ihse Bursie]

On 2015-01-19 14:33, Erik Joelsson wrote:
> Hello,
>
> Please review this enhancement to the incremental build correctness. I 
> have implemented a way to track make dependencies to make variable 
> values, so that relevant changes to makefiles or configure will 
> properly trigger rebuilds of affected targets. See bug description for 
> more details.
>
> Note that I've included new tests for these makefile features in the 
> file test/make/TestMakeBase.gmk. These can be run with the target 
> "test-make".
>
> The macro MakeDir has been changed so that it no longer needs to be 
> called with $(eval ).
>
> I have chosen to define the new macros with = assignment instead of 
> using "define", because they are meant to be called without $(eval ), 
> which means they can only be one logical line. Since "define" is used 
> to define multi line variables, it makes no sense to use it for these 
> macros.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8069261
> Webrev: http://cr.openjdk.java.net/~erikj/8069261/webrev.01/

Erik,

Thanks for addressing this!

The patch looks overall good. Lots of karma for using the test framework! :)

Some minor comments.
The name standard of the new vardeps files seems a bit vague. :) I 
suggest the following:

* In JavaCompilation:
   $1_VARDEPS_FILE := $$(call DependOnVariable, $1_VARDEPS, $$(dir 
$$($1_JAR))_the.$$($1_JARNAME)_vardeps)
Use .vardeps instead

* In MakeBase:
DependOnVariableFileName = \
     $(strip $(if $(strip $2), $2, \
$(MAKESUPPORT_OUTPUTDIR)/vardeps/$(DependOnVariableDirName)/$(strip 
$1).value))
Use .vardeps instead

* Also, in Tools.gmk the additions:
     CFLAGS_windows := -nologo, \
seems to have crept in by mistake.

* In NativeCompilation, the line:
  $$($1_RES): $$($1_VERSIONINFO_RESOURCE) $$($1_RES_VARDEPS_FILE)
is mis-indented

* Perhaps you can add some comment clarifying that $1_COMPILE_VARDEPS 
contains exactly the variables that add_native_source depend on,
and that $1_COMPILE_VARDEPS_FILE is used by add_native_source?

Otherwise, it looks all very good!

/Magnus