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
+

Reply via email to