[PATCH] Add cross-compilation support

[Thierry Reding]

* Andrew John Hughes wrote:
> On 8 June 2010 13:08, Thierry Reding <thierry.reding at avionic-design.de> wrote:
> > The following patch adds rudimentary support for cross-compiling IcedTea 7.
> > This only works for non-bootstrapped builds. I've also tested building IcedTea
> > natively and it still works fine with this patch. The cross-compile was done
> > with an ARM target, but I currently have no hardware to test this on. I may
> > be able to runtime test the cross-compile on an i686 target later this week.
> >
> > Cross-compiling also requires a native build of IcedTea/OpenJDK, so you need
> > to pass --with-jdk-home=/path/to/icedtea/or/openjdk and --disable-bootstrap
> > for this to work. I have also not tested cross-building the plugin yet.
> >
> > Cheers,
> > Thierry
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.10 (GNU/Linux)
> >
> > iEYEARECAAYFAkwOMtcACgkQZ+BJyKLjJp9BtACgl4gbPauTVwgT0U6DeokFIhbq
> > IHcAn21GYiURLUtsHwu9fbTlbpfyJfkg
> > =ZESt
> > -----END PGP SIGNATURE-----
> >
> >
> 
> While you say it works, you don't really explain what this patch
> changes and why.  It's quite a big change and I don't really
> understand the reason for most of them.
> 
> Before I go into specifics about the patch, the primary issue is with
> maintaining this.  Upstream OpenJDK7 moves on rapidly.  It's no good
> dumping a patch like this into IcedTea7 where it rapidly bit rots.  If
> we're going to add this, there needs to be someone maintaining it and
> doing regular cross-compiles.  A big part of that is that it needs to
> be enabled on normal builds as well, otherwise this patch is liable to
> get broken every time there is a new build drop (which is weekly).  It
> also means we have a chance of getting it upstreamed.  IcedTea6 may be
> an easy target with less turbulence, but there still needs to be
> ongoing maintenance.  We already have a half-hearted attempt in
> IcedTea6 from a patch that was never approved on the list but was
> committed anyway.  I doubt it works now given things have changed
> quite a bit since and no-one has maintained it.  I'm worried the ARM
> assembler port is also headed down that road.  There aren't that many
> people working on IcedTea (and only me on IcedTea7), so if you want to
> add something that's not going to be tested regularly by normal usage
> like this, you need to step up to maintain and support it.

Unfortunately this is pretty much out of my hands. I've been working on this
for a project where I need to cross-compile Java, so I was thinking IcedTea
was my best bet. After the project I cannot give any guarantees. That is the
reason why I wanted to share this patch and try to get as much of it
upstreamed as possible so others can profit from it and in the hopes that the
patch will "maintain itself". I realize that this may require some more work
on my part, but once cross-compilation support goes into the mainline IcedTea
tree, I think perhaps others may chime in and fix things if they should break.

> With that out of the way,  some comments on the patch:
> 
> Makefile.am changes:
> 
> * Why are we changing the names of the FREETYPE variables?  It adds
> clarity but should be in a separate patch.  It's unnecessary for
> cross-compile support.

I agree that this should be a separate patch but I wanted to dump this all
here so we could discuss all of it together.

The FREETYPE variable changes are for consistency only and IMO makes the
parts where they are referenced easier to understand.

> * The cross-compile patch is added in the wrong place.  It needs to be
> on the end of the big ICEDTEA_PATCHES definition if it's a
> non-conditional patch.

I applied it at the very end because it makes working on it easier and
because it won't disturb any other patches in between. Before this is
mainlined, I'm all in favour of putting this in the right place.

> * Same applies with ICEDTEA_ENV.  Please extend the main definition
> for non-conditional additions.
> * The if CROSS_COMPILING additions; can these not be applied
> generally?  Why are these cross-compilation specific?  If they are not
> generally applied, they will rarely be tested.

I think most of them can be generally applied. In particular those setting
the tools and variables CC, EXTRA_CFLAGS, and friends should always be
applicable because they force the compiler as detected by configure to be
used.

> There is no reason to change ALT_OUTPUTDIR to its already default setting.

Then the ALT_OUTPUTDIR default doesn't seem to be correct. In a cross-compile
it would always end up putting files into linux-amd64 instead of linux-arm.

> Can you explain the other changes in that block?

BUILD_ARCH_DIR and LIBARCH need to be set because otherwise the OpenJDK
makefiles will set it to autodetected values that are plain wrong for
cross-compiling. CROSS_COMPILING is obvious.

I think all of those variables could be added unconditionally if we change
CROSS_COMPILING to be set to @CROSS_COMPILING@ and AC_SUBST it in the
configure script.

> configure.ac changes
> 
> * Please don't add yet another option.  autoconf has support for
> cross-compiling with host, target, build hosts.  Please use that.
> Again, an option will never get used.

I don't understand. No option is added. This is just the standard code to
check for the build compiler.

> corba-bootstrap.patch:
> 
> * What is this for? None of these changes seem to be cross-compilation related.

