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

[Andy Herrick]

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")


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 ?


/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.
>>>