On 2016-01-27 19:37, Bob Vandette wrote:
Here’s the updated webrev with updates based on your feedback.
http://cr.openjdk.java.net/~bobv/8148302/webrev.01
I implemented all of the changes suggested.
For the X11 issue, I removed my changes and replaced BUILD_HEADLESS_ONLY with
SUPPORT_HEADFUL. This allows a headless only build to be fully controlled by
—disable-headful. This shouldn’t impact our ARMv8 build since we set both
—disable-headful
and BUILD_HEADLESS_ONLY.
Ah, right, you started cleaning up the mess already. :) That's nice,
SUPPORT_HEADFUL==yes is at least expressing the right kind of semantics.
The new webrev looks good to me.
Thank you Bob for helping keeping the quality of the build system code high!
/Magnus
Thanks,
Bob.
On Jan 27, 2016, at 4:56 AM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>
wrote:
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
