[PATCH] Add cross-compilation support

[Andrew John Hughes]

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.

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.
* 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.
* 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.  There is no reason to
change ALT_OUTPUTDIR to its already default setting.  Can you explain
the other changes in that block?

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.

corba-bootstrap.patch:

* What is this for? None of these changes seem to be cross-compilation related.

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)

- 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.  There seems to be a redundant change to 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.
- stubRoutines_zero.hpp: What is this? Why are you extending the Zero
assembler port?
- freetypecheck: Aren't the CFLAGS changes already covered by
icedtea-headers.patch?  The test turn-off seems correct.
- jdk/make/common/Defs.gmk - what are EXTRA_{CFLAGS,CPPFLAGS,LDFLAGS}?
- java/nio/Makefile: Makes sense, similar to BOOT_JAVAC and JAVAC.
Can we not thus use BOOT_CC rather than CC_FOR_BUILD?
- 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?

- sun/font/Makefile: does icedtea-headers not cover these FT changes?
- openjdk/Makefile: seems to be no reason for this
- openjdk/corba/make/common/Defs-linux.gmk: This is the same as
corba-bootstrap.patch!

adlc.make: Again spurious renaming.
platform_zero.in: why is @ZERO_ARCHDEF@ removed?

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?

Thanks,
-- 
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8