Preliminary request for review: 7025066 Build system changes to support SE Embedded integration

[Dr Andrew John Hughes]

On 10:51 Tue 08 Mar     , David Holmes wrote:
> Andrew,
> 
> Many thanks for the feedback:
> 
> Dr Andrew John Hughes said the following on 03/08/11 04:30:
> > On 09:39 Mon 07 Mar     , Kelly O'Hair wrote:
> > 
> > General comments:
> > 
> > * Could this not be broken up into smaller changesets to make it easier to review and catch regressions?
> 
> In theory probably, in practice, not really. Please see below.
> 
> > * There seem to be some whitespace changes that shouldn't be there.
> > 
> > e.g.
> > 
> > -	sane-msvcrt_path \
> > +	sane-msvcrt_path \
> 
> This will all be double-checked before the push.
> 
> >> Just to clarify for people, BUILD_CLIENT_ONLY refers to building the client VM only.
> >> Some of these variables should be documented in the top level README-builds.html file, but that
> >> can be done under a separate CR if necessary.
> >>
> > 
> > What happens if there is no client VM e.g. x86_64?
> 
> If you are not building a client-only configuration then you should not 
> set BUILD_CLIENT_ONLY. It's main purpose is to not attempt to copy 
> lib/<arch>/server/*.so files into the JRE/JDK image when they don't exist.
> 

But what if someone does?  Will the sanity check not catch this?

> > I think if this patch introduces it, the documentation should be part of this patch.
> 
> Definitely.
> 
> >> The Library.gmk file seems like it is just a leftover debugging echo?
> 
> This was Kelly's comment, but yes it is a _very_ useful debugging echo 
> that I suggest leaving in.
> 
> >> The Release.gmk file needs  a major overhaul. :^(  I'm wondering if the various things being added here could
> >> have their own separate functionality flags, rather than JAVASE_EMBEDDED,  have COMPRESS_JARS=true/false,
> >> REDUCED_JRE=true/false, then have one place where JAVASE_EMBEDDED turns on what it wants?
> > 
> > Strongly agree with this.  It would make more sense to have these sort
> > of changes introduced by separate changesets and then have this change
> > set them for the JAVASE_EMBEDDED build.  In particular, COMPRESS_JARS
> > was something I had planned on fixing.  To Kelly's list, I'd add
> > BUILD_ALSA and LIBZIP_CAN_USE_MMAP (mentioned below).  This would make
> > this work far more generally useful than it is at present.
> 
> I can certainly refactor into general flags, however applying those 
> flags to all parts of the JDK build process that might want to use them 
> is out-of-scope for what I am trying to achieve here (I just won't have 
> time to do this and go through a myriad of builds and test runs). I'd 
> also prefer not to create multiple changesets, which implies multiple 
> CRs. I can't easily test these things in isolation, and individual 
> changesets would require complete build/test cycles that I again do not 
> have time to perform.
> 

I can see the issue with it taking more time, but committing the patch in its
current form is unfair on others who now have a more convoluted build system
to deal with (and it's already bad to start with) and no gain from the patch.

Does your employer not allocate sufficient time to do the best work you can
on such changes?

> > Has this been tested on an OpenJDK build at all?  It also seems to create a fonts.dir file with fonts
> > that aren't part of OpenJDK.
> 
> No, not OpenJDK. I have done a regular JDK build, but not OpenJDK. And I 
> have not attempted to test things like BUILD_CLIENT_ONLY outside of a 
> JAVASE_EMBEDDED build.
> 
> With regard to the fonts, my understanding is that we simply define a 
> subset of the fonts used in the regular JDK. I have no idea how such 
> fonts get shipped nor whether they are part of the OpenJDK or only 
> Oracle's JDK.
> 

An OpenJDK build definitely needs to be performed before this is committed,
as I think it may break the build or, at the very least, create references
to non-existent fonts.

> Thanks again,
> 
> David
> 
> >> -kto
> >>
> >>
> >> On Mar 7, 2011, at 2:14 AM, David Holmes wrote:
> >>
> >>> http://cr.openjdk.java.net/~dholmes/7025066/webrev/
> >>>
> >>> The SE-Embedded product is a combination of open and closed sources. To allow SE-Embedded to be built from standard OpenJDK sources we need to apply a number of changes to the SE 7 build system:
> >>>
> >>> - support for building the hotspot client compiler only (hotspot already supports this, this is the JDK side of things)
> >>> - support for doing cross-compilation (Linux only)
> >>> - minimal support for ARM/PPC architectures in the open code that currently only knows about x86 and sparc
> >>> - SE-Embedded specific build settings and targets (specifically the headful and headless reduced JRE images)
> >>>
> >>> ---
> >>>
> >>> These changes are obviously primarily for Oracle's benefit, but some aspects may be use of externally as well. The hope is that these changes won't have an adverse affect on any downstream OpenJDK builders, but until I get feedback on that I won't know.
> >>>
> >>> Thanks,
> >>> David Holmes
> >>> SE Embedded Team
> >>>
> > 

-- 
Andrew :)

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

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37