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