RFR: JDK-8212780: JEP 343: Packaging Tool Implementation (update 2)

[Magnus Ihse Bursie]

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 at openjdk.java.net
>
> /Andy Herrick
>