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

[Andy Herrick]

yes - The attempted incremental patch is not much use.  The source files 
were moved several times, and despite using "hg addremove -s 60", many 
of the files show as a remove and a new file.

/Andy


On 11/26/2019 9:01 PM, Philip Race wrote:
> > I believe otherwise it is an accurate incremental webrev of the 
> jpackage changes since EA-5.
>
> It is also not very incremental.
> *
> *src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/main/Main.java
>
> is definitely not "new" ...
>
> -phil.
>
> On 11/26/19, 2:17 PM, Andy Herrick wrote:
>> yes,  this incremental webrev contains the JNLPConverter code, which 
>> it should not.
>>
>> I believe otherwise it is an accurate incremental webrev of the 
>> jpackage changes since EA-5.
>>
>> /Andy
>>
>> On 11/26/2019 4:53 PM, Phil Race wrote:
>>> Andy,
>>>
>>> I am puzzled by what I see here
>>> > The webrev at [3] shows the changes since EA-06 (Build 
>>> 13-jpackage+1-49 ) up to the current point.
>>>
>>> > [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/
>>>
>>> This includes the JNLPConverter which isn't what I expected to see and
>>> (correctly) isn't in the cumulative webrev ....
>>>
>>> Since [3] seemed like the most obvious thing to do a review of the 
>>> recent
>>> changes I'd like to be sure it has the correct content and I am not 
>>> sure it does ...
>>>
>>> -phil.
>>>
>>> On 11/26/19 1:36 PM, Kevin Rushforth wrote:
>>>> This all looks good.
>>>>
>>>> +1
>>>>
>>>> -- Kevin
>>>>
>>>>
>>>> On 11/26/2019 12:56 PM, Erik Joelsson wrote:
>>>>> (adding build-dev)
>>>>>
>>>>> Build changes look good.
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2019-11-20 09:37, Andy Herrick wrote:
>>>>>> I posted new composite webrev [7], and git patch [8] after 
>>>>>> pushing fix for JDK-8234402 [6].
>>>>>>
>>>>>> [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/
>>>>>>
>>>>>> [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch
>>>>>>
>>>>>> /Andy
>>>>>>
>>>>>> On 11/19/2019 3:13 PM, Kevin Rushforth wrote:
>>>>>>> I took the "git diff" patch [5] that you uploaded yesterday, 
>>>>>>> applied it, and verified that it is the same as what is in the 
>>>>>>> JDK-8200758-branch branch of the sandbox. It builds and runs 
>>>>>>> fine for me.
>>>>>>>
>>>>>>> I ran jcheck and it found the following three files with 
>>>>>>> whitespace errors that will need to be fixed before you push:
>>>>>>>
>>>>>>> src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: 
>>>>>>> Trailing whitespace
>>>>>>> src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: 
>>>>>>> Trailing whitespace
>>>>>>> test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing 
>>>>>>> whitespace
>>>>>>>
>>>>>>> The second of these will go away with the fix for JDK-8234402 
>>>>>>> [6], so you don't need to do anything there. Once the fix for 
>>>>>>> JDK-8234402 is pushed to sandbox, I presume you will update the 
>>>>>>> webrev, right?
>>>>>>>
>>>>>>> Pending the fix for JDK-8234402 and the needed white-space 
>>>>>>> fixes, this is a +1 from me (although I am not a jdk Project 
>>>>>>> Reviewer, so you will need at least one review from someone who 
>>>>>>> is).
>>>>>>>
>>>>>>> -- Kevin
>>>>>>>
>>>>>>> [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch
>>>>>>> [6] https://bugs.openjdk.java.net/browse/JDK-8234402
>>>>>>>
>>>>>>>
>>>>>>> On 11/13/2019 4:23 PM, Andy Herrick wrote:
>>>>>>>> Please review  changes for [1] which is the implementation bug 
>>>>>>>> for JEP-343.
>>>>>>>>
>>>>>>>> The webrev at [2] is the total cumulative webrev of changes for 
>>>>>>>> the jpackage tool, currently in the JDK-8200758-branch branch 
>>>>>>>> of the open sandbox repository.
>>>>>>>>
>>>>>>>> The webrev at [3] shows the changes since EA-06 (Build 
>>>>>>>> 13-jpackage+1-49 ) up to the current point.
>>>>>>>>
>>>>>>>> The latest EA (Build 14-jpackage+1-49 ) is posted at [4].
>>>>>>>>
>>>>>>>> Please send feedback to core-libs-dev at openjdk.java.net
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8212780
>>>>>>>>
>>>>>>>> [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/
>>>>>>>>
>>>>>>>> [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/
>>>>>>>>
>>>>>>>> [4] http://jdk.java.net/jpackage/
>>>>>>>>
>>>>>>>
>>>>
>>>