RFR: 7115200

[Robert Ottenhag]

Rickard,

as discussed with you off-line, I see that you were only following 
existing patterns, which IMHO is a good thing and simplifies further 
maintenance.

So let's go with the with the changes in 7115200.u2.

Thanks

/Robert

On 01/20/2012 08:46 AM, Rickard Bäckman wrote:
> Inline.
>
> On 01/19/2012 11:00 PM, Kelly O'Hair wrote:
>> I tend to agree with Robert.
>>
>> -kto
>>
>> On Jan 19, 2012, at 9:14 AM, Robert Ottenhag wrote:
>>
>>> Rickard,
>>>
>>> * make/com/oracle/Makefile:
>>> - just skip the separate JFR variable and use "SUBDIRS += jfr" to 
>>> add to the previously defined SUBDIRS variable, or if necessary 
>>> "SUBDIRS = jfr $(SUBDIRS)" if it is necessary to build it first, 
>>> which I doubt.
>>>
>>> + ifndef OPENJDK
>>> +   ifndef JAVASE_EMBEDDED
>>> +     SUBDIRS += jfr
>>> +   endif
>>> + endif
>
> Sure, I just tried to follow the existing conventions of the file. 
> I've changed it to be SUBDIRS += jfr.
>
>>>
>>> * make/common/Defs.gmk:
>>> - skip VPATH0.h and prepend it using
>>>
>>> + ifndef OPENJDK
>>> +   VPATH.h = 
>>> $(CLOSED_SHARE_SRC)/javavm/export$(CLASSPATH_SEPARATOR)$(VPATH.h)
>>> + endif
>>>
>
> Once again I tried to reuse existing patterns (VPATH.java). Using the 
> suggested pattern also fails as makefile variables are "lazy", so 
> rather than prepending you are creating an infinite loop.
>
> /R
>
>>>
>>> /Robert
>>>
>>>
>>> On 01/19/2012 11:03 AM, Rickard Bäckman wrote:
>>>> David,
>>>>
>>>> thanks for the review.
>>>> I've tested with JAVASE_EMBEDDED=true and there is no jfr.jar in 
>>>> j2sdk-image/jre/lib, the same is true with OPENJDK=true. Running 
>>>> without those creates the jfr.jar in that directory.
>>>>
>>>> I added another set of ifndef's to get rid of warnings.
>>>> The change is here: 
>>>> http://http://cr.openjdk.java.net/~rbackman/7115200.u2/webrev/
>>>>
>>>> Thanks
>>>> /R
>>>>
>>>> On 01/19/2012 01:24 AM, David Holmes wrote:
>>>>> Rickard,
>>>>>
>>>>> This looks okay to me (have you tested setting JAVASE_EMBEDDED?)
>>>>>
>>>>> For the record once 7130909 is pushed I'll be looking at moving 
>>>>> the JFR
>>>>> build rules, and the SE Embedded build rules out of the Open 
>>>>> repository.
>>>>>
>>>>> David
>>>>>
>>>>> On 18/01/2012 6:48 PM, Rickard Bäckman wrote:
>>>>>> Please review the updated webrev, David Holmes pointed out that we
>>>>>> shouldn't build JFR for the embedded environments.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/7115200.u1/webrev/
>>>>>>
>>>>>> Thanks
>>>>>> Rickard
>>>>>>
>>>>>> On 01/17/2012 04:26 PM, Rickard Bäckman wrote:
>>>>>>> CR7115200: Add Java FlightRecorder phase 1
>>>>>>> Makefile changes to enable builds of the JDK with JFR.
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/7115200/webrev/
>>>>>>> CR: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7115200
>>>>>>>
>>>>>>> Thanks
>>>>>>> Rickard
>>>>>>
>>>>
>>>
>>>
>>> -- 
>>> Oracle
>>> Robert Ottenhag | Senior Member of Technical Staff
>>> Phone: +46850630961 | Fax: +46850630911 | Mobile: +46707106161
>>> Oracle Java HotSpot Virtual Machine
>>> ORACLE Sweden | Folkungagatan 122 | SE-116 30 Stockholm
>>>
>>> Oracle Svenska AB, Kronborgsgränd 17, S-164 28 KISTA, reg.no. 
>>> 556254-6746
>>>
>>> Green Oracle
>>>
>>> Oracle is committed to developing practices and products that help 
>>> protect the environment
>>> -- 
>>>
>>
>


-- 
Oracle
Robert Ottenhag | Senior Member of Technical Staff
Phone: +46850630961 | Fax: +46850630911 | Mobile: +46707106161
Oracle Java HotSpot Virtual Machine
ORACLE Sweden | Folkungagatan 122 | SE-116 30 Stockholm

Oracle Svenska AB, Kronborgsgränd 17, S-164 28 KISTA, reg.no. 556254-6746

Green Oracle

Oracle is committed to developing practices and products that help protect the environment
--