URGENT code review request for Solaris FDS fix (7175255)

Kelly O'Hair kelly.ohair at oracle.com
Wed Jun 20 20:13:14 UTC 2012


Looks fine.

-kto

On Jun 20, 2012, at 12:04 PM, Daniel D. Daugherty wrote:

> Kelly,
> 
> The JPRT job testing the latest HSX-24 version has finished and I
> have identical test results to the original version of this fix.
> The JPRT job testing the latest HSX-23.2 version is still running,
> but so far there are no issues.
> 
> Could you let me know if this latest version resolves your
> code review comments?
> 
> Dan
> 
> 
> 
> On 6/20/12 11:02 AM, Daniel D. Daugherty wrote:
>> 
>> 
>> On 6/20/12 10:44 AM, Daniel D. Daugherty wrote:
>>> On 6/20/12 10:41 AM, Kelly O'Hair wrote:
>>>> You still have repeated patterns like:
>>>>  131         ( set -e ; \
>>>>  132           cd $(XLIBJVM_DIR) ; \
>>>>  133           $(ADD_GNU_DEBUGLINK) $(LIBJVM_DB_DEBUGINFO) $(LIBJVM_DB) ; \
>>>>  134         )
>>>> When
>>>>  131         ( cd $(XLIBJVM_DIR)&&  $(ADD_GNU_DEBUGLINK) $(LIBJVM_DB_DEBUGINFO) $(LIBJVM_DB) )
>>>> does the same thing and is more obvious, no need for the set -e
>>> 
>>> Yes, that's because I didn't hear back from you after my reply from
>>> late last night. The "set -e" is there because you asked me to add
>>> it in a previous FDS fix.
>>> 
>>> I'll switch it.
>> 
>> Fixed. See the following:
>> 
>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7175255-webrev/2/
>> 
>> Only make/solaris/makefiles/dtrace.make has changed in this round...
>> 
>> Dan
>> 
>> 
>>> 
>>> Dan
>>> 
>>> 
>>>> -kto
>>>> 
>>>> On Jun 20, 2012, at 9:23 AM, Daniel D. Daugherty wrote:
>>>> 
>>>>> Greetings,
>>>>> 
>>>>> I've updated the fix to (hopefully) address Kelly's and David H's
>>>>> concerns. Here is the URL for code review round 1:
>>>>> 
>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7175255-webrev/1/
>>>>> 
>>>>> Brief summary of changes relative to code review round 0:
>>>>> 
>>>>> - removed definition of GENERATED from both add_gnu_debuglink.make
>>>>>  and fix_empty_sec_hdr_flags.make; this will decouple these work
>>>>>  around Makefiles from the regular HotSpot Makefiles that define
>>>>>  the GENERATED macro. No, I'm not cleaning up that mess. :-)
>>>>> - add new "XLIBJVM_DIR = 64" variable and change all uses of a
>>>>>  literal "64" in dtrace.make to the new variable
>>>>> - drop uses of $(QUIETLY) in sub-shell constructs; I don't think
>>>>>  $(QUIETLY) works in sub-shells anyway, but my memory is fuzzy
>>>>>  there
>>>>> 
>>>>> Test JPRT jobs for HSX-24 and HSX-23.2 are in flight right now.
>>>>> 
>>>>> Thanks, in advance, for any reviews!
>>>>> 
>>>>> Dan
>>>>> 
>>>>> 
>>>>> On 6/19/12 7:21 PM, Daniel D. Daugherty wrote:
>>>>>> Greetings,
>>>>>> 
>>>>>> This is an URGENT code review request for a Solaris specific Full Debug
>>>>>> Symbols (FDS) fix. Due to a Makefile logic error, the full debug symbol
>>>>>> files and related '_g' symlinks are created in the wrong sub-directory
>>>>>> for a couple of the dtrace libraries. The incorrect paths have a double
>>>>>> "64/" sub-directory, e.g.:
>>>>>> 
>>>>>> solaris-<arch>/jre/lib/<arch>/client/64/64/libjvm_db.debuginfo
>>>>>> 
>>>>>> These are the correct symlink paths:
>>>>>> 
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/client/64/libjvm_g_db.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/client/64/libjvm_g_dtrace.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/server/64/libjvm_g_db.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/server/64/libjvm_g_dtrace.debuginfo
>>>>>> 
>>>>>> and these are the correct debug info file paths:
>>>>>> 
>>>>>>    solaris-<arch>/jre/lib/<arch>/client/64/libjvm_db.debuginfo
>>>>>>    solaris-<arch>/jre/lib/<arch>/client/64/libjvm_dtrace.debuginfo
>>>>>>    solaris-<arch>/jre/lib/<arch>/server/64/libjvm_db.debuginfo
>>>>>>    solaris-<arch>/jre/lib/<arch>/server/64/libjvm_dtrace.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/client/64/libjvm_db.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/client/64/libjvm_dtrace.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/server/64/libjvm_db.debuginfo
>>>>>>    solaris-<arch>/fastdebug/jre/lib/<arch>/server/64/libjvm_dtrace.debuginfo
>>>>>> 
>>>>>> where "<arch>" is "i586" or "sparc". The 64-bit Solaris platforms ("amd64"
>>>>>> and "sparcv9") don't have this issue because they don't have the "64/"
>>>>>> sub-directories.
>>>>>> 
>>>>>> This fix is targeted at HSX-24/JDK8 and HSX-23.2/JDK7u6 and will resolve
>>>>>> an issue that is preventing Oracle's Release Engineering scripts from
>>>>>> running properly.
>>>>>> 
>>>>>> Here is the webrev URL for the HSX-24/JDK8 version:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7175255-webrev/0/
>>>>>> 
>>>>>> The HSX23.3/JDK7u6 version is the same except for the changes to
>>>>>> make/solaris/makefiles/defs.make which are not needed in HSX23.2.
>>>>>> 
>>>>>> Thanks, in advance, for any reviews!
>>>>>> 
>>>>>> Dan
>>>>>> 
>>> 
>> 




More information about the build-dev mailing list