<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
<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>
</body>
</html>