<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#CCCCCC" text="#000000">
    Dan,<br>
    <br>
    I've reviewed this:<br>
    &nbsp;
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/</a><br>
    <br>
    <br>
    <br>
    <br>
    Wow, you fixed two existing bugs in the make file:<br>
    <br>
    &nbsp;
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <b>make/solaris/makefiles/dtrace.make<br>
      <br>
    </b>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <pre><span class="removed">-        [ -f $(XLIBJVM_DB_G_DEBUGINFO) ] || { ln -s $(LIBJVM_DB_DEBUGINFO) $(XLIBJVM_DB_G_DEBUGINFO); }</span>
<span class="new">+        [ -f $(XLIBJVM_DB_G_DEBUGINFO) ] || { ln -s $(XLIBJVM_DB_DEBUGINFO) $(XLIBJVM_DB_G_DEBUGINFO); }</span></pre>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <pre><span class="removed">-        [ -f $(XLIBJVM_DTRACE_G_DEBUGINFO) ] || { ln -s $(LIBJVM_DTRACE_DEBUGINFO) $(XLIBJVM_DTRACE_G_DEBUGINFO); }</span>
<span class="new">+        [ -f $(XLIBJVM_DTRACE_G_DEBUGINFO) ] || { ln -s $(XLIBJVM_DTRACE_DEBUGINFO) $(XLIBJVM_DTRACE_G_DEBUGINFO); }</span></pre>
    <br>
    <br>
    Wrong indent:<br>
    <br>
    &nbsp; <b>make/solaris/makefiles/defs.make</b><br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <pre><span class="changed"> 221   ifeq ($(ZIP_DEBUGINFO_FILES),1)</span>
<span class="changed"> 222   EXPORT_LIST += $(EXPORT_JRE_LIB_ARCH_DIR)/libsaproc.diz</span>
<span class="changed"> 223   else

</span></pre>
    Question: Just curious why the <span class="removed">$(ALT_STRIP_POLICY)</span>&nbsp;
    is decommissioned?<br>
    <br>
    &nbsp;
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <b>make/linux/makefiles/defs.make</b><br>
    <b>&nbsp; make/solaris/makefiles/defs.make</b><br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <pre><span class="removed">-    DEF_STRIP_POLICY="min_strip"</span>
