RFR: 8017480: Move copying of jfr files to closed makefile
Please review this simple patch, removing copying of jfr files from CopyFiles.gmk and adding an optional include of a custom CopyFiles.gmk. http://cr.openjdk.java.net/~erikj/8017114/webrev.jdk.01/ /Erik
Re: RFR: 8017480: Move copying of jfr files to closed makefile
Looks good. /Staffan On 24 jun 2013, at 13:57, Erik Joelsson erik.joels...@oracle.com wrote: Please review this simple patch, removing copying of jfr files from CopyFiles.gmk and adding an optional include of a custom CopyFiles.gmk. http://cr.openjdk.java.net/~erikj/8017114/webrev.jdk.01/ /Erik
Re: HSX-24: Code Review (round 0) request for MacOS X exported symbols fix (8014326)
Added back build-dev.
On 6/19/2013 3:58 PM, Coleen Phillimore wrote:
Dan,
This looks good!
Coleen
On 6/18/2013 4:50 PM, Daniel D. Daugherty wrote:
Greetings,
I have picked up David Holmes' work on the following bug:
8014326 [OSX] All libjvm symbols are exported
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014326
https://jbs.oracle.com/bugs/browse/JDK-8014326
Here are the HSX-24 backport webrev URLs:
OpenJDK: http://cr.openjdk.java.net/~dcubed/8014326-webrev/0-hsx24/
Internal: http://javaweb.us.oracle.com/~ddaugher/8014326-webrev/0-hsx24/
Testing:
- JPRT test job on MacOS X
- (in process) Aurora Adhoc vm.quick batch for MacOS X in the following
configs: {Server VM} x {fastdebug} x {-Xmixed}
Gory details are below. As always, comments, questions and
suggestions are welome.
Dan
Gory Details:
The script and Makefile changes are easy to review via the webrev.
However, every function name in the MacOS X mapfiles had to be modified
to match the MacOS X symbol export syntax. I created normalized
copies of the mapfiles in order to compare the semantic changes (and
ignore the syntactic changes). These diffs are added to bug report,
but JBS is not yet accessible outside Oracle and bugs.sun.com can
be slow to update so I've included the diffs here.
mapfile-vers-debug: current BSD versus updated BSD
$ diff bsd-funcs-debug{.orig,}
191a192
JVM_SetNativeThreadName
215,232d215
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4
# but their code has not been removed yet because there will not
# be a substantial code savings until JVM_InvokeMethod and
# JVM_NewInstanceFromConstructor can also be removed; see
# reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
239,241d221
fork1
numa_warn
numa_error
243,245d222
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 192: add missing JVM_SetNativeThreadName entry
Delete the old reflection routines!
Delete other functions not compiled into BSD/MacOS X.
mapfile-vers-product: current BSD versus updated BSD
$ diff bsd-funcs-product.orig bsd-funcs-product
191a192
JVM_SetNativeThreadName
215,232d215
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4
# but their code has not been removed yet because there will not
# be a substantial code savings until JVM_InvokeMethod and
# JVM_NewInstanceFromConstructor can also be removed; see
# reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
239,241d221
fork1
numa_warn
numa_error
243,245d222
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 192: add missing JVM_SetNativeThreadName entry
Delete the old reflection routines!
Delete other functions not compiled into BSD/MacOS X.
Since the MacOS X port was originally derived from the Linux port,
it also makes sense to compare the updated BSD files versus the
current Linux files. This is a useful sanity check.
mapfile-vers-debug: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-debug
214c214
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
226a227,229
fork1
numa_warn
numa_error
227a231,233
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 214 is an obvious rename.
Lines 227-229 are functions not compiled into BSD/MacOS X.
Line 232 is an error on Linux; sysThreadAvailableStackWithSlack is
only on Solaris, but we won't fix that with this bug since we want
the MacOS X fix in HSX24 and in HSX25.
mapfile-vers-product: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-product
214c214
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
221a222,224
fork1
numa_warn
numa_error
222a226,228
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 214 is an obvious rename.
Lines 222-224 are functions not compiled into BSD/MacOS X.
Line 227 is an error on Linux; sysThreadAvailableStackWithSlack is
only on Solaris, but we won't fix that with this bug since we want
the MacOS X fix in HSX24 and in HSX25.
Lastly,
RFR (S): Enable new build on Linux/PPC64 (top-level part)
Hi, could somebody please review the following change and create an appropriate Bug ID for it: http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_top/ The change will enable to build the PowerPC/AIX port on Linux/PPC64 with the new build system using an interpreter only VM using the C++ interpreter. Therefore it introduces a new JVM variant called core which enables an interpreter only build and a new configure option called --with-jvm-interpreter which can be used to choose either the default template interpreter or the C++ interpreter. Notice that this 'core' build currently only works on Linux/PPC64 and it is used for the PowerPC/AIX porting project to bootstrap its build. (See 8016476: PPC64 (part 1): reenable CORE buildhttp://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016476for the corresponding HotSpot change). Once we have all the required patches in the HotSpot repository (and the small JDK changes from http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_jdk), building on newer Linux distros like Fedora 17 will be as easy as: sh /priv/openjdk/OpenJDK/ppc-aix-port/jdk8/configure --with-boot-jdk=/priv/openjdk/OpenJDK/openjdk1.7.0-ppc-aix-port-b03 --with-jvm-variants=core --with-jvm-interpreter=cpp --with-debug-level=slowdebug make images LOG=debug On older releases like SLES 10 you'll have to use something like: sh /priv/openjdk/OpenJDK/ppc-aix-port/jdk8/configure --with-boot-jdk= /priv/openjdk/OpenJDK/openjdk1.7.0-ppc-aix-port-b03--with-jvm-variants=core --with-jvm-interpreter=cpp--with-debug-level=release --with-extra-cflags=-m64 --with-extra-cxxflags=-m64 --with-extra-ldflags='-m64 -L/lib64' --x-libraries=/usr/X11R6/lib64 --x-includes=/usr/X11R6/include CFLAGS=-m64 CXXFLAGS=-m64 make images LOG=debug The extra options and flags are mainly necessary because the GCC on the latter platform produces 32-bit binaries by default. Notice that the environment variables CFLAGS=-m64 CXXFLAGS=-m64 are necessary for the configure script itself to detect the correct 64-bit platform while the configure options like --with-extra-cflags are needed in order to select the right flags for the build. Following some more detailed descriptions of the change: common/autoconf/boot-jdk.m4 Linux/PPC64 needs a slightly bigger heap for the huge JDK batch. Treat it like MacOS which uses '-Xmx1600M'. common/autoconf/configure.ac common/autoconf/hotspot-spec.gmk.in common/autoconf/jdk-options.m4 Add --with-interpreter option to configure which can be used th choose one of the 'template' or the 'C++' interpreter. Default is the 'template' interpreter. common/autoconf/jdk-options.m4 common/autoconf/spec.gmk.in make/hotspot-rules.gmk Add JVM variant 'core' to configure which can be used th choose the HotSpot 'core' (i.e. 'interpreter only') build. Notice that this 'core' build currently only works on Linux/PPC64 and it is used for the PowerPC/AIX porting project to bootstrap its build. (See 8016476: PPC64 (part 1): reenable CORE buildhttp://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016476for the corresponding HotSpot change). We currently also don't build the serviceability agent on PPC64. common/autoconf/libraries.m4 Older Linuxes (e.g. SLES 10) may still have the X11R6 directory but also a symlink from /usr/include/X11 to ../X11R6/include/X11 so we need to check for the X11R6 directory AFTER we checked for /usr/include/X11 Thank you and best regards, Volker
RFR (XS): Enable new build on Linux/PPC64 (jdk part)
Hi, could somebody please review the following change and create an appropriate Bug ID for it: http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_jdk/ The patch contains two little changes which are required to build the class library part of the OpenJDK on Linux/PPC64. Most of the build magic is contained in the top-level part of this change which is separately reviewed at http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_top CompileLaunchers.gmk Remove mapfile from build instructions of BUILD_UNPACKEXE: CFLAGS_macosx:=-fPIC, \ - MAPFILE:=$(JDK_TOPDIR)/makefiles/mapfiles/libunpack/mapfile-vers-unpack200,\ LDFLAGS:=$(UNPACKEXE_ZIPOBJS),\ I think it makes no sense to use a version script file for an executable and older linkers (e.g. on SLES 10) complain with: *Invalid version tag `SUNWprivate_1.1'. Only anonymous version tag is allowed in executable.* The GNU ld manualhttp://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_25.htmlstates: *Version scripts are only meaningful when creating shared libraries.* Morover unpack200 was the only executable which used a version script file. Fix typo (replace 'defalt: all' by 'default') default: all CompileNativeLibraries.gmk Only use $(OPENWIN_LIB) for linking LIBSPLASHSCREEN on Solaris! The old code worked only accidentally when the X-libraries are in the default linker path anyway. The right solution is to use $(X_LIBS) if not on Windows or Solaris. Append -DX_ARCH=X_PPC64 to LIBJSOUND_CFLAGS on PPC64. The value of X_ARCHisn't actually used on the PPC architectures, but there's a check to verify that it is set. Fix typo (replace 'defalt: all' by 'default') default: all Thank you and best regards, Volker
Re: RFR: 8017047: Can't use --with-java-devtools and --with-devkit at the same time
Erik: Open part of this review: There is an artificial limitation put into configure that prevents usage of options --with-devkit and --with-tools-dir at the same time. The reason was probably because they could potentially be conflicting. This patch removes this check and makes them work together, both ending up on the search path for tools (with --with-tools-dir ending up ahead of devkit). I also stumbled on some dead code that was causing warnings to be printed when using --with-devkit and decided to remove that while touching that option anyway. http://cr.openjdk.java.net/~erikj/8017047/webrev.root.01/ In the new basics.m4: 362 TOOLS_DIR=$TOOLS_DIR:$with_devkit/bin If TOOLS_DIR was empty, this will add an extra : Probably harmless, but I'd prefer not to see it happen. If you feel it is harmless, go ahead. Code deletion looks good. Tim
Re: RFR: 8017480: Move copying of jfr files to closed makefile
Looks good to me as well. Tim On 06/24/13 05:08 AM, Staffan Larsen wrote: Looks good. /Staffan On 24 jun 2013, at 13:57, Erik Joelssonerik.joels...@oracle.com wrote: Please review this simple patch, removing copying of jfr files from CopyFiles.gmk and adding an optional include of a custom CopyFiles.gmk. http://cr.openjdk.java.net/~erikj/8017114/webrev.jdk.01/ /Erik
Re: RFR: 8012564: The SOURCE value in release file of JDK 8 doesn't contain valid changesets for some OS since b74
Erik: Simple patch removing unnecessary check for existence of mercurial for getting the hgtips for the release file. This check prevented the backup solution of using the .hgtip files from working when building from source bundles. http://cr.openjdk.java.net/~erikj/8012564/webrev.root.01 Looks good. Tim
Re: Add optional support for using the system libicu
Hi Erik, Thanks for your comments. Sorry I couldn't get back to you earlier, I was busy with a conference. Here's an updated webrev: http://cr.openjdk.java.net/~omajid/webrevs/system-icu/01/ On 06/05/2013 05:21 AM, Erik Joelsson wrote: Is it required to remove the font/layout dir to use the system version? I would expect USE_EXTERNAL_ICU_LE=true would add an exclude for that directory to SetupNativeCompilation,BUILD_LIBFONTMANAGER. If you expect to delete the source tree on your side, why not have configure check for the existence of the font/layout directory to determine the default behavior for system vs bundled? That's a good point. The font/layout directory is only problematic because the build attempts to look up headers there instead of the system include paths first. Since it's desirable to make sure that the header files used are compatible with the icu library, removing the font/layout directory on build is one option. I have now worked around it by using different #includes for system/bundled cases. Everything should work correctly without needing to remove any files/directories. It seems you forgot to add USE_EXTERNAL_ICU_LE to spec.gmk.in. You are right, of course. Fixed. Are ICU_LE_CFLAGS and ICE_LE_LIBS supposed to be empty? Yes, they are empty on bundled builds. On system builds, they may not be. On my machine, with --with-icu-le=system, I get: ICU_LE_CFLAGS:= ICU_LE_LIBS:=-licule -licuuc -licudata On 2013-06-04 23:30, Omair Majid wrote: FYI: I am quite sure the build breaks if built using the old build system; I will fix that next. This is still on my TODO list. Thanks, Omair
Re: Add optional support for using the system libicu
Hi Phil, Updated webrev: http://cr.openjdk.java.net/~omajid/webrevs/system-icu/01/ It's still against jdk8/build and missing support for the old build system. On 06/05/2013 02:02 PM, Phil Race wrote: Since this entirely affects a 2D component, please include 2d-dev in this discussion. I would have been 'surprised' to see this change if I hadn't just spotted this thread. Mea culpa. I pushed a few similar system-library patches using this identical process and no one corrected me. So I assumed this was right. For the future, I will ensure the relevant groups are CC'd. And I believe this change should be integrated via the 2D forest. Okay. Are there any guidelines for this? It's not obvious to me when a change is more appropriate for build vs 2D. I am not sure what, if any, runtime problems might occur from using a different version. We've generally been able to swap in newer versions of ICU without much trouble, but I recommend to run appropriate tests before shipping .. Thanks for the suggestions. Do you have any particular tests in mind? For some background, the goal with this change is two fold: 1. Gain benefits from system-installed libraries. This is one library where OpenJDK does not lag behind in security updates, but in some other cases, embedded libraries can lag behind. It also makes it easier to use the distributions preferred policies (hardening flags on libraries and so on). 2. Make it easier to switch to HarfBuzz at some later point. ICU itself strongly recommends switching [1]. Through ice-le-hb [2], only a recompile should be needed to switch (hopefully). Note that JDK8 in fact has a very current ICU with some security fixes. So I would not recommend using the native lib on a system with an older ICU. Thanks for pointing it out. I see that ICU recently released a security update [1] too, but probably many distributions will not have this up-to-date version (my current distro, a little out-of-date, does not :( ). Thanks, Omair [1] http://site.icu-project.org/download/51 [2] https://github.com/behdad/icu-le-hb -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: [OpenJDK 2D-Dev] Add optional support for using the system libicu
Omair, I'm sorry I did not see this before. I probably would not have noticed it except I happened to notice this reply. I'm responsible for maintaining the version of ICU in JDK, and also (read: main day job) IBM's lead for ICU for C/C++. ( But I speak for myself here, of course. ) I think this patch is certainly a good direction (especially with regards HarfBuzz - I wrote that recommendation you cite), at least longer term, however, I have quite a number of concerns with this patch. • The big one - is that the ICU and JDK versions of the LE are divergent, so much so that, to put it mildly, I am a little surprised that this patch builds at all. Now, I would like - love - for enough upstream changes to ICU to happen so that the APIs are compatible enough for this change to be possible. But we aren't there yet. • … and when we are there, it won't be compatible with any shipping ICU, so you will need to wait until the future ICU is picked up. • You mentioned the issues about which version of the headers to compile against. You MUST compile against the same version of the headers you are linking against. Period. This is C++, not Java or even C. The vtable expected in the ICU version versus the JDK version of the LE are different, have different parentage. • As to tests, I would start with everything in jdk/test/java/awt/font/TextLayout ( NB Phil- I had started a port of ICU's letest.xml conformance test to the JDK- this would be helpful in making sure we get the same results.) I'm sorry I didn't see this earlier. I don't always monitor this address or even 2d-dev as closely as I could. Regards, Steven ( If you don't get a response from me, you can always ping me at s...@icu-project.org … ) On 6/24/13 3:14 p.m., Omair Majid wrote: Hi Phil, Updated webrev: http://cr.openjdk.java.net/~omajid/webrevs/system-icu/01/ It's still against jdk8/build and missing support for the old build system. On 06/05/2013 02:02 PM, Phil Race wrote: Since this entirely affects a 2D component, please include 2d-dev in this discussion. I would have been 'surprised' to see this change if I hadn't just spotted this thread. Mea culpa. I pushed a few similar system-library patches using this identical process and no one corrected me. So I assumed this was right. For the future, I will ensure the relevant groups are CC'd. And I believe this change should be integrated via the 2D forest. Okay. Are there any guidelines for this? It's not obvious to me when a change is more appropriate for build vs 2D. I am not sure what, if any, runtime problems might occur from using a different version. We've generally been able to swap in newer versions of ICU without much trouble, but I recommend to run appropriate tests before shipping .. Thanks for the suggestions. Do you have any particular tests in mind? For some background, the goal with this change is two fold: 1. Gain benefits from system-installed libraries. This is one library where OpenJDK does not lag behind in security updates, but in some other cases, embedded libraries can lag behind. It also makes it easier to use the distributions preferred policies (hardening flags on libraries and so on). 2. Make it easier to switch to HarfBuzz at some later point. ICU itself strongly recommends switching [1]. Through ice-le-hb [2], only a recompile should be needed to switch (hopefully). Note that JDK8 in fact has a very current ICU with some security fixes. So I would not recommend using the native lib on a system with an older ICU. Thanks for pointing it out. I see that ICU recently released a security update [1] too, but probably many distributions will not have this up-to-date version (my current distro, a little out-of-date, does not :( ). Thanks, Omair [1] http://site.icu-project.org/download/51 [2] https://github.com/behdad/icu-le-hb
