URGENT code review request for Solaris FDS fix (7175255)

[Daniel D. Daugherty]

Thanks for the quick review!


On 6/19/12 8:06 PM, David Holmes wrote:
> Hi Dan,
>
> 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.


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

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