RFR: JDK-8036003: Add --with-debug-symbols=[none|internal|external|zipped]

[David Holmes]

On 19/11/2015 9:19 PM, Yasumasa Suenaga wrote:
> Hi David,
>
>> My main point of contention is that zipping is simply a packaging
>> choice for the externally generated symbols - and we already have a
>> configure option to control that so I don't see why we would want to
>> double up on that part.
>
> I think this configure option is useful for packaging and debugging.
> For rpmbuild, rpmbuild strips debuginfo from binaries and build
> debuginfo package.
> For HotSpot developer, they can run GDB without any operations (unzip,
> set to GDB).

I'm not understanding what you are referring to here, but it doesn't 
seem to address the overlap between what you propose and what we already 
have with --disable-zip-debug-info.

David

> Currently, DEBUG_BINARIES and STRIP_POLICY are available in Makefile only.
> So we cannot set them when we run configure script.
>
> Thus I want to merge this patch for packaging, debugging, and
> consistency of build options.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2015/11/18 18:51, David Holmes wrote:
>> Hi Yasumasa,
>>
>> I had to wade back through all of the discussion from 2014. I'm still
>> not sure it really concluded exactly how to proceed with this, and the
>> current proposal still leaves me unhappy in some regards.
>>
>> My main point of contention is that zipping is simply a packaging
>> choice for the externally generated symbols - and we already have a
>> configure option to control that so I don't see why we would want to
>> double up on that part. (I don't think we have a deprecation policy
>> for configure args.)
>>
>> I also don't like that stripping is only considered for the "internal"
>> case. I think all of the aspects should be handled together.
>>
>> On 17/11/2015 1:32 PM, Yasumasa Suenaga wrote:
>>> Thank you for your comment!
>>>
>>> I've uploaded new webrev:
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.01/
>>
>> I don't think the key logic belongs in spec.gmk.in - it should be
>> where the new option is processed in the .m4 file; or where the
>> related logic is already handled.
>>
>> Thanks,
>> David
>> -----
>>
>>> Could you review it again?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2015/11/17 0:51, Andrew Hughes wrote:
>>>>
>>>> ----- Original Message -----
>>>>> Hello,
>>>>>
>>>>> I think this looks mostly OK, but I'm pretty sure Magnus will want to
>>>>> comment. I'm afraid he likely won't be able to until next week
>>>>> however.
>>>>>
>>>>> One thing, could you name the variable DEBUG_SYMBOLS, with the S at
>>>>> the
>>>>> end so that it matches the configure option?
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2015-11-16 10:38, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I would like to propose JDK-8036003: Add
>>>>>> --with-debug-symbols=[none|internal|external|zipped] .
>>>>>>
>>>>>> I had posted this enhancement which provides new make variable [1].
>>>>>> But it does not fit current jdk 9. So I propose it again as new
>>>>>> configure
>>>>>> option.
>>>>>>
>>>>>> I'm sure that this enhancement helps Linux distros to package
>>>>>> OpenJDK.
>>>>>> For example, if you create RPM package of OpenJDK, you have to pass
>>>>>> many
>>>>>> make variables [2].
>>>>>>
>>>>>> Could you review it?
>>>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.00/
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> [1]
>>>>>> http://mail.openjdk.java.net/pipermail/build-dev/2014-February/012019.html
>>>>>>
>>>>>>
>>>>>> [2]
>>>>>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-November/034036.html
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>> It looks mostly ok to me too. It would be good to finally get this
>>>> simplified.
>>>>
>>>> I agree on the pluralisation, but the problem there is the existing
>>>> badly
>>>> named ENABLE_DEBUG_SYMBOLS which actually seems to enable splitting
>>>> debuginfo,
>>>> going by this proposed change. Technically, I don't think the two
>>>> conflict, but
>>>> it is potentially (even more) confusing to read.
>>>>
>>>> The message AC_MSG_CHECKING([type of debug symbol]) should
>>>> definitely be
>>>> changed to the plural. I suggest AC_MSG_CHECKING([what type of debug
>>>> symbols to use]).
>>>> Also, AC_MSG_RESULT should precede the error check so the user can see
>>>> the bad
>>>> value that is failing:
>>>>
>>>> +  AC_MSG_CHECKING([what type of debug symbols to use])
>>>> +  AC_ARG_WITH([debug-symbols], [AS_HELP_STRING([--with-debug-symbols],
>>>> +      [set the debug symbol configuration (none, internal, external,
>>>> zipped) @<:@zipped@:>@])],
>>>> +      [
>>>> +        DEBUG_SYMBOL="${withval}"
>>>> +      ],
>>>> +      [
>>>> +        DEBUG_SYMBOL="zipped"
>>>> +      ])
>>>> +  AC_MSG_RESULT([$DEBUG_SYMBOL])
>>>> +
>>>> +  if test "x$DEBUG_SYMBOL" != xzipped && \
>>>> +      test "x$DEBUG_SYMBOL" != xnone && \
>>>> +      test "x$DEBUG_SYMBOL" != xinternal && \
>>>> +      test "x$DEBUG_SYMBOL" != xexternal; then
>>>> +    AC_MSG_ERROR([Allowed debug symbols are: none, internal,
>>>> external, zipped])
>>>> +  fi
>>>> +
>>>>