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

[Andrew Hughes]

----- 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
+
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07