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

[Yasumasa Suenaga]

Thank you for your comment!

I've uploaded new webrev:
   http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.01/

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