RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Roger Riggs]

Hi,

Some comments, a initial batch...

Having support for the ToolProvider is great.
However, there is no indication about whether it is valid to use it from 
multiple threads.
The implementation is structured to be deliberately single thread use only
with the invocation being via a static method and the logging being via 
static methods.
There will need to be a disclaimer and perhaps an exception should be 
thrown.

A future improvement:
The implementation should be methods on the instance created by the 
ToolProvider
and the logging relative to that instance/delegated where needed.
Main can then be a simple lookup of the tool provider and invoke.

jpackage.main.Main:

  - Main.run returns -1, which then is used as exit status, -1 is not 
the usual exit status for a C/Unix main.

65,  the run() method is usually not static and should be re-named to 
avoid confusion

92: Implies something should be logged on a failure.

65: Run (Pw, Pw, args...) doesn't use try ... finally to ensure log is 
flushed.

65: 'throws Exception' implies it can throw an exception but is 
ambiguous as to whether
it returns an error number or throws on what kinds of errors?

91: Ambiguous as to whether processArguments() throws or return false!

CommandLine:
59: Would be more flexible and powerful to use List<String> consistently...

67: use arg.startsWith() for cleaner code.

102: Seems wasteful to parse all the arguments twice.

jdk.jpackage.internal classes:

AbstractAppImageBuilder:
  57: The constructor does not need to throw IOException

60:  are .diz files common enough to preemptively exclude (w/o 
documentation)

89: Can the mix of old File API and new Path/Files APIs be avoided?

Adding javadoc to the abstract builder methods will help future maintainers.

203: is a bit more generous than most CLASSPATH parsing and might lead 
to non-obvious bugs.
    For example, a path component with a space in it!

229:  There is no mention of debugging support in JEP 343.
   Where is this functionality defined or is it to be identified as 
undocumented internal implementation

Regards, Roger