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

[Roger Riggs]

Hi,

On 11/14/2018 09:52 AM, Andy Herrick wrote:
>
> On 11/13/2018 5:50 PM, Philip Race wrote:
>>
>>
>> On 11/13/18, 12:52 PM, Roger Riggs wrote:
>>> Hi,
>>>
>>> There are enough files unique to each platform to put them in 
>>> separate packages
>>> otherwise you get too many (IMHO) files in a single 
>>> package/directory and
>>> its harder to tell which go with which.  There isn't much of a 
>>> problem with
>>> classes being public because they are all in a module and not exported.
>>>
>>> I would put them all under 
>>> share/classes/jdk/jpackagers/internal/<OS> and
>>> save a directory level.
>>
>> That isn't how the rest of the JDK is organised.
>> Platform specific classes are split where you have "share" in the 
>> path above.
>> So whilst the other issues are more arguable I don't think the build 
>> team
>> would like platform specific classes under share. There is already an
>> objection to that for the native files.
>>
>> To the "too many files in one package/directory" point.
>> I think that might happen at the directory level if Andy went through 
>> with
>> his suggestion but I don't think it will happen with what I proposed and
>> we probably should get some benefit from being able to make classes + 
>> methods
>> package private.
>
> but my proposal only increases the number of files in each directory 
> as follows:
>
> share/classes/jdk/jpackager/internal       - from 28 to 31
>
> windows/classes/jdk/jpackager/internal - from 6 to 7
>
> macosx/classes/jdk/jpackager/internal   - from 6 to 7
>
> linux/classes/jdk/jpackager/internal        - from 3 to 4
>
> (adding Main.java, that makes are only 50 source files total)
>
> so I hardly see any any impact of having "too many files in one 
> package/directory"
>
> and for Phil's part of the change, although the platform dependent 
> source files would be in the same package as the platform independent 
> files, they are still in a different source dir, and every platform 
> specific source file's name starts with the platform ("Win", "Mac", or 
> "Linux")
This should be fine, since there is no need to for cross-platform support,
the reason for the top level platform directory is so the build can be 
selectively
include the right files.

I assume the service loader mechanism deals with all the issues in 
locating platform specific files.
>
>
> finally a question about resources:
>
> Each source file that references resources has it's own resource file 
> (23 resource files in each of 3 languages for a total of 69 properties 
> files).
>
> That seems like overkill to me. One result is duplicate resources 
> whenever a key is referenced in more than one source file.
>
> Wouldn't it simplify things to combine these into a smaller set of 
> resource files ?
agreed, there should be fewer resource files to track and maintain and 
NO duplication.

Roger

>
>
> /Andy
>
>
>>
>> So I think what I proposed is about right ..
>>
>> phil
>>>
>>> $.02, Roger
>>>
>>>
>>> On 11/13/2018 03:46 PM, Andy Herrick wrote:
>>>> I agree with this and would take it further.
>>>>
>>>> 1 file is in ./share/classes/jdk/jpackager/internal/builders - why 
>>>> not just ./share/classes/jdk/jpackager/internal
>>>>
>>>> 2 files are in ./share/classes/jdk/jpackager/internal/bundlers - 
>>>> why not just in ./share/classes/jdk/jpackager/internal
>>>>
>>>> 1 file is in ./linux/classes/jdk/jpackager/internal/builders/linux 
>>>> - why not just ./linux/classes/jdk/jpackager/internal
>>>>
>>>> 1 file is in ./macosx/classes/jdk/jpackager/internal/builders/mac - 
>>>> why not just ./macosx/classes/jdk/jpackager/internal
>>>>
>>>> 1 file is in 
>>>> ./windows/classes/jdk/jpackager/internal/builders/windows - why not 
>>>> just ./windows/classes/jdk/jpackager/internal
>>>>
>>>> then just move the associated resources -
>>>>
>>>> Basically put everything except Main in same package - everything 
>>>> would be easier to find, and we could make almost all methods 
>>>> package-private (the only exception would be the few things called 
>>>> by Main, and the ToolProvider.
>>>>
>>>>
>>>> On 11/13/2018 2:54 PM, Phil Race wrote:
>>>>> Question .. why is "mac", "linux" and "windows" necessary in the 
>>>>> package name here ?
>>>>>
>>>>>  src/jdk.jpackager/macosx/classes/jdk/jpackager/internal/mac/MacAppBundler.java 
>>>>>
>>>>>  src/jdk.jpackager/windows/classes/jdk/jpackager/internal/builders/windows/WindowsAppImageBuilder.java 
>>>>>
>>>>> src/jdk.jpackager/linux/classes/jdk/jpackager/internal/linux/LinuxRpmBundler.java 
>>>>>
>>>>>
>>>>> There's not likely to be a clash, so is there some other reason 
>>>>> not to want these
>>>>> in the same package as the shared internals like
>>>>> src/jdk.jpackager/share/classes/jdk/jpackager/internal/Param.java
>>>>>
>>>>> ?
>>>>>
>>>>> -phil.
>>>>