Hi Andy,
This is looking really sweet from a build perspective!
Just a few minor nits:
* In Launcher-jdk.jpackage.gmk, please indent the else clause ("$(eval
$(call SetupBuildLauncher, jpackage ....") two spaces.
* In Lib-jdk.jpackage.gmk, I think there's still room to prune some more
unnecessary compiler flags and parameters to SetupJdkExecutable:
65 CFLAGS_linux := -fPIC, \
66 CFLAGS_solaris := -KPIC, \
67 CFLAGS_macosx := -fPIC, \
I wonder if these really are needed. Normally, only shared libraries
neeed PIC code. (And for those we set it automatically.)
69 LDFLAGS_windows := -nologo, \
This should not be needed, it's incorporated in CXXFLAGS_JDKEXE. (Also
in the second block, for jpackageapplauncherw).
72 LIBS_solaris := -lc, \
Same here; this should not be needed. It's in GLOBAL_LIBS on Solaris,
and is always included.
75 VERSIONINFO_RESOURCE := $(GLOBAL_VERSION_INFO_RESOURCE), \
This is setup by default by SetupJdkExecutable, so you can remove it.
(Also in the second block, for jpackageapplauncherw).
Finally, I do believe that the setups of jpackageapplauncher and
jpackageapplauncherw should be done in Launcher-jdk.jpackage.gmk, not
Lib-jdk.jpackage.gmk. Since they are to be shipped as resources, they
are not "really" launchers in our normal sense, but they are at least
more launchers than they are libraries.
As we've talked about privately, in the future I'd like to see Windows
too use the SetupBuildLauncher method for creating the launcher, but
this is clearly good enough for inclusion in the mainline.
I also have a question about
test/jdk/tools/jpackage/resources/license.txt. What is it used for? And
why the odd (incorrect?) spelling of license?
/Magnus
On 2019-01-11 20:41, Andy Herrick wrote:
This is the second update to the Request For Review of the
implementation of the Java Packager Tool (jpackager) as described in
JEP 343: Packaging Tool
<https://bugs.openjdk.java.net/browse/JDK-8200758>
This webrev corresponds to the second EA build, Build 8 (2019/1/8),
posted at http://jdk.java.net/jpackage/
This update renames the package used to "jdk.jpackage", removes the
Single Instance Service (and CLI option --singleton), adds several
other CLI options, adds more automated tests, and contains fixes to
the following issues (since update 1 on 11/09/2018):
JDK-8212164 resolve jre.list and jre.module.list
JDK-8212936 Makefile and other improvements for jpackager
JDK-8213385 jpackager Command-Line Argument File.
JDK-8213392 Enhance --help and --version
JDK-8213425 Analyze output from Source code scanner and fix where
needed.
JDK-8213747 Makefile Improvements to Lib-jdk.packager.gmk
JDK-8213748 jpackager native launcher for macosx, linux.
JDK-8213756 SingleInstance runtime improvements
JDK-8213962 JPackageCreateImageRuntimeModuleTest fails
JDK-8213963 Flatten out jpackager packages and resources.
JDK-8214021 Create additional automated tests for jpackager
JDK-8214051 rename jpackager tool to jpackage
JDK-8214070 Analyze and Fix issues reported by Parfait
JDK-8214143 Reduce Resource files
JDK-8214495 Change behavior of --license-file
JDK-8214575 Exe installers will install application over existing
installation
JDK-8214899 rename papplauncher and it's library and move src to
appropriate places
JDK-8214982 jpackage causes failures in existing HelpFlagsTest and
VersionCheck tests
JDK-8215020 create-jre-installer exe fails when --runtime-image is
specified.
JDK-8215036 Create initial set of tests for jpackage
create-installer mode
JDK-8215453 remove unused BundlerParams and fix misleading messages
JDK-8215515 Add a command line option to override internal resources.
JDK-8215900 Without --files args, only jars in the top level of
--input are added to class-path
JDK-8215903 modify behavior of retaining temporary output dir
JDK-8216190 Remove Single Instance Service support from jpackage
JDK-8216313 Add ToolProvider information to jdk.jpackage API docs
JDK-8216373 temporary build-root left behind when using secondary
launcher(s)
JDK-8216492 Update copyright of all new jpackage files to 2019
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.03
Note: SingleInstanceService API was removed (see
https://bugs.openjdk.java.net/browse/JDK-8216190).
An example stand alone implementation can be found at:
http://cr.openjdk.java.net/~herrick/jpackage/Singleton.java
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
