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

[Mikael Vidstedt]

Erik/David - Thanks for the reviews!

Cheers,
Mikael

On 2/12/2013 6:35 PM, David Holmes wrote:
> 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
>>>>>
>>