<span class="removed">-    ifeq ($(ALT_STRIP_POLICY),)</span>
<span class="removed">-      STRIP_POLICY=$(DEF_STRIP_POLICY)</span>
<span class="removed">-    else</span>
<span class="removed">-      STRIP_POLICY=$(ALT_STRIP_POLICY)</span>
<span class="removed">-    endif</span>
<span class="new">+    # Currently, STRIP_POLICY is only used when Full Debug Symbols is enabled.</span>
<span class="new">+    #</span>
<span class="new">+    STRIP_POLICY ?= min_strip</span></pre>
    <br>
    The fix is good in general.<br>
    <br>
    Thanks,<br>
    Serguei<br>
    <br>
    <br>
    <br>
    On 3/16/12 12:58 PM, Daniel D. Daugherty wrote:
    <blockquote cite="mid:4F639B66.8090902@oracle.com" type="cite">Greetings,
      <br>
      <br>
      I need code reviews for some Makefile and packaging changes.
      <br>
      Wait, come back! They're not that scary...
      <br>
      <br>
      These are Full Debug Symbols changes... so maybe they are that
      scary...
      <br>
      <br>
      These changes have gone through two rounds of internal review.
      <br>
      <br>
      The following bugs are being used to revamp the OpenJDK side of
      the
      <br>
      Full Debug Symbols (FDS) implementation:
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; 7102323 4/4 RFE: enable Full Debug Symbols Phase 1 on Solaris
      <br>
      &nbsp;&nbsp;&nbsp; 7136506 3/4 FDS: rework jdk repo Full Debug Symbols support
      <br>
      <br>
      FDS Revamp Summary
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; The build infrastructure that supports the Full Debug Symbols
      (FDS)
      <br>
      &nbsp;&nbsp;&nbsp; project is being revamped to reduce the default on-disk
      footprint
      <br>
      &nbsp;&nbsp;&nbsp; along with other improvements. FDS info will have to be
      unzip'ed
      <br>
      &nbsp;&nbsp;&nbsp; before being usable in the default build config, but the
      zip'ed FDS
      <br>
      &nbsp;&nbsp;&nbsp; info occupies about 25% of the disk space as the original FDS
      info.
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; Change summary for the group of fixes:
      <br>
      &nbsp;&nbsp;&nbsp; - ENABLE_FULL_DEBUG_SYMBOLS build flag controls the Full Debug
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Symbols feature; enabled by default
      (ENABLE_FULL_DEBUG_SYMBOLS=1)
      <br>
      &nbsp;&nbsp;&nbsp; - ZIP_DEBUGINFO_FILES build flag controls the zip'ing of
      "debug info"
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; during the build; enabled by default
      (ZIP_DEBUGINFO_FILES=1).
      <br>
      &nbsp;&nbsp;&nbsp; - FDS is enabled by default for Linux X86/X64, Solaris
      SPARC/SPARC-V9,
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Solaris X86, and Windows X86/X64.
      <br>
      &nbsp;&nbsp;&nbsp; - HSX developer builds will put debug info into .diz files
      that are
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; co-located with the built object, e.g., there will be a
      libjvm.diz
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; file right next to libjvm.so.
      <br>
      &nbsp;&nbsp;&nbsp; - HSX JPRT jobs will also contain .diz files co-located with
      the built
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; objects
      <br>
      &nbsp;&nbsp;&nbsp; - RE promoted bits will include new debuginfo.zip bundles that
      contain
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; all the .debuginfo, .diz, .map and/or .pdb files generated
      by the
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; various repos that make up the RE promotion.
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; Notes: FDS is not enabled on Solaris X64 due to a bug in
      gobjcopy.
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; FDS has not yet been implemented on MacOS X.
      <br>
      <br>
      Just like the original FDS changes, the FDS Revamp changes are in
      <br>
      multiple repos:
      <br>
      <br>
      'hotspot' repo change summary:
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; - add support for exporting .diz (Debug Info Zip) files
      <br>
      &nbsp;&nbsp;&nbsp; - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; (replaces overloaded uses of OBJCOPY variable)
      <br>
      &nbsp;&nbsp;&nbsp; - add support for ZIP_DEBUGINFO_FILES build flag
      <br>
      &nbsp;&nbsp;&nbsp; - clean up STRIP_POLICY on Linux and Solaris
      <br>
      &nbsp;&nbsp;&nbsp; - On Solaris, also fixes an incorrect 64-bit libjvm_db_g
      symlink
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; and an incorrect 64-bit libjvm_dtrace_g symlink
      <br>
      &nbsp;&nbsp;&nbsp; - The Full Debug Symbols feature is now controllable via
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; ENABLE_FULL_DEBUG_SYMBOLS and ZIP_DEBUGINFO_FILES on
      Windows.
      <br>
      &nbsp;&nbsp;&nbsp; - On Windows, fixed a few hardcoded "sawindbg" uses
      <br>
      <br>
      'hotspot' repo webrev:
      <br>
      &nbsp;&nbsp;&nbsp;
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/</a><br>
      <br>
      &nbsp;&nbsp;&nbsp; The HotSpot changes are relative to the HSX-24-B03 snapshot
      plus
      <br>
      &nbsp;&nbsp;&nbsp; one additional fix and are targeted at JDK8-B33/HSX-24-B06.
      <br>
      <br>
      <br>
      'jdk' repo change summary:
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; - add support for importing .diz (Debug Info Zip) files
      <br>
      &nbsp;&nbsp;&nbsp; - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
      <br>
      &nbsp;&nbsp;&nbsp; - add support for ZIP_DEBUGINFO_FILES build flag
      <br>
      &nbsp;&nbsp;&nbsp; - clean up STRIP_POLICY on Linux and Solaris
      <br>
      &nbsp;&nbsp;&nbsp; - LIBRARY_SUPPORTS_FULL_DEBUG_SYMBOLS is only needed in
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; FDS Phase 2 so just a comment for now
      <br>
      &nbsp;&nbsp;&nbsp; - JPRT needs to use the '-y' option with zip on non-Windows
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; builds of the jdk repo in order to preserve symbolic links
      <br>
      <br>
      'jdk' repo webrev:
      <br>
      &nbsp;&nbsp;&nbsp;
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/</a>
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; The JDK changes are relative to the T&amp;L snapshot for
      JDK8-B30
      <br>
      &nbsp;&nbsp;&nbsp; and are targeted at JDK8-B33.
      <br>
      <br>
      <br>
      'root' repo change summary:
      <br>
      <br>
      &nbsp;&nbsp;&nbsp; - JPRT needs to use the '-y' option with zip on non-Windows
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; control builds in order to preserve symbolic links
      <br>
      <br>
      'root' repo webrev:
      <br>
      &nbsp;&nbsp;&nbsp;
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/</a><br>
      <br>
      &nbsp;&nbsp;&nbsp; The root changes are relative to the T&amp;L snapshot for
      JDK8-B30
      <br>
      &nbsp;&nbsp;&nbsp; and are targeted at JDK8-B33.
      <br>
      <br>
      Thanks, in advance, for any review comments.
      <br>
      <br>
      Dan
      <br>
    </blockquote>
    <br>
  </body>
</html>