Dan, Thank you for the explanation.
The fix looks good for me. -Dmitry On 2014-11-11 18:40, Daniel D. Daugherty wrote: > Dmitry, > > Thanks for the quick review! > > Replies embedded below... > > > On 11/11/14 1:35 AM, Dmitry Samersoff wrote: >> Dan, >> >> 1. defs.make: >> >> It might be better to join obcopy version check and condition at ll.190 > > I looked at that... The seemingly natural place to put the version check > is actually in the else branch on line 194... However, if the version > check is bad, then you have to make a second check for a reset OBJCOPY > value (along with indenting all the code another level or two). > > It just looked ugly... it seemed better to keep the version check > separate from the other logic. > > >> otherwise the user will have a wrong version warning and then misleading >> message "no objcopy cmd found" > > However, part of that wrong version warning is this line: > > WARNING: ignoring above objcopy command. > > so in reality that "no objcopy cmd found" is just confirming > that we are indeed ignoring the objcopy cmd that we found... > > >> 2. Did you consider moving objcopy detection to configure? > > No because this fix has to be backported to JDK8u and JDK7 since > we support FDS in those releases... > > Of course, the build-infra team is always welcome to use a new > bug to evolve this code for JDK9 and newer. > > Again, thanks for the review! > > Dan > > >> >> >> -Dmitry >> >> >> On 2014-11-11 03:00, Daniel D. Daugherty wrote: >>> Greetings, >>> >>> I have a Solaris Full Debug Symbols (FDS) fix ready for review. >>> Yes, it is a small fix, but it is in Makefiles so feel free to >>> run screaming from the room... :-) On the plus side the fix does >>> delete two work around source files (Coleen would say that's a >>> Good Thing (TM)!) >>> >>> The fix is to detect the version of GNU objcopy that is being >>> used on the machine and only enable Full Debug Symbols when that >>> version is 2.21.1 or newer. If you don't have the right version, >>> then the build drops back to pre-FDS build configs with a message >>> like this: >>> >>> WARNING: /usr/sfw/bin/gobjcopy --version info: >>> WARNING: GNU objcopy 2.15 >>> WARNING: an objcopy version of 2.21.1 or newer is needed to create valid >>> .debuginfo files. >>> WARNING: ignoring above objcopy command. >>> WARNING: patch 149063-01 or newer contains the correct Solaris 10 SPARC >>> version. >>> WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86 >>> version. >>> WARNING: Solaris 11 Update 1 contains the correct version. >>> INFO: no objcopy cmd found so cannot create .debuginfo files. >>> INFO: ENABLE_FULL_DEBUG_SYMBOLS=0 >>> >>> This work is being tracked by the following bug IDs: >>> >>> JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC >>> https://bugs.openjdk.java.net/browse/JDK-8033602 >>> >>> JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on >>> Solaris X86 >>> https://bugs.openjdk.java.net/browse/JDK-8034005 >>> >>> Here is the webrev URL: >>> >>> http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/ >>> >>> Testing: >>> >>> - JPRT test jobs to verify that the current JPRT Solaris hosts >>> are happy >>> - local builds on my Solaris 10 X86 machine to verify that the >>> wrong version of GNU objcopy is caught >>> >>> Thanks, in advance, for any comments, questions or suggestions. >>> >>> Dan >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
