Hello David,
Most of it looks good, but profile-includes.txt could certainly benefit
from some reduction in duplication. (Aside from the extension of the
file itself, which I find a bit weird, but it's already there.) The two
common structures that I can see are:
1. Expanding debuginfo files for macosx.
2. Assigning/expanding libraries to the correct variable.
For macosx debuginfo expanding, I would define a macro to something like
this:
# Expand the contents of the .dSYM directories on macosx.
# Param 1 - debug files list
# Param 2 - libraries list
define expand-debuginfo
$(if $(and $(filter-out true, $(ZIP_DEBUGINFO_FILES)), $(filter
macosx, $(OPENJDK_TARGET_OS))), \
$(foreach i, $1, $(addsuffix /Contents/Info.plist, $i)) \
$(foreach i, $2, $(addsuffix /Contents/Resources/DWARF/$i,
$(filter $i.%, $1))), \
$1)
endef
And use it like this:
PROFILE_1_DEBUG_FILES := $(call expand-debuginfo,
$(PROFILE_1_DEBUG_FILES), $(PROFILE_1_LIBRARIES))
Filtering out jsig can be done on the parameters at the macro call.
For the conditional addprefix of OPENJDK_TARGET_CPU_LEGACY_LIB, I would
do something like:
ifeq (, $(findstring $(OPENJDK_TARGET_OS), windows macosx))
LIB_SUBDIR := $(OPENJDK_TARGET_CPU_LEGACY_LIB)/
else
LIB_SUBDIR :=
endif
And then always assign PROFILE_*_JRE_LIB_FILES with $(addprefix
$(LIB_SUBDIR), ...). The conditional on Windows for assigning to
...BIN_FILES or ...LIB_FILES I would leave in place since I don't think
a macro solution would make it easier to understand.
/Erik
On 2014-12-01 01:09, David Holmes wrote:
Main bug is 8038189 but that is a closed bug, the open backport issue is:
https://bugs.openjdk.java.net/browse/JDK-8066200
Compact Profiles support was added in 8, but only for the Linux
platform. I've now generalized this support to all the other
platforms. This is an 8u only change, targetting 8u60, as soon as the
jdk8u/dev starts accepting things for 8u60.
The changes are not that extensive - mostly generalizing the lists and
accounting for different platforms putting files into different places
(jre/lib/<arch> vs jre/lib vs jre/bin). It isn't necessary to produce
detailed per-platform lists as files that don't exist simply don't get
copied; but when files are obviously platform specific then I add them
under suitable guards.
The biggest complexity comes from the debuginfo files, and in
particular unzipped debuginfo files. This accounts for the bulk of the
changes in profiles-includes.txt, as we try to identify the set of
debug files that might exist for each library (and OSX is the main
complication due to the .dSYM directory because the existing rules
only copy files not directories). Suggestions for reducing the
duplicated patterns would be appreciated.
Platform specific contents were determined in conjunction with
examination of what Jigsaw is using in JDK 9 for the base module.
I tested all the main platforms (Windows, Linux, Solaris and OSX) and
with/without zipping of debuginfo files. Note that Windows builds will
not work with unzipped debuginfo files due to 8025936, but I checked
that multiple debug info files were expanded into the right set of
targets.
webrevs:
http://cr.openjdk.java.net/~dholmes/8038189/webrev.top/
make/Main.gmk:
- Remove the os-check that constrained profiles to linux
http://cr.openjdk.java.net/~dholmes/8038189/webrev.jdk/
make/CreateJars.gmk:
- Check for empty variables (Solaris doesn't like them)
- Fix # # comments
- Add explicit targets for the beanless classes with $ in them (the %
substitution doesn't work on all platforms when $ is also present)
make/Images.gmk
- Don't filter out server VM from compact profiles
make/Import.gmk
- Add Info.plist to the exported files (for unzipped debuginfo files
- this is a general fix not profiles specific so may warrant its own CR)
make/Profiles.gmk
- Remove linux-only comment
make/Tools.gmk
- Fix tool definitions to use $(PATH_SEP) and quote cp entries
make/profile-includes.txt
- Bulk of changes as described above
make/profile-rtjar-includes.txt
- Additional packages that exist on OSX only (but don't need to be
conditionally added)
- NOTE: if AIX or other platform add platform specific packages not
already included by an enclosing package, then they will also need to
be added
Thanks,
David
