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

[Yasumasa Suenaga]

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

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