On 20/06/2012 1:23 PM, Daniel D. Daugherty wrote:
Thanks for the quick review!
You said it was urgent ;-)
On 6/19/12 8:06 PM, David Holmes wrote:
It would be nice if the cd into the 64 directory could be handled
internally to the link logic rather than occurring at the top-level (I
say this as someone who will need to hand merge this into another
workspace ;-) ).
I'm not quite sure what you mean by this: "the cd into the 64 directory
could be handled internally to the link logic rather than occurring at
the top-level".
This isn't "at the top-level". It's in the dtrace.make subsidiary
Makefile and it only happens for the context of the subshell.
The problem is that ADD_GNU_DEBUGLINK and the ZIP_EXE need to be
done in the 64 directory, but nothing else in that crazy Makefile
logic does need to be done in the 64 directory.
I "simply" meant doing the "cd 64" inside the ADD_GNU_DEBUGLINK logic
rather than in the makefile that invokes that logic. That said I can
live with Kelly's shorter form.
Also in make/solaris/makefiles/add_gnu_debuglink.make I don't
understand the logic change:
GENERATED = ../generated
becomes
TOPDIR = $(shell echo `pwd`)
GENERATED = $(TOPDIR)/../generated
but at what time is "pwd" evaluated? If we have: /out/lib/64 and
originally we started in lib then GENERATED==lib/../generated ie
out/generated. If we have now done a cd into 64 then: pwd =
/out/lib/64 and so GENERATED==/out/lib/64/../generated ie
/out/lib/generated. I may well be missing something but this doesn't
seem right.
Please see the very long answer that I gave Kelly about the GENERATED
variable and HotSpot Makefiles. In particular, the TOPDIR followed
by the setting of GENERATED is used in the Linux and BSD HotSpot
makefiles (most of them anyway)...
I'll wait to see what you can clarify about the "top-level" comment
above before I try to resolve this any further...
I read the long answer and yes it is complex. As I recently had to
untangle this for other reasons you (ie the hotspot developer cursed to
modify the build system) need to be aware that we effectively have three
levels of "make" invocations when building hotspot:
- "top level" Makefile or <os>/Makefile
- buildtree.make sub-make
- make of the makefiles generated by buildtree.make
determining which files get included at each level, and which variables
get passed through the submakes is complex. Lazy evaluation of variables
adds to the complexity.
That all said, it still seems to me that the logic for GENERATED is not
correct if you have done a cd into the 64 subdirectory.
Cheers,
David
Dan
David
-----
On 20/06/2012 11:21 AM, 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
