JDK-8154898 : [packager] Refactor Java Packager
  • Type: Bug
  • Component: deploy
  • Sub-Component: packager
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-04-21
  • Updated: 2016-04-29
  • Resolved: 2016-04-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Blocks :  
Blocks :  
Blocks :  
Relates :  
Description
This task is the JavaFX side to refactor the parts of the jdk.packager that depend on jdk.jlink.
Comments
changeset: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/2b562a2d0fca
29-04-2016

Looks great. +1
29-04-2016

You're right, the order could matter. The source map is a LinkedHashMap. Do you want a new webrev for that change? http://cr.openjdk.java.net/~cbensen/JDK-8154898/webrev.09/
29-04-2016

Looks good. I tested an FX and JDK build with the jdk.packager disabled and then with it enabled. I do have one minor formatting comment and one question: modules/fxpackager/src/main/java/com/oracle/tools/packager/JLinkBundlerHelper.java * Indentation is wrong for the closing curly brace in the following: 255 imageBuilder.prepareApplicationFiles(); 256 } 257 258 private static Map<String, String> convertMapOfObjectToMapOfStrings(Map<String, Object> options) { No need to generate a new webrev in order to fix the indentation. * I see the following used as the return value in convertMapOfObjectToMapOfStrings: 259 Map<String, String> result = new HashMap<String, String>(); Does the order of arguments matter? If so, then you might consider using a LinkedHashMap (here and any other place where you need to preserve the order). If this is just an arbitrary collection and order is irrelevant, then it might be OK as is. +1
29-04-2016

9-dev review: http://cr.openjdk.java.net/~cbensen/JDK-8154898/webrev.08/
28-04-2016

Thanks for the detailed list Kevin. I have added all dependencies all all four related issues: JDK-8150990 JDK-8155055 JDK-8155056 and this one. - I think this should be pushed to 9-dev. - I fixed the copyright. I'll send out a new review against 9-dev shortly.
27-04-2016

I'll do a more detailed review in a bit. A couple quick comments: 1) This change is dependent on (is blocked by) JDK-8150990 so please add the appropriate issue link. Once the review is done, the logistics of getting this in will be: A) disable the fxpackager build B) you push the change C) after we have promoted jdk9 build that includes the fix for JDK-8150990 we need to upgrade our JIGSAW_HOME to use it D) enable the fxpackager build B and C can go in either order, but the rest have to be sequential. 2) The patch was generated against sandbox-9-jake rather than 9-dev (the patch fails to apply in the latter). Where do you propose to push it? 3) Minor: the module-info.java.extra file is new this year, so just needs to be Copyright 2016.
27-04-2016

Review: http://cr.openjdk.java.net/~cbensen/JDK-8154898/webrev.03/
27-04-2016

Review: http://cr.openjdk.java.net/~cbensen/JDK-8154898/webrev.01/
22-04-2016