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

Tue Oct 15 02:33:18 PDT 2013

Daniel,
Looks good to me.

/Erik

On 2013-10-14 17:52, Daniel D. Daugherty wrote:
> On 10/14/13 9:18 AM, Daniel D. Daugherty wrote:
>> On 10/13/13 7:57 PM, David Holmes wrote:
>>
>>> 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.
>
> I somehow missed/lost two 1-liner changes:
>
> hg diff common/autoconf/basics.m4 common/autoconf/spec.gmk.in
> diff -r 4faa09c7fe55 common/autoconf/basics.m4
> --- a/common/autoconf/basics.m4 Wed Oct 02 15:08:03 2013 +0200
> +++ b/common/autoconf/basics.m4 Mon Oct 14 08:49:37 2013 -0700
> @@ -644,6 +644,7 @@ fi
>  fi
>
>  if test "x$OPENJDK_TARGET_OS" = "xmacosx"; then
> +  BASIC_REQUIRE_PROG(DSYMUTIL, dsymutil)
>    BASIC_REQUIRE_PROG(XATTR, xattr)
>    AC_PATH_PROG(CODESIGN, codesign)
>    if test "x$CODESIGN" != "x"; then
> diff -r 4faa09c7fe55 common/autoconf/spec.gmk.in
> --- a/common/autoconf/spec.gmk.in       Wed Oct 02 15:08:03 2013 +0200
> +++ b/common/autoconf/spec.gmk.in       Mon Oct 14 08:49:37 2013 -0700
> @@ -484,6 +484,7 @@ DATE:=@DATE@
>  DATE:=@DATE@
>  DIFF:=@DIFF@
>  DIRNAME:=@DIRNAME@
> +DSYMUTIL:=@DSYMUTIL@
>  FIND:=@FIND@
>  FIND_DELETE:=@FIND_DELETE@
>  ECHO:=@ECHO@
>
>
> I don't think that these two 1-liners are worth re-rolling the webrev.
> Please let me know if you don't agree.
>
> Dan
>
>
>>
>> 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
>>
>>
>