code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

Thanks for the re-review!


On 10/13/13 7:57 PM, David Holmes wrote:
> Hi Dan,
>
> Only further comment I have, and it may well be deferred for future 
> work, is that we should be able to abstract away the actual 
> "extension" used for the "debuginfo" file so that we don't need macosx 
> conditionals as much eg one:
>
> if macosx
>  DEBUGINFO_EXTENSION=dSYM
> else
>  DEBUGINFO_EXTENSION=debuginfo
> endif
>
> then eg:
>
> JSIG_DEBUGINFO := $(strip $(wildcard 
> $(HOTSPOT_DIST)/jre/lib$(OPENJDK_TARGET_CPU_LIBDIR)/libjsig.$DEBUGINFO_EXTENSION) 
> \
>                     $(wildcard 
> $(HOTSPOT_DIST)/jre/lib$(OPENJDK_TARGET_CPU_LIBDIR)/libjsig.diz) )

The biggest worry that I have about something like DEBUGINFO_EXTENSION
is that on MacOS X the '.dSYM' objects are directories instead of files.
This requires that different cmd options be used in many/most/all of the
cases. More thought would be needed.


> Hmm second comment - I don't see a .m4 file change that corresponds to 
> the DSYMUTIL configure change ??

Yikes! I'll check into that shortly. Not sure what happened here.

Dan


>
> Thanks,
> David
>
> On 12/10/2013 6:27 AM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I'm ready for code review round 1 of the FDS on MacOS X hotspot changes.
>> Below is the original code review round 0 invite (slightly edited for
>> clarity). Working on FDS is like pulling a thread on a sweater... so
>> there are four additional changed files along with more changes to
>> many of the files from round 0.
>>
>> Here is the code review round 1 JDK8/HSX-25 webrev URL:
>>
>> OpenJDK:
>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
>> Internal:
>> http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/
>>
>> Changes from code review round 0 to round 1:
>>
>>    root repo:
>>      - add configure support for DSYMUTIL macro
>>
>>    hotspot repo:
>>      - drop Minimal1 config support for FDS on MacOS X
>>      - use DSYMUTIL macro instead of literal 'dsymutil'
>>      - fix FASTDEBUG_CFLAGS typo and add USE_CLANG support
>>      - STRIP is not used on MacOS X
>>      - fix $(UNIVERSAL_COPY_LIST) rule
>>      - refine install-dir function
>>
>>    jdk repo:
>>      - add prep-target-dir, install-import-dir, and
>> install-import-debuginfo
>>        functions
>>      - support importing .dSYM directories from hotspot
>>      - create libjsig.dSYM symlink from VM specific directory to
>>        the real location
>>
>> Files changed from code review round 0 to round 1:
>>
>>      common/autoconf/generated-configure.sh
>>      hotspot/make/Makefile
>>      hotspot/make/bsd/makefiles/defs.make
>>      hotspot/make/bsd/makefiles/dtrace.make
>>      hotspot/make/bsd/makefiles/gcc.make
>>      hotspot/make/bsd/makefiles/jsig.make
>>      hotspot/make/bsd/makefiles/saproc.make
>>      hotspot/make/bsd/makefiles/universal.gmk
>>      hotspot/make/bsd/makefiles/vm.make
>>      hotspot/make/defs.make
>>
>> New files changed in code review round 1:
>>
>>      hotspot/make/bsd/makefiles/product.make
>>      jdk/make/common/Defs.gmk
>>      jdk/make/java/redist/Makefile
>>      jdk/makefiles/Import.gmk
>>
>> Additional bugs filed based on code review comments, testing and
>> just plain looking at the Makefiles:
>>
>>      8026280 implement Full Debug Symbols on MacOS X jdk
>>      8026281 hotspot 'jdk6_or_earlier logic' for FDS needs to be removed
>>      8026282 the '64' subdir logic in Makefiles might be removable
>>      8026283 literal 'lipo' cmd uses in HotSpot makefiles
>>      8026284 Minimal1 references/support should be removed from 
>> BSD/MacOS X
>>      8026285 JPRT -buildenv ZIP_DEBUGINFO_FILES=0 no longer works
>>
>> As always, comments, questions and suggestions are welcome.
>>
>> Dan
>>
>>  > Greetings,
>>  >
>>  > I have the initial support for Full Debug Symbols (FDS) on MacOS X 
>> done
>>  > and ready for review:
>>  >
>>  >     7165611 implement Full Debug Symbols on MacOS X hotspot
>>  >     https://bugs.openjdk.java.net/browse/JDK-7165611
>>  >
>>  > Here is the JDK8/HSX-25 webrev URL:
>>  >
>>  > OpenJDK:
>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/0-jdk8/
>>  > Internal:
>> http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/0-jdk8/
>>  >
>>  > This webrev includes changes for the follow repos:
>>  >
>>  >     jdk8
>>  >     jdk8/hotspot
>>  >     jdk8/jdk
>>  >
>>  > Once these changes are approved, I'm planning to push them to
>>  > RT_Baseline. From there, they can follow the normal path to
>>  > Main_Baseline and eventually JDK8.
>>  >
>>  > This work enables FDS on MacOS X for the 'hotspot' repo; the 
>> changes in
>>  > the other repos are necessary to support importing the .diz files 
>> from
>>  > the MacOS X 'hotspot' build into the forest build. I also fixed a few
>>  > FDS related errors in the magic incantations for the new build. 
>> This is
>>  > mostly a port from Linux -> MacOS X/BSD with the dtrace changes 
>> ported
>>  > from Solaris. In other words, this is Frankenstein's monster...
>>  >
>>  > Thanks to Staffan Larsen for providing an initial set of changes
>>  > which I morphed into what you see here.
>>  >
>>  > Testing:
>>  > - JPRT HSX build and test on all platforms; verification of .diz
>>  >   files in the MacOS X JPRT bundles
>>  > - JPRT JDK8 forest build and test on all platforms; verification of
>>  >   .diz files in the MacOS X JPRT bundles
>>  >   Note: In previous FDS changesets, I also did a standalone 'jdk'
>>  >   repo build and test, but that no longer seems to work.
>>  >
>>  > As always, comments, questions and suggestions are welcome.
>>  >
>>  > Dan