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

Erik Joelsson erik.joelsson at oracle.com
Fri Jan 23 11:17:41 UTC 2015


Hello,

Thanks for the comments. Here is a new version:
http://cr.openjdk.java.net/~erikj/8069261/webrev.02/

In addition to fixing the concerns below, I fixed the following:

* Added test-make to the all target so that it gets tested in JPRT. It 
takes close to no time to run.

* Added 1 second sleeps in the tests when running on macosx, since the 
filesystem on mac only has 1 second resolution. (WOW!)

* Changed WriteFile to use printf instead of echo since echo on Solaris 
interprets things like '\t', which messes up comparisons.

* Reworked how DependOnVariable is used in JavaCompilation.gmk. Instead 
of just blindly adding all input variables, I moved the definitions 
closer to the recipes, like in NativeCompilation.gmk. I think this is 
generally better, but the triggering reason was that too much unneeded 
stuff got into the vardep variable value and the command line got too 
long on Windows.

* For SetupArchive, I discovered that for vardeps changes, we have to 
force a full recreation of the jar to be sure those changes are really 
picked up. I also discovered that we currently didn't rebuild a jar if 
the manifest template file was changed. (I'm happy I had test-make tests 
for SetupArchive when fixing this.)

/Erik

On 2015-01-21 15:42, Magnus Ihse Bursie wrote:
>
> 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




More information about the build-dev mailing list