Just a minor note already mentioned in another review.
I'm seeing a number of new files that were obviously copied from
existing files but the copyright information has not been updated. I
know some files have been in the aix-port repo for a while but not, I
think, since 2004 :)
David
On 27/06/2013 1:28 AM, Volker Simonis wrote:
Hi Erik,
thank you for looking at this. I've prepared a new webrev at:
http://cr.openjdk.java.net/~simonis/webrevs/8017568_jdk/
What do you think, do you want to push this directly into the build
repositories or should I push it into the staging repository first?
Please see my further comments inline.
Thank you and best regards,
Volker
On Tue, Jun 25, 2013 at 12:41 PM, Erik Joelsson <erik.joels...@oracle.com>wrote:
On 2013-06-25 12:27, Erik Joelsson wrote:
Hello Volker,
On 2013-06-24 19:23, Volker Simonis wrote:
Hi,
could somebody please review the following change and create an
appropriate
Bug ID for it:
http://cr.openjdk.java.net/~**simonis/webrevs/linux_ppc_**build_jdk/<http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_jdk/>
The patch contains two little changes which are required to build the
class
library part of the OpenJDK on Linux/PPC64. Most of the build magic is
contained in the top-level part of this change which is separately
reviewed
at
http://cr.openjdk.java.net/~**simonis/webrevs/linux_ppc_**build_top<http://cr.openjdk.java.net/~simonis/webrevs/linux_ppc_build_top>
CompileLaunchers.gmk
Remove mapfile from build instructions of BUILD_UNPACKEXE:
CFLAGS_macosx:=-fPIC, \
- MAPFILE:=$(JDK_TOPDIR)/**makefiles/mapfiles/libunpack/**
mapfile-vers-unpack200,\
LDFLAGS:=$(UNPACKEXE_ZIPOBJS),**\
I think it makes no sense to use a version script file for an
executable
and older linkers (e.g. on SLES 10) complain with: "*Invalid version tag
`SUNWprivate_1.1'. Only anonymous version tag is allowed in executable.*"
The GNU ld manual<http://ftp.gnu.org/old-**gnu/Manuals/ld-2.9.1/html_**
node/ld_25.html<http://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_25.html>
states:
"*Version scripts are only meaningful when creating shared libraries.*"
Morover unpack200 was the only executable which used a version script
file.
Unpackexe has some weirdness and this isn't surprising me. Would be
good if someone with more historic knowledge could fill in on the reason
for this. Someone apparently went through the trouble of creating a special
mapfile for this executable. Also, if not using it, should it be removed?
I looked closer at this. These mapfiles were explicitly added in
http://bugs.sun.com/view_bug.**do?bug_id=7033954<http://bugs.sun.com/view_bug.do?bug_id=7033954>,
but it was noted that it broke builds for architectures that didn't have
mapfiles defined. If you look at the launchers, the mapfile is only set if
the arch specific one exists. I think a safer change here would be to make
the mapfile conditional on platform or arch for unpackexe.
I still do not fully understand why we need map-files for executables, but
I also understand that you don't want to change the current setup. So I
went the hard (and hopefully right:) way and implemented a detection of the
buggy linkers on older SuSE distros (e.g. on SLES 10) which complain with:
"Invalid version tag `SUNWprivate_1.1' during the configure step (see
top-level change). Unfortunately we still have quite a lot of these systems
so we really need the build with that buggy ld.
I've therefore added map files with anonymous version tags for these buggy
linkers which are only used if the buggy linker was detected during the
configure step (i.e. if USING_BROKEN_SUSE_LD=yes). Notice that this is no
PPC64 specific problem but a occurs on all SuSE 10 platforms.
And you've been right. I also had to add the arch specific map files for
ppc64 in order to use them for the other launchers.
Kumar, you made the change referred to here, do you have anything to add?
/Erik
Fix typo (replace 'defalt: all' by 'default')
default: all
CompileNativeLibraries.gmk
Only use $(OPENWIN_LIB) for linking LIBSPLASHSCREEN on Solaris! The old
code worked only accidentally when the X-libraries are in the default
linker path anyway. The right solution is to use $(X_LIBS) if not on
Windows or Solaris.
Append -DX_ARCH=X_PPC64 to LIBJSOUND_CFLAGS on PPC64. The value of
X_ARCHisn't actually used on the PPC architectures, but there's a
check to verify
that it is set.
Fix typo (replace 'defalt: all' by 'default')
default: all
Otherwise looks good.
/Erik
