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

[David Holmes]

Looks okay to me.

David

On 13/02/2013 12:32 PM, Mikael Vidstedt wrote:
>
> Good catch! There is no fundamental reason for switching to CFLAGS, and
> since the other defines are added to CXXFLAGS so should this. Updated
> webrev here:
>
> http://cr.openjdk.java.net/~mikael/webrevs/8007639/webrev.01/webrev/
>
> Cheers,
> Mikael
>
> On 2013-02-12 18:02, David Holmes wrote:
>> Oops! Hit reply instead of reply-all. Adding back the lists.
>>
>> On 12/02/2013 11:36 AM, David Holmes wrote:
>>> Hi Mikael,
>>>
>>> So after our side-bar discussions on other ways to tackle this, the only
>>> query I have now is why the original change adjusted CXXFLAGS and this
>>> change adjusts CFLAGS instead?
>>>
>>> Thanks,
>>> David
>>>
>>> On 9/02/2013 4:30 AM, 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
>>>>
>