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
+
