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