<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#CCCCCC" text="#000000">
    Ok, thanks!<br>
    <br>
    Thumb up.<br>
    <br>
    Thanks,<br>
    Serguei<br>
    <br>
    On 3/17/12 6:29 PM, Daniel D. Daugherty wrote:
    <blockquote cite="mid:4F653A96.7030805@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <tt>Thanks for the review. Replies embedded below...<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:4F642B43.9050100@oracle.com" type="cite">Dan,<br>
        <br>
        I've reviewed this:<br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edcubed/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>
         
        <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>
      </blockquote>
      <tt><br>
        Yup! I called that out in the 'hotspot' repo change summary:<br>
        <br>
        <blockquote type="cite">    - On Solaris, also fixes an
          incorrect 64-bit libjvm_db_g symlink <br>
                and an incorrect 64-bit libjvm_dtrace_g symlink </blockquote>
        <br>
        However, right after these changes go in, I'll be removing all<br>
        the '_g' support via:<br>
        <br>
            7153050 4/4 remove crufty '_g' support from HotSpot repo<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:4F642B43.9050100@oracle.com" type="cite"> <br>
        Wrong indent:<br>
        <br>
          <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>
      </blockquote>
      <tt><br>
        Nicely spotted! I'll fix that.<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:4F642B43.9050100@oracle.com" type="cite">
        <pre><span class="changed">
</span></pre>
        Question: Just curious why the <span class="removed">$(ALT_STRIP_POLICY)</span> 
        is decommissioned?<br>
        <br>
         
        <meta http-equiv="content-type" content="text/html;
          charset=ISO-8859-1">
        <b>make/linux/makefiles/defs.make</b><br>
        <b>  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>
      </blockquote>
      <tt><br>
        I figured out a different way to do it. If you invoke like so:<br>
        <br>
            gnumake STRIP_POLICY=no_strip ...<br>
        <br>
        then that works. Also, when I looked around ALT_* variables tend<br>
        to be used with paths.<br>
        <br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:4F642B43.9050100@oracle.com" type="cite">
        The fix is good in general.<br>
      </blockquote>
      <tt><br>
        Thanks!<br>
        <br>
        Dan<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:4F642B43.9050100@oracle.com" type="cite"> <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>
              7102323 4/4 RFE: enable Full Debug Symbols Phase 1 on
          Solaris <br>
              7136506 3/4 FDS: rework jdk repo Full Debug Symbols
          support <br>
          <br>
          FDS Revamp Summary <br>
          <br>
              The build infrastructure that supports the Full Debug
          Symbols (FDS) <br>
              project is being revamped to reduce the default on-disk
          footprint <br>
              along with other improvements. FDS info will have to be
          unzip'ed <br>
              before being usable in the default build config, but the
          zip'ed FDS <br>
              info occupies about 25% of the disk space as the original
          FDS info. <br>
          <br>
              Change summary for the group of fixes: <br>
              - ENABLE_FULL_DEBUG_SYMBOLS build flag controls the Full
          Debug <br>
                Symbols feature; enabled by default
          (ENABLE_FULL_DEBUG_SYMBOLS=1) <br>
              - ZIP_DEBUGINFO_FILES build flag controls the zip'ing of
          "debug info" <br>
                during the build; enabled by default
          (ZIP_DEBUGINFO_FILES=1). <br>
              - FDS is enabled by default for Linux X86/X64, Solaris
          SPARC/SPARC-V9, <br>
                Solaris X86, and Windows X86/X64. <br>
              - HSX developer builds will put debug info into .diz files
          that are <br>
                co-located with the built object, e.g., there will be a
          libjvm.diz <br>
                file right next to libjvm.so. <br>
              - HSX JPRT jobs will also contain .diz files co-located
          with the built <br>
                objects <br>
              - RE promoted bits will include new debuginfo.zip bundles
          that contain <br>
                all the .debuginfo, .diz, .map and/or .pdb files
          generated by the <br>
                various repos that make up the RE promotion. <br>
          <br>
              Notes: FDS is not enabled on Solaris X64 due to a bug in
          gobjcopy. <br>
                     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>
              - add support for exporting .diz (Debug Info Zip) files <br>
              - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag <br>
                (replaces overloaded uses of OBJCOPY variable) <br>
              - add support for ZIP_DEBUGINFO_FILES build flag <br>
              - clean up STRIP_POLICY on Linux and Solaris <br>
              - On Solaris, also fixes an incorrect 64-bit libjvm_db_g
          symlink <br>
                and an incorrect 64-bit libjvm_dtrace_g symlink <br>
              - The Full Debug Symbols feature is now controllable via <br>
                ENABLE_FULL_DEBUG_SYMBOLS and ZIP_DEBUGINFO_FILES on
          Windows. <br>
              - On Windows, fixed a few hardcoded "sawindbg" uses <br>
          <br>
          'hotspot' repo webrev: <br>
              <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edcubed/fds_revamp/7102323-webrev/1-hotspot-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/</a><br>
          <br>
              The HotSpot changes are relative to the HSX-24-B03
          snapshot plus <br>
              one additional fix and are targeted at
          JDK8-B33/HSX-24-B06. <br>
          <br>
          <br>
          'jdk' repo change summary: <br>
          <br>
              - add support for importing .diz (Debug Info Zip) files <br>
              - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag <br>
              - add support for ZIP_DEBUGINFO_FILES build flag <br>
              - clean up STRIP_POLICY on Linux and Solaris <br>
              - LIBRARY_SUPPORTS_FULL_DEBUG_SYMBOLS is only needed in <br>
                FDS Phase 2 so just a comment for now <br>
              - JPRT needs to use the '-y' option with zip on
          non-Windows <br>
                builds of the jdk repo in order to preserve symbolic
          links <br>
          <br>
          'jdk' repo webrev: <br>
              <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edcubed/fds_revamp/7136506-webrev/1-jdk-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/</a>
          <br>
          <br>
              The JDK changes are relative to the T&L snapshot for
          JDK8-B30 <br>
              and are targeted at JDK8-B33. <br>
          <br>
          <br>
          'root' repo change summary: <br>
          <br>
              - JPRT needs to use the '-y' option with zip on
          non-Windows <br>
                control builds in order to preserve symbolic links <br>
          <br>
          'root' repo webrev: <br>
              <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edcubed/fds_revamp/7136506-webrev/1-root-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/</a><br>
          <br>
              The root changes are relative to the T&L snapshot for
          JDK8-B30 <br>
              and are targeted at JDK8-B33. <br>
          <br>
          Thanks, in advance, for any review comments. <br>
          <br>
          Dan <br>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>