RFR (S): 8007639: Workaround for ccache in vm.make is incorrect

[Erik Joelsson]

Looks good to me.

/Erik

On 2013-02-08 19:30, Mikael Vidstedt wrote:
>
> Please review the following change:
>
> Webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8007639/webrev.00/webrev
> Bug: http://bugs.sun.com/view_bug.do?bug_id=8007639
>
> This change corrects the workaround that was introduced in vm.make for 
> enabling ccache for HotSpot builds. The change introduces a file 
> specific makefile variable (CFLAGS/<file.o>) which is used to only set 
> the JRE_RELEASE_VERSION define when compiling vm_version.o.
>
> Verified manually by observing command lines with/without fix, also 
> passes JPRT.
>
>
> Some additional background below, more information is available in the 
> bug report:
>
> To enable the use of ccache 7132779 [1] introduced some new makefile 
> logic in vm.make to only pass the JRE_RELEASE_VERSION define when 
> compiling the vm_version.cpp file. The underlying reason for that is 
> that the JRE_RELEASE_VERSION can in some cases contain a time stamp 
> and since ccache checks that the defines match before reusing the 
> cache version of the object file that would mean that if the time 
> stamp changed all files would have to be recompiled. With the fix only 
> vm_version.cpp needs to be recompiled.
>
> This almost works, but the logic introduced in the makefile is 
> actually incorrect. Today it looks like this:
>
>
> # This is VERY important! The version define must only be supplied to 
> vm_version.o
> # If not, ccache will not re-use the cache at all, since the version 
> string might contain
> # a time and date.
> vm_version.o: CXXFLAGS += ${JRE_VERSION}
>
> However, the above syntax does not only add the ${JRE_VERSION} to the 
> CXXFLAGS of vm_version.o as originally intended - it also /in some 
> cases/ adds it to the CXXFLAGS for any and all prerequisites of 
> vm_version.o. And vm_version.o depends on all other object files, 
> which means in theory this actually passes in that potentially time 
> stamped version string define to all object files anyway. For various 
> reasons in reality it only passes it to files that are 
> lexicographically after vm_version.o - see bug report for more 
> background on why - but there's still a handful of files that will be 
> recompiled unnecessarily.
>
> Cheers,
> Mikael
>
> [1] http://bugs.sun.com/view_bug.do?bug_id=7132779
>