URGENT code review request for Solaris FDS fix (7175255)

[Daniel D. Daugherty]

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
>>>>>
>>
>