RFR: 8148302: Initial Bringup of Android in mobile/dev

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Wed Jan 27 09:56:54 UTC 2016


On 2016-01-26 20:50, Bob Vandette wrote:
> Please review the changes for two improvements for the Mobile Dev Forest.
>
> With these changes, the mobile/dev forest can successfully build x86 and ARM Android
> JDK 9 binaries.   The ARM binaries use the Zero ARM interpreter while the x86
> implementation uses standard x86 Hotspot VMs.
>
> The webrev also includes 1 fix for iOS BSD builds in hotspot/make/bsd/makefiles/vm.make.
>
> JDK-8148304	 Mobile: iOS builds fail on assembler files
>                           https://bugs.openjdk.java.net/browse/JDK-8148304
>
> JDK-8148302	 Mobile: Initial bring of Android x86 and ARM
>                           https://bugs.openjdk.java.net/browse/JDK-8148302
>
> Here’s the webrev:
>
> http://cr.openjdk.java.net/~bobv/8148302/webrev

Hi Bob,

Some comments:

* In jni_util_md.c, you have accidentally lost a trailing ).

* In spec.gmk.in, please use ":=" instead of "=". The former is 
assignment, the latter is used for macro expansion. (As a rule of thumb, 
:= should always be used unless you have explicit reasons.)

* In CompileJavaModules.gmk, why do you remove the exclude for 
BUILD_HEADLESS_ONLY? This change affects all platforms, not only android.

* In lib-x11.m4, I *really* don't like adding X11 paths when we should 
not have X11! (And it's even worse since you do not check for Android 
but do so for all platforms.) What files are needed, and for what 
library? Can you not fix this dependency by removing the #include 
statements in the source code instead? Otherwise, I'd say that android 
*is* indeed dependent on X11, and you need to express this properly.

* In lib-ffi.m4 and Copy-java.base.gmk: I think this a bit problematic. 
I believe what you're trying to achieve is similar to what we do for 
freetype, where we "bundle" the library with the resulting product. I'd 
rather see this libffi bundling based on that behavior. More concretely:
  - The name of the variable should be like LIBFFI_BUNDLE_LIB_PATH, to 
clearly show that this is used only when bundling the product, not when 
compiling it.
  - Whether to bundle or not should be determined in lib-ffi.m4. The 
default value should be "bundle if android+zero, otherwise not". If you 
want to, you can add a --enable-libffi-bundling to override this. I 
won't require it, but I generally find such things useful since you can 
test that bundling works even without building for a specific target. 
The code in Copy-java.base.gmk should then only test if this variable is 
present, compare with freetype in Copy-java.desktop. Finally, the 
configure code must check for the case that libffi bundling is enabled 
but no explicit libffi path is set (e.g. it is autodetected). It's okay 
to abort with an error, but it must be detected.

And finally, a note of appreciation. :-) I'm really happy to see the 
ugly solution with the hard-coded linker scripts go away in flags.m4!

/Magnus


More information about the mobile-dev mailing list