This is actually required in order to undo a change from the
icedtea-cross-compile patch that is applied to the openjdk subtree but will
not work correctly for the openjdk-boot tree, or rather break one of the
bootstrap patches.

> icedtea-cross-compile.patch  (can we not just call this
> cross-compile.patch?  I've been dropping the icedtea- prefix, there
> seems to be no need for it)

Certainly.

> - gcc.make: The CC and CPP changes make sense, but won't these fail
> given you only pass in CC for CROSS_COMPILATION builds?  Checking if
> $(CC) and $(CPP) are empty and setting them if they are would be
> better.

Right. If we always set CC and CPP in Makefile.am, this should be okay. Oddly
enough even non-cross-compiles worked for me. Possibly because make uses the
defaults for CC and CPP.

> There seems to be a redundant change to AS.

Yes, this doesn't need to change. Optionally we could drop this and also set
it in Makefile.am, but then again, configure's definition of AS would more
likely default to just plain 'as'.

> - rules.make: This seems to rename variables for no reason.  If there
> are really changes drowning in this sea of renaming, I can't see them.
> - vm.make: Same as rules.make.

This is just to make the usage of the C++ compiler consistent. I find it
confusing that there are 3 different variables for one and the same tool. But
you are right. We can get away with less changes if we define CCC to equal
CXX at the top of that file and leave the COMPILE.CC and friends untouched.

> - stubRoutines_zero.hpp: What is this? Why are you extending the Zero
> assembler port?

Those changes actually come from a more recent version of hotspot which in the
current IcedTea seems broken. The original change comes from hotspot changeset
1346:47742b654fcb. For some reason that snippet is missing from the tree
built by IcedTea.

> - freetypecheck: Aren't the CFLAGS changes already covered by
> icedtea-headers.patch?  The test turn-off seems correct.

Not quite. When building I would always get errors about wrong flags. If I
remember correctly FREETYPE_HEADERS_PATH wasn't set correctly.

> - jdk/make/common/Defs.gmk - what are EXTRA_{CFLAGS,CPPFLAGS,LDFLAGS}?

Those are set in Makefile.am and are flags either detected by configure or
set by the user when calling configure. Usually for cross-compiles, only
CFLAGS, CPPFLAGS and LDFLAGS would be set but that doesn't work with OpenJDK
because setting CFLAGS et al. directly would override any changes to them by
OpenJDK's makefiles.

> - java/nio/Makefile: Makes sense, similar to BOOT_JAVAC and JAVAC.
> Can we not thus use BOOT_CC rather than CC_FOR_BUILD?

Fine with me. CC_FOR_BUILD is the standard name with autotools, but if you
prefer BOOT_CC and BOOT_CFLAGS, that's just as well.

> - sun/xawt/Makefile:
> 
> + ifeq  ($(PLATFORM), solaris)
> +-CPPFLAGS += -DFUNCPROTO=15
> ++DEFINES += -DFUNCPROTO=15
> + dummy := $(shell $(MKDIR) -p $(LIB_LOCATION))
> + endif
> 
> Have you tested on Solaris?
> Again, why all the renaming?

Nope, I have no Solaris box. But as you can see at the end of that block,
DEFINES and INCLUDES are both added to CPPFLAGS again. This is important for
building the sizer tools later on. They need those defines and includes but
*not* all other CPPFLAGS. That's why they are separated like this.

> - sun/font/Makefile: does icedtea-headers not cover these FT changes?

See my comments above about freetypecheck.

> - openjdk/Makefile: seems to be no reason for this

There is a very good reason for this. Corba (specifically libioser12.so)
links against libjvm.so and libjava.so. But libjvm.so and libjava.so are
built *after* corba, namely during the hotspot and jdk targets respectively.
In bootstrapped, non-cross-compiled builds, this is worked around by linking
libioser12.so against the libraries from the gcj installation, which
obviously doesn't work for cross-compiles.

It strikes me as odd that this is even done like this. Shouldn't corba really
require the libraries as built in the current configuration. In other words,
is it not an error to link against the gcj libjvm.so and libjava.so?

> - openjdk/corba/make/common/Defs-linux.gmk: This is the same as
> corba-bootstrap.patch!

This is actually the patch that I was talking about earlier. These changes
are incompatible with the bootstrap stage. That is why the corba-bootstrap
patch reverts them.

> adlc.make: Again spurious renaming.

Yes, obviously required by those earlier changes. If you'd rather have this
as a separate patch or don't like it at all I can probably work around this
by defining CCC to CXX and drop the other changes.

> platform_zero.in: why is @ZERO_ARCHDEF@ removed?

This is necessary because the libffi headers already define the same symbols,
therefore resulting in an error when they are defined on the command-line. I
don't understand why nobody else has seen this, though.

> In summary, the only viable changes I see are a few changes from $(CC)
> to $(BOOT_CC) and the conditional running of the FreeType test.  Can
> you explain the need for the rest, if any?

I should really have explained these changes in more detail to start with.
But I hope this clears things up a little.

Cheers,
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20100609/1cfd3e37/attachment.bin