RFR: 8033580: Old debug information in IMPORT_JDK is not removed (original) (raw)

Erik Joelsson erik.joelsson at oracle.com
Mon Feb 17 08:09:07 UTC 2014

Hello,

The change looks good to me too.

Regarding indentation, for the rest of the JDK, we indent rules with make ifeqs like this:

target: sources <8 spaces>ifeq (something) <tab+2 spaces>recipe line <8 spaces>endif

Our reasoning is that it should still look like a recipe when glancing over it quickly (so 8 characters base indentation). After that we apply our standard 2 chars per logical level. Since make regards tab characters as special, we have to use space for non recipe lines.

The rest of our guidelines can be found here: http://openjdk.java.net/groups/build/doc/code-conventions.html

/Erik

On 2014-02-14 17:45, Daniel D. Daugherty wrote:

> http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/

make/Makefile No comments. Makefile style question: Since all this ifeq(...) logic is around Makefile rules, should the rules themselves be indented by one tab or by one tab and some number of spaces. I scrolled around the Makefile and I don't really see consistent indenting of the rule lines themselves... If you change the indenting, I don't see a reason for another round of review. Dan

On 2/14/14 6:21 AM, Erik Helin wrote: Dan,

On 2014-02-14 00:29, Daniel D. Daugherty wrote: > http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

make/Makefile + ifeq ($(OSNAME), bsd) + (RM)−rf(RM) -rf (RM)−rf(EXPORTSERVERDIR)/libjvm.dylib.dSYM The above needs to be: ifeq ($(OSVENDOR), Darwin) $(RM) -rf $(EXPORTSERVERDIR)/libjvm.dylib.dSYM I don't think that BSD uses .dylib stuff; that's MacOS X specific. You are right, thanks for the correction. Please see new webrev at: http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/ Again, same testing as for prior patches were run. Thanks, Erik Dan

On 2/13/14 1:59 PM, Erik Helin wrote: Hi Dan, thanks for reviewing! See my replies inline. On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote: > http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

make/Makefile 277 ifeq ($(ZIPDEBUGINFOFILES),1) 278 ifeq ($(OSNAME), windows) 279 (RM)−f(RM) -f (RM)−f(EXPORTSERVERDIR)/jvm.map $(EXPORTSERVERDIR)/jvm.pdb 280 else 281 (RM)−f(RM) -f (RM)−f(EXPORTSERVERDIR)/libjvm.debuginfo 282 endif On MacOS X, the debuginfo will be in libjvm.dylib.dSYM so you'll need a MacOS X specific rule. You don't need an update for the JVMVARIANTCLIENT version because MacOS X doesn't support the Client VM, but if it did... Thanks for catching this! I've uploaded a new webrev at: http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/ The same testing was done as for the first patch. On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote: So the above change handles libjvm, but what about the other libraries exported by HotSpot? libjsig, libjvmdb, and libjvmdtrace come to mind... As we discussed, I would like to implement this for libjvm first and then take care of the other libraries in a separate patch. Thanks, Erik Dan

On 2/12/14 8:03 AM, Erik Helin wrote: Hi all, this patch changes how old debug information copied from IMPORTJDK is treated. When running the copy*jdk target, HotSpot's makefiles copies the entire IMPORTJDK folder, including additional files (such as unzipped debug information). The export*jdk targets will then, via the genericexport target, copy the build artifacts via implicit rules to the correct destination in hotspot/build/JDKIMAGEDIR. The bug arises when IMPORTJDK contains unzipped debug info (libjvm.debuginfo) and the make target produces zipped debug info (libjvm.diz), or vice versa. hotspot/build/JDKIMAGEDIR will then contain both libjvm.debuginfo and libjvm.diz, but only one of them will match libjvm.so. Finally, the JPRT make targets jprtbuild* just zips hotspot/build/$(JDKIMAGEDIR). The zipped JPRT bundle will end up having different zipped and unzipped debug info, since the IMPORTJDK in JPRT contains libjvm.debuginfo and we build libjvm.diz by default. This patch removes the debug info that we did not build. If we build libjvm.diz, then libjvm.debuginfo will be removed (if it exists). Correspondingly, if we build libjvm.debuginfo, then libjvm.diz will be removed (if it exists). Patch: http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8033580 Testing: - Building in JPRT - Building Linux 32-bit locally on a Linux 64-bit machine - Building Linux 64-bit locally on a Linux 64-bit machine For all of the above, verify that only the correct debug info is present in the output. Thanks, Erik

More information about the build-dev mailing list