RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
Roger.Riggs at oracle.com
Tue May 7 20:39:35 UTC 2019
Additional comments/observations on the jpackage webrev.
Cleanup opportunities for more maintainable code.
When using the ToolProvider API in an application that has a security
what permissions must be available? Properties, files, ...?
91: "processArguments" seems misnamed for the method that does
- 116: Why type-variable "C" instead of the conventional T?
- Many unused imports (according to IntelliJ) including some in the
same jdk.jpackage.internal that are not needed.
138: TODO and typo in comment.
148: when does the pathSep get replaced with spaces? Will that cause
an issue in re-parsing the string?
661-662: escaped is never set to true, so escaping is not supported?
747: System.getProperty can require permissions to get properties if run
under a security manager.
- 58, 65, 72, 80: Indentation of pLaunchOptions does not line up.
- 262: why the mix of ProcessBuilder and Runtime.exec - stick to
"JPACKAGE_DEBUG" environment variable - ? uppercase, documented?
ValidOptions: 46: typo in comment: "in the a"
VersionExtractor.java seems to be dead code.
Dead code in RelativeFileset: upshift(), contains(), copy constructor().
- should use System Runtime.Version class.
Params.java: Dead code does not seem to be used in DeployParams
(and would probably be better as a map than an individual object).
setParams is unused.
- If there are accessor methods, use them!
- Lots of unused methods. (According to IntelliJ)
- setSystemWide should use primitive boolean, not Boolean; conversions
will be done as needed.
- Ditto installDirChooser
- Ditto SignBundle flag
Many .java files that pass a map of parameters use the argument name "p"
but using "params"
would communicate better and be more consistent across the different files.
LinuxDebBundler and LinuxRpmBundler should share more code.
- list of categories will need maintenance to stay in sync with
Apple. Can the list be derived from the installed Mac tool chain?
- or don't validate the argument until it is built and let the mac app
builder do the checking.
On 04/29/2019 05:58 PM, Andy Herrick wrote:
> jpackage reviewers:
> We hope to move JEP 343 to "Proposed to Target" next week, so we would
> like expedite the review process as much as possible.
More information about the core-libs-dev