Hi David,
Can you reinstate the comment:
581 # This must be done after the toolchain is setup, since we're looking
at objcopy.
as this constraint still exists.
I will reinstate it.
It would be nice if the external and zipped cases could somehow be shared so
that we don't have to duplicate everything just to alter the value of a single
variable.
I guess that it is difficult.
For example, ZIP_DEBUGINFO_FILES is used by many makefiles in hotspot,
ENABLE_DEBUG_SYMBOLS
is used in jdk.
Variables which are used in this patch are used in many makefiles.
So we have to work for big refactoring if we do that.
Thus, I want to use them to make simple.
634 # --enable-zip-debug-info is deprecated.
635 # Please use --with-debug-symbols=zipped .
636 BASIC_DEPRECATED_ARG_ENABLE(zip-debug-info, zip_debug_info)
I would have hoped that the deprecation macro enabled you to inform the user
what the replacement is as per the comments! This seems unsatisfactory. :(
Currently, we cannot pass any message to BASIC_DEPRECATED_ARG_ENABLE (and
BASIC_DEPRECATED_ARG_WITH).
Thus we have to change them if we want to inform to user.
It is out of scope of this enhancement.
Should I file it as another issue?
I'm assuming there is no change to the default behaviour if none of these
options are specified.
Yes. Default behavior is zipped.
Thanks,
Yasumasa
On 2015/12/03 15:51, David Holmes wrote:
On 1/12/2015 12:34 AM, Yasumasa Suenaga wrote:
Hi Magnus,
I've uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.05/
common/autoconf/jdk-options.m4
Can you reinstate the comment:
581 # This must be done after the toolchain is setup, since we're looking
at objcopy.
as this constraint still exists.
It would be nice if the external and zipped cases could somehow be shared so
that we don't have to duplicate everything just to alter the value of a single
variable.
634 # --enable-zip-debug-info is deprecated.
635 # Please use --with-debug-symbols=zipped .
636 BASIC_DEPRECATED_ARG_ENABLE(zip-debug-info, zip_debug_info)
I would have hoped that the deprecation macro enabled you to inform the user
what the replacement is as per the comments! This seems unsatisfactory. :(
I'm assuming there is no change to the default behaviour if none of these
options are specified. I'm unclear whether we will need to coordinate this
change with any internal processes that might explicitly be using the
deprecated options - I think Magnus should have a better idea about that.
Thanks,
David
-----
If we do implement this flag, it should work as expected. If you
believe this is too hard to get right, I'm alright with fixing this as
a separate bug.
Okay, I removed --enable-java-debug-symbols in this webrev.
I will file to JBS and try to create patch for it after this issue.
That is, have you tried building with the four different values of
--with-native-debug-symbols and verified that the result is indeed as
you specified?
Of course, I've checked all pattern of --with-native-debug-symbols.
I ran readelf -S for libjvm, libnio, libsaproc, and java command.
I checked debug section (.debug_info, and more) in ELF binaries.
Just by my quick glance I can already spot what I believe is a
mistake: for "none" you set STRIP_POLICY to min_strip, but it should
reasonably be no_strip.
I've fixed in this webrev.
Thanks,
Yasumasa
On 2015/11/30 21:26, Magnus Ihse Bursie wrote:
On 2015-11-26 15:19, Yasumasa Suenaga wrote:
I uploaded a new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.04/
I implemented --with-native-debug-symbols and
--enable-java-debug-symbols in it.
And I deprecated --enable-debug-symbols and --enable-zip-debug-info .
Hi Yasumasa,
I apologize I have not been able to spend more time on this. I still
only had time to take a short glimpse, but I found a few issues anyway.
First, the new -enable-java-debug-symbols is problematic. If you build
with "--enable-debug --disable-java-debug-symbols", then this implies
that you would get a build without java debug symbols. However, this
will not happen, since changing debug level wil enable java debug
symbols, but the new java-debug-symbols flag will not manage to
disable this.
If we do implement this flag, it should work as expected. If you
believe this is too hard to get right, I'm alright with fixing this as
a separate bug.
Second, all the flags we use to manage this internally makes me
nervous. :) Are you sure you're getting them right? That is, have you
tried building with the four different values of
--with-native-debug-symbols and verified that the result is indeed as
you specified? You'd need to check both libjvm.so and at least a jdk
library, like libjava.so or so, since they use different build system
and flags.
Just by my quick glance I can already spot what I believe is a
mistake: for "none" you set STRIP_POLICY to min_strip, but it should
reasonably be no_strip. (I'd be surprised if strip was run if I
compiled without debug symbols).
Other than that, I believe this patch is starting to get ready.
/Magnus
Thanks,
Yasumasa
On 2015/11/25 23:52, Yasumasa Suenaga wrote:
Hi Magnus,
--with-java-debug-symbols=none,all
(or "none,internal" for consistency).
Can I change this option to --enable-java-debug-symbols ?
This value set to none or all. So I think that we should use
AC_ARG_ENABLE for it.
Thanks,
Yasumasa
On 2015/11/25 23:35, Magnus Ihse Bursie wrote:
On 2015-11-21 14:12, Yasumasa Suenaga wrote:
My point with current proposal is that either it doesn't touch
the zipping because we already have an option for that; or it
should deprecate the existing option (now Erik has pointed out
that capability).
I'm using BASIC_DEPRECATED_ARG_ENABLE for debug-symbols and
zip-debug-info in new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.03/
I had thought this new option as a wrapper for existing options.
However, if current options should be deprecated, I will follow it.
Hi Yasumasa,
Sorry for the delay in my response.
I had to go back and re-read the old discussions and my personal
notes about this. It turned out that some of the complications that
existed then is not relevant from Oracle's point of view anymore
(more specifically, stripping to several distinct levels). With
that complication gone, the basic case boils down to the three ways
of doing this:
1) Compile without debug information ("none")
2) Compile with debug information and keep it ("internal")
3) Compile with debug information, copy it to an external file, and
strip the binary ("external")
So far, so good. I agree with David Holmes that the zipping
complicates things somewhat. The problem here is two-fold: Firstly,
I believe that the "external" choice will be relevant to nobody (as
in, neither the community nor Oracle), and secondly, there have
been discussions of handling the external debuginfo differently
(namely, to move all debug info, unzipped, into a separate image,
and zip them all at once). It's a bit unfortunate if we create an
interface to configure that cannot grow properly with future changes.
On the other hand, it does seem convenient to express the end
result for the Oracle build as "zipped" (or possibly
"external-zipped", to emphasize the connection to "external"?).
So I'm not sure what to think myself. But if David agrees that
adding "zipped" (or "external-zipped") to the new argument, and
deprecating the old, then I agree with it as well. :-)
The second discussion here is the connection between java and
native debug symbols.
Fact: We do not currently have a story for setting java debug
symbols. That's bad, and a regression as pointed out.
Fact: That does not mean it has to be fixed in this patch.
I do think that java and native debug symbols are different things
and should be treated differently. But I also think we should have
configure arguments that show their similarity. So, I propose that
we have a
--with-native-debug-symbols=none,internal,external,zipped
as you propose, and then also a
--with-java-debug-symbols=none,all
(or "none,internal" for consistency).
I think it would be nice if that flag could be implemented in the
same patch, but since it is a different thing I understand if you
want to leave it out. There is some added complication here, since
we already set -g for java compilation depending on debug level,
and the native debug symbol patch is obviously tricky as it is.
/Magnus
