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
